From 1deca0425db3e74a61cb732e729c0777904e549c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 19 Jun 2022 13:59:36 -0500 Subject: [PATCH 1/2] Greatly improve error reporting for futures and generators in `note_obligation_cause_code` Most futures don't go through this code path, because they're caught by `maybe_note_obligation_cause_for_async_await`. But all generators do, and `maybe_note` is imperfect and doesn't catch all futures. Improve the error message for those it misses. At some point, we may want to consider unifying this with the code for `maybe_note_async_await`, so that `async_await` notes all parent constraints, and `note_obligation` can point to yield points. But both functions are quite complicated, and it's not clear to me how to combine them; this seems like a good incremental improvement. --- .../src/traits/error_reporting/suggestions.rs | 105 ++++++++++++++++-- src/test/ui/async-await/issue-68112.stderr | 30 +++-- ...e-70935-complex-spans.drop_tracking.stderr | 40 +++++++ ...> issue-70935-complex-spans.normal.stderr} | 8 +- .../async-await/issue-70935-complex-spans.rs | 16 ++- .../partial-drop-partial-reinit.rs | 8 ++ .../partial-drop-partial-reinit.stderr | 20 +++- src/test/ui/closures/closure-move-sync.stderr | 16 ++- src/test/ui/generator/issue-68112.rs | 22 +++- src/test/ui/generator/issue-68112.stderr | 41 +++++-- src/test/ui/generator/not-send-sync.stderr | 11 +- .../print/generator-print-verbose-1.stderr | 33 +++++- .../print/generator-print-verbose-2.stderr | 11 +- src/test/ui/impl-trait/auto-trait-leak2.rs | 18 ++- .../ui/impl-trait/auto-trait-leak2.stderr | 32 ++++-- .../interior-mutability.stderr | 6 +- .../ui/kindck/kindck-nonsendable-1.stderr | 6 +- src/test/ui/no-send-res-ports.stderr | 11 +- src/test/ui/not-clone-closure.stderr | 9 +- .../auto-trait-leakage2.rs | 8 +- .../auto-trait-leakage2.stderr | 10 +- 21 files changed, 389 insertions(+), 72 deletions(-) create mode 100644 src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr rename src/test/ui/async-await/{issue-70935-complex-spans.stderr => issue-70935-complex-spans.normal.stderr} (81%) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 09b73b982a0c1..159fcf932a1da 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -8,6 +8,7 @@ use crate::infer::InferCtxt; use crate::traits::normalize_to; use hir::HirId; +use rustc_ast::Movability; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{ @@ -2397,24 +2398,104 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { { let parent_trait_ref = self.resolve_vars_if_possible(data.parent_trait_pred); - let ty = parent_trait_ref.skip_binder().self_ty(); - matches!(ty.kind(), ty::Generator(..)) - || matches!(ty.kind(), ty::Closure(..)) + let nested_ty = parent_trait_ref.skip_binder().self_ty(); + matches!(nested_ty.kind(), ty::Generator(..)) + || matches!(nested_ty.kind(), ty::Closure(..)) } else { false } }; + let future_trait = self.tcx.lang_items().future_trait().unwrap(); + let opaque_ty_is_future = |def_id| { + self.tcx.explicit_item_bounds(def_id).iter().any(|(predicate, _)| { + if let ty::PredicateKind::Trait(trait_predicate) = + predicate.kind().skip_binder() + { + trait_predicate.trait_ref.def_id == future_trait + } else { + false + } + }) + }; + + let from_generator = tcx.lang_items().from_generator_fn().unwrap(); + // Don't print the tuple of capture types - if !is_upvar_tys_infer_tuple { - let msg = format!("required because it appears within the type `{}`", ty); - match ty.kind() { - ty::Adt(def, _) => match self.tcx.opt_item_ident(def.did()) { - Some(ident) => err.span_note(ident.span, &msg), - None => err.note(&msg), - }, - _ => err.note(&msg), - }; + 'print: { + if !is_upvar_tys_infer_tuple { + let msg = format!("required because it appears within the type `{}`", ty); + match ty.kind() { + ty::Adt(def, _) => { + // `gen_future` is used in all async functions; it doesn't add any additional info. + if self.tcx.is_diagnostic_item(sym::gen_future, def.did()) { + break 'print; + } + match self.tcx.opt_item_ident(def.did()) { + Some(ident) => err.span_note(ident.span, &msg), + None => err.note(&msg), + } + } + ty::Opaque(def_id, _) => { + // Avoid printing the future from `core::future::from_generator`, it's not helpful + if tcx.parent(*def_id) == from_generator { + break 'print; + } + + // If the previous type is `from_generator`, this is the future generated by the body of an async function. + // Avoid printing it twice (it was already printed in the `ty::Generator` arm below). + let is_future = opaque_ty_is_future(def_id); + debug!( + ?obligated_types, + ?is_future, + "note_obligation_cause_code: check for async fn" + ); + if opaque_ty_is_future(def_id) + && obligated_types.last().map_or(false, |ty| match ty.kind() { + ty::Opaque(last_def_id, _) => { + tcx.parent(*last_def_id) == from_generator + } + _ => false, + }) + { + break 'print; + } + err.span_note(self.tcx.def_span(def_id), &msg) + } + ty::GeneratorWitness(bound_tys) => { + use std::fmt::Write; + + // FIXME: this is kind of an unusual format for rustc, can we make it more clear? + // Maybe we should just remove this note altogether? + // FIXME: only print types which don't meet the trait requirement + let mut msg = + "required because it captures the following types: ".to_owned(); + for ty in bound_tys.skip_binder() { + write!(msg, "`{}`, ", ty).unwrap(); + } + err.note(msg.trim_end_matches(", ")) + } + ty::Generator(def_id, _, movability) => { + let sp = self.tcx.def_span(def_id); + + // Special-case this to say "async block" instead of `[static generator]`. + let kind = if *movability == Movability::Static { + "async block" + } else { + "generator" + }; + err.span_note( + sp, + &format!("required because it's used within this {}", kind), + ) + } + ty::Closure(def_id, _) => err.span_note( + self.tcx.def_span(def_id), + &format!("required because it's used within this closure"), + ), + _ => err.note(&msg), + }; + } } obligated_types.push(ty); diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index 28d94b14ac914..b8d3e1540d8a9 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -42,15 +42,27 @@ LL | require_send(send_fut); | = help: the trait `Sync` is not implemented for `RefCell` = note: required because of the requirements on the impl of `Send` for `Arc>` - = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:47:31: 47:36]` - = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:47:31: 47:36]>` - = note: required because it appears within the type `impl Future>>` - = note: required because it appears within the type `impl Future>>` - = note: required because it appears within the type `impl Future>>` - = note: required because it appears within the type `{ResumeTy, impl Future>>, (), i32, Ready}` - = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:55:26: 59:6]` - = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:55:26: 59:6]>` - = note: required because it appears within the type `impl Future` +note: required because it's used within this async block + --> $DIR/issue-68112.rs:47:31 + | +LL | async fn ready2(t: T) -> T { t } + | ^^^^^ +note: required because it appears within the type `impl Future>>` + --> $DIR/issue-68112.rs:48:31 + | +LL | fn make_non_send_future2() -> impl Future>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: required because it captures the following types: `ResumeTy`, `impl Future>>`, `()`, `i32`, `Ready` +note: required because it's used within this async block + --> $DIR/issue-68112.rs:55:26 + | +LL | let send_fut = async { + | __________________________^ +LL | | let non_send_fut = make_non_send_future2(); +LL | | let _ = non_send_fut.await; +LL | | ready(0).await; +LL | | }; + | |_____^ note: required by a bound in `require_send` --> $DIR/issue-68112.rs:11:25 | diff --git a/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr b/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr new file mode 100644 index 0000000000000..19fd5eb7c73fb --- /dev/null +++ b/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr @@ -0,0 +1,40 @@ +error[E0277]: `Sender` cannot be shared between threads safely + --> $DIR/issue-70935-complex-spans.rs:13:45 + | +LL | fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { + | ^^^^^^^^^^^^^^^^^^ `Sender` cannot be shared between threads safely + | + = help: the trait `Sync` is not implemented for `Sender` + = note: required because of the requirements on the impl of `Send` for `&Sender` +note: required because it's used within this closure + --> $DIR/issue-70935-complex-spans.rs:25:13 + | +LL | baz(|| async{ + | _____________^ +LL | | foo(tx.clone()); +LL | | }).await; + | |_________^ +note: required because it's used within this async block + --> $DIR/issue-70935-complex-spans.rs:9:67 + | +LL | async fn baz(_c: impl FnMut() -> T) where T: Future { + | ___________________________________________________________________^ +LL | | +LL | | } + | |_^ + = note: required because it captures the following types: `ResumeTy`, `impl Future`, `()` +note: required because it's used within this async block + --> $DIR/issue-70935-complex-spans.rs:23:16 + | +LL | async move { + | ________________^ +LL | | +LL | | baz(|| async{ +LL | | foo(tx.clone()); +LL | | }).await; +LL | | } + | |_____^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/async-await/issue-70935-complex-spans.stderr b/src/test/ui/async-await/issue-70935-complex-spans.normal.stderr similarity index 81% rename from src/test/ui/async-await/issue-70935-complex-spans.stderr rename to src/test/ui/async-await/issue-70935-complex-spans.normal.stderr index db3099381196b..2174f260a7135 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.stderr +++ b/src/test/ui/async-await/issue-70935-complex-spans.normal.stderr @@ -1,12 +1,12 @@ error: future cannot be sent between threads safely - --> $DIR/issue-70935-complex-spans.rs:10:45 + --> $DIR/issue-70935-complex-spans.rs:13:45 | LL | fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { | ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send` | = help: the trait `Sync` is not implemented for `Sender` note: future is not `Send` as this value is used across an await - --> $DIR/issue-70935-complex-spans.rs:15:11 + --> $DIR/issue-70935-complex-spans.rs:27:11 | LL | baz(|| async{ | _____________- @@ -14,9 +14,9 @@ LL | | foo(tx.clone()); LL | | }).await; | | - ^^^^^^ await occurs here, with the value maybe used later | |_________| - | has type `[closure@$DIR/issue-70935-complex-spans.rs:13:13: 15:10]` which is not `Send` + | has type `[closure@$DIR/issue-70935-complex-spans.rs:25:13: 27:10]` which is not `Send` note: the value is later dropped here - --> $DIR/issue-70935-complex-spans.rs:15:17 + --> $DIR/issue-70935-complex-spans.rs:27:17 | LL | }).await; | ^ diff --git a/src/test/ui/async-await/issue-70935-complex-spans.rs b/src/test/ui/async-await/issue-70935-complex-spans.rs index 2965a7e0654a4..4bf94fe342c1d 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.rs +++ b/src/test/ui/async-await/issue-70935-complex-spans.rs @@ -1,16 +1,28 @@ // edition:2018 +// revisions: normal drop_tracking +// [drop_tracking]compile-flags:-Zdrop-tracking // #70935: Check if we do not emit snippet // with newlines which lead complex diagnostics. use std::future::Future; async fn baz(_c: impl FnMut() -> T) where T: Future { +//[drop_tracking]~^ within this async block } fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { - //~^ ERROR: future cannot be sent between threads safely + //[normal]~^ ERROR: future cannot be sent between threads safely + //[drop_tracking]~^^ ERROR: `Sender` cannot be shared + //[drop_tracking]~| NOTE: cannot be shared + //[drop_tracking]~| NOTE: requirements on the impl of `Send` + //[drop_tracking]~| NOTE: captures the following types + //[drop_tracking]~| NOTE: in this expansion + //[drop_tracking]~| NOTE: in this expansion + //[drop_tracking]~| NOTE: in this expansion + //[drop_tracking]~| NOTE: in this expansion async move { - baz(|| async{ + //[drop_tracking]~^ within this async block + baz(|| async{ //[drop_tracking]~ NOTE: used within this closure foo(tx.clone()); }).await; } diff --git a/src/test/ui/async-await/partial-drop-partial-reinit.rs b/src/test/ui/async-await/partial-drop-partial-reinit.rs index 73f0ca8153cb9..4fcfacea3f886 100644 --- a/src/test/ui/async-await/partial-drop-partial-reinit.rs +++ b/src/test/ui/async-await/partial-drop-partial-reinit.rs @@ -5,9 +5,15 @@ fn main() { gimme_send(foo()); //~^ ERROR cannot be sent between threads safely + //~| NOTE cannot be sent + //~| NOTE bound introduced by + //~| NOTE appears within the type + //~| NOTE captures the following types } fn gimme_send(t: T) { +//~^ NOTE required by this bound +//~| NOTE required by a bound drop(t); } @@ -20,6 +26,8 @@ impl Drop for NotSend { impl !Send for NotSend {} async fn foo() { +//~^ NOTE used within this async block +//~| NOTE within this `impl Future let mut x = (NotSend {},); drop(x.0); x.0 = NotSend {}; diff --git a/src/test/ui/async-await/partial-drop-partial-reinit.stderr b/src/test/ui/async-await/partial-drop-partial-reinit.stderr index a1c4957e9843a..96d0c71f103fb 100644 --- a/src/test/ui/async-await/partial-drop-partial-reinit.stderr +++ b/src/test/ui/async-await/partial-drop-partial-reinit.stderr @@ -11,13 +11,21 @@ LL | async fn foo() { | = help: within `impl Future`, the trait `Send` is not implemented for `NotSend` = note: required because it appears within the type `(NotSend,)` - = note: required because it appears within the type `{ResumeTy, (NotSend,), impl Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]` - = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]>` - = note: required because it appears within the type `impl Future` - = note: required because it appears within the type `impl Future` + = note: required because it captures the following types: `ResumeTy`, `(NotSend,)`, `impl Future`, `()` +note: required because it's used within this async block + --> $DIR/partial-drop-partial-reinit.rs:28:16 + | +LL | async fn foo() { + | ________________^ +LL | | +LL | | +LL | | let mut x = (NotSend {},); +... | +LL | | bar().await; +LL | | } + | |_^ note: required by a bound in `gimme_send` - --> $DIR/partial-drop-partial-reinit.rs:10:18 + --> $DIR/partial-drop-partial-reinit.rs:14:18 | LL | fn gimme_send(t: T) { | ^^^^ required by this bound in `gimme_send` diff --git a/src/test/ui/closures/closure-move-sync.stderr b/src/test/ui/closures/closure-move-sync.stderr index 618c9a172473c..cbc4e2e52315f 100644 --- a/src/test/ui/closures/closure-move-sync.stderr +++ b/src/test/ui/closures/closure-move-sync.stderr @@ -6,7 +6,15 @@ LL | let t = thread::spawn(|| { | = help: the trait `Sync` is not implemented for `std::sync::mpsc::Receiver<()>` = note: required because of the requirements on the impl of `Send` for `&std::sync::mpsc::Receiver<()>` - = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6]` +note: required because it's used within this closure + --> $DIR/closure-move-sync.rs:6:27 + | +LL | let t = thread::spawn(|| { + | ___________________________^ +LL | | recv.recv().unwrap(); +LL | | +LL | | }); + | |_____^ note: required by a bound in `spawn` --> $SRC_DIR/std/src/thread/mod.rs:LL:COL | @@ -21,7 +29,11 @@ LL | thread::spawn(|| tx.send(()).unwrap()); | = help: the trait `Sync` is not implemented for `Sender<()>` = note: required because of the requirements on the impl of `Send` for `&Sender<()>` - = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:18:19: 18:42]` +note: required because it's used within this closure + --> $DIR/closure-move-sync.rs:18:19 + | +LL | thread::spawn(|| tx.send(()).unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^ note: required by a bound in `spawn` --> $SRC_DIR/std/src/thread/mod.rs:LL:COL | diff --git a/src/test/ui/generator/issue-68112.rs b/src/test/ui/generator/issue-68112.rs index feb07c9bf8802..3fcef773b68ab 100644 --- a/src/test/ui/generator/issue-68112.rs +++ b/src/test/ui/generator/issue-68112.rs @@ -20,6 +20,10 @@ pub fn make_gen1(t: T) -> Ready { } fn require_send(_: impl Send) {} +//~^ NOTE required by a bound +//~| NOTE required by a bound +//~| NOTE required by this bound +//~| NOTE required by this bound fn make_non_send_generator() -> impl Generator>> { make_gen1(Arc::new(RefCell::new(0))) @@ -28,29 +32,39 @@ fn make_non_send_generator() -> impl Generator>> { fn test1() { let send_gen = || { let _non_send_gen = make_non_send_generator(); + //~^ NOTE not `Send` yield; - }; + //~^ NOTE yield occurs here + //~| NOTE value is used across a yield + }; //~ NOTE later dropped here require_send(send_gen); //~^ ERROR generator cannot be sent between threads + //~| NOTE not `Send` } pub fn make_gen2(t: T) -> impl Generator { - || { +//~^ NOTE appears within the type +//~| NOTE expansion of desugaring + || { //~ NOTE used within this generator yield; t } } -fn make_non_send_generator2() -> impl Generator>> { +fn make_non_send_generator2() -> impl Generator>> { //~ NOTE appears within the type +//~^ NOTE expansion of desugaring make_gen2(Arc::new(RefCell::new(0))) } fn test2() { - let send_gen = || { + let send_gen = || { //~ NOTE used within this generator let _non_send_gen = make_non_send_generator2(); yield; }; require_send(send_gen); //~^ ERROR `RefCell` cannot be shared between threads safely + //~| NOTE `RefCell` cannot be shared between threads safely + //~| NOTE requirements on the impl + //~| NOTE captures the following types } fn main() {} diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr index a7d7a73254886..83f068c207643 100644 --- a/src/test/ui/generator/issue-68112.stderr +++ b/src/test/ui/generator/issue-68112.stderr @@ -1,17 +1,19 @@ error: generator cannot be sent between threads safely - --> $DIR/issue-68112.rs:33:5 + --> $DIR/issue-68112.rs:40:5 | LL | require_send(send_gen); | ^^^^^^^^^^^^ generator is not `Send` | = help: the trait `Sync` is not implemented for `RefCell` note: generator is not `Send` as this value is used across a yield - --> $DIR/issue-68112.rs:31:9 + --> $DIR/issue-68112.rs:36:9 | LL | let _non_send_gen = make_non_send_generator(); | ------------- has type `impl Generator>>` which is not `Send` +LL | LL | yield; | ^^^^^ yield occurs here, with `_non_send_gen` maybe used later +... LL | }; | - `_non_send_gen` is later dropped here note: required by a bound in `require_send` @@ -21,18 +23,41 @@ LL | fn require_send(_: impl Send) {} | ^^^^ required by this bound in `require_send` error[E0277]: `RefCell` cannot be shared between threads safely - --> $DIR/issue-68112.rs:52:5 + --> $DIR/issue-68112.rs:63:5 | LL | require_send(send_gen); | ^^^^^^^^^^^^ `RefCell` cannot be shared between threads safely | = help: the trait `Sync` is not implemented for `RefCell` = note: required because of the requirements on the impl of `Send` for `Arc>` - = note: required because it appears within the type `[generator@$DIR/issue-68112.rs:38:5: 41:6]` - = note: required because it appears within the type `impl Generator>>` - = note: required because it appears within the type `impl Generator>>` - = note: required because it appears within the type `{impl Generator>>, ()}` - = note: required because it appears within the type `[generator@$DIR/issue-68112.rs:48:20: 51:6]` +note: required because it's used within this generator + --> $DIR/issue-68112.rs:48:5 + | +LL | / || { +LL | | yield; +LL | | t +LL | | } + | |_____^ +note: required because it appears within the type `impl Generator>>` + --> $DIR/issue-68112.rs:45:30 + | +LL | pub fn make_gen2(t: T) -> impl Generator { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: required because it appears within the type `impl Generator>>` + --> $DIR/issue-68112.rs:53:34 + | +LL | fn make_non_send_generator2() -> impl Generator>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: required because it captures the following types: `impl Generator>>`, `()` +note: required because it's used within this generator + --> $DIR/issue-68112.rs:59:20 + | +LL | let send_gen = || { + | ____________________^ +LL | | let _non_send_gen = make_non_send_generator2(); +LL | | yield; +LL | | }; + | |_____^ note: required by a bound in `require_send` --> $DIR/issue-68112.rs:22:25 | diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr index 4d2faa198f1af..edf9ee628a2bc 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -6,7 +6,16 @@ LL | assert_send(|| { | = help: the trait `Sync` is not implemented for `Cell` = note: required because of the requirements on the impl of `Send` for `&Cell` - = note: required because it appears within the type `[generator@$DIR/not-send-sync.rs:16:17: 20:6]` +note: required because it's used within this generator + --> $DIR/not-send-sync.rs:16:17 + | +LL | assert_send(|| { + | _________________^ +LL | | +LL | | drop(&a); +LL | | yield; +LL | | }); + | |_____^ note: required by a bound in `assert_send` --> $DIR/not-send-sync.rs:7:23 | diff --git a/src/test/ui/generator/print/generator-print-verbose-1.stderr b/src/test/ui/generator/print/generator-print-verbose-1.stderr index 2f7faf520d969..3ee4c1458bad3 100644 --- a/src/test/ui/generator/print/generator-print-verbose-1.stderr +++ b/src/test/ui/generator/print/generator-print-verbose-1.stderr @@ -28,11 +28,34 @@ LL | require_send(send_gen); | = help: the trait `Sync` is not implemented for `RefCell` = note: required because of the requirements on the impl of `Send` for `Arc>` - = note: required because it appears within the type `[make_gen2>>::{closure#0} upvar_tys=(Arc>) {()}]` - = note: required because it appears within the type `Opaque(DefId(0:39 ~ generator_print_verbose_1[749a]::make_gen2::{opaque#0}), [std::sync::Arc>])` - = note: required because it appears within the type `Opaque(DefId(0:42 ~ generator_print_verbose_1[749a]::make_non_send_generator2::{opaque#0}), [])` - = note: required because it appears within the type `{Opaque(DefId(0:42 ~ generator_print_verbose_1[749a]::make_non_send_generator2::{opaque#0}), []), ()}` - = note: required because it appears within the type `[test2::{closure#0} upvar_tys=() {Opaque(DefId(0:42 ~ generator_print_verbose_1[749a]::make_non_send_generator2::{opaque#0}), []), ()}]` +note: required because it's used within this generator + --> $DIR/generator-print-verbose-1.rs:42:5 + | +LL | / || { +LL | | yield; +LL | | t +LL | | } + | |_____^ +note: required because it appears within the type `Opaque(DefId(0:39 ~ generator_print_verbose_1[749a]::make_gen2::{opaque#0}), [std::sync::Arc>])` + --> $DIR/generator-print-verbose-1.rs:41:30 + | +LL | pub fn make_gen2(t: T) -> impl Generator { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: required because it appears within the type `Opaque(DefId(0:42 ~ generator_print_verbose_1[749a]::make_non_send_generator2::{opaque#0}), [])` + --> $DIR/generator-print-verbose-1.rs:47:34 + | +LL | fn make_non_send_generator2() -> impl Generator>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: required because it captures the following types: `Opaque(DefId(0:42 ~ generator_print_verbose_1[749a]::make_non_send_generator2::{opaque#0}), [])`, `()` +note: required because it's used within this generator + --> $DIR/generator-print-verbose-1.rs:52:20 + | +LL | let send_gen = || { + | ____________________^ +LL | | let _non_send_gen = make_non_send_generator2(); +LL | | yield; +LL | | }; + | |_____^ note: required by a bound in `require_send` --> $DIR/generator-print-verbose-1.rs:26:25 | diff --git a/src/test/ui/generator/print/generator-print-verbose-2.stderr b/src/test/ui/generator/print/generator-print-verbose-2.stderr index fb2a5754dd306..1356fa5f15295 100644 --- a/src/test/ui/generator/print/generator-print-verbose-2.stderr +++ b/src/test/ui/generator/print/generator-print-verbose-2.stderr @@ -6,7 +6,16 @@ LL | assert_send(|| { | = help: the trait `Sync` is not implemented for `Cell` = note: required because of the requirements on the impl of `Send` for `&'_#4r Cell` - = note: required because it appears within the type `[main::{closure#1} upvar_tys=(&'_#4r Cell) _#17t]` +note: required because it's used within this generator + --> $DIR/generator-print-verbose-2.rs:19:17 + | +LL | assert_send(|| { + | _________________^ +LL | | +LL | | drop(&a); +LL | | yield; +LL | | }); + | |_____^ note: required by a bound in `assert_send` --> $DIR/generator-print-verbose-2.rs:10:23 | diff --git a/src/test/ui/impl-trait/auto-trait-leak2.rs b/src/test/ui/impl-trait/auto-trait-leak2.rs index a464f576dc783..09450089adaa8 100644 --- a/src/test/ui/impl-trait/auto-trait-leak2.rs +++ b/src/test/ui/impl-trait/auto-trait-leak2.rs @@ -3,23 +3,37 @@ use std::rc::Rc; // Fast path, main can see the concrete type returned. fn before() -> impl Fn(i32) { +//~^ NOTE within this `impl Fn +//~| NOTE within the type `impl Fn +//~| NOTE expansion of desugaring let p = Rc::new(Cell::new(0)); - move |x| p.set(x) + move |x| p.set(x) //~ NOTE used within this closure } fn send(_: T) {} +//~^ NOTE required by a bound +//~| NOTE required by a bound +//~| NOTE required by this bound +//~| NOTE required by this bound fn main() { send(before()); //~^ ERROR `Rc>` cannot be sent between threads safely + //~| NOTE `Rc>` cannot be sent between threads safely + //~| NOTE required by a bound send(after()); //~^ ERROR `Rc>` cannot be sent between threads safely + //~| NOTE `Rc>` cannot be sent between threads safely + //~| NOTE required by a bound } // Deferred path, main has to wait until typeck finishes, // to check if the return type of after is Send. fn after() -> impl Fn(i32) { +//~^ NOTE within this `impl Fn(i32)` +//~| NOTE in this expansion +//~| NOTE appears within the type let p = Rc::new(Cell::new(0)); - move |x| p.set(x) + move |x| p.set(x) //~ NOTE used within this closure } diff --git a/src/test/ui/impl-trait/auto-trait-leak2.stderr b/src/test/ui/impl-trait/auto-trait-leak2.stderr index 37ae3c6802964..d825843492d49 100644 --- a/src/test/ui/impl-trait/auto-trait-leak2.stderr +++ b/src/test/ui/impl-trait/auto-trait-leak2.stderr @@ -1,5 +1,5 @@ error[E0277]: `Rc>` cannot be sent between threads safely - --> $DIR/auto-trait-leak2.rs:13:10 + --> $DIR/auto-trait-leak2.rs:20:10 | LL | fn before() -> impl Fn(i32) { | ------------ within this `impl Fn(i32)` @@ -10,16 +10,24 @@ LL | send(before()); | required by a bound introduced by this call | = help: within `impl Fn(i32)`, the trait `Send` is not implemented for `Rc>` - = note: required because it appears within the type `[closure@$DIR/auto-trait-leak2.rs:7:5: 7:22]` - = note: required because it appears within the type `impl Fn(i32)` +note: required because it's used within this closure + --> $DIR/auto-trait-leak2.rs:10:5 + | +LL | move |x| p.set(x) + | ^^^^^^^^^^^^^^^^^ +note: required because it appears within the type `impl Fn(i32)` + --> $DIR/auto-trait-leak2.rs:5:16 + | +LL | fn before() -> impl Fn(i32) { + | ^^^^^^^^^^^^ note: required by a bound in `send` - --> $DIR/auto-trait-leak2.rs:10:12 + --> $DIR/auto-trait-leak2.rs:13:12 | LL | fn send(_: T) {} | ^^^^ required by this bound in `send` error[E0277]: `Rc>` cannot be sent between threads safely - --> $DIR/auto-trait-leak2.rs:16:10 + --> $DIR/auto-trait-leak2.rs:25:10 | LL | send(after()); | ---- ^^^^^^^ `Rc>` cannot be sent between threads safely @@ -30,10 +38,18 @@ LL | fn after() -> impl Fn(i32) { | ------------ within this `impl Fn(i32)` | = help: within `impl Fn(i32)`, the trait `Send` is not implemented for `Rc>` - = note: required because it appears within the type `[closure@$DIR/auto-trait-leak2.rs:24:5: 24:22]` - = note: required because it appears within the type `impl Fn(i32)` +note: required because it's used within this closure + --> $DIR/auto-trait-leak2.rs:38:5 + | +LL | move |x| p.set(x) + | ^^^^^^^^^^^^^^^^^ +note: required because it appears within the type `impl Fn(i32)` + --> $DIR/auto-trait-leak2.rs:33:15 + | +LL | fn after() -> impl Fn(i32) { + | ^^^^^^^^^^^^ note: required by a bound in `send` - --> $DIR/auto-trait-leak2.rs:10:12 + --> $DIR/auto-trait-leak2.rs:13:12 | LL | fn send(_: T) {} | ^^^^ required by this bound in `send` diff --git a/src/test/ui/interior-mutability/interior-mutability.stderr b/src/test/ui/interior-mutability/interior-mutability.stderr index cb22d5adb717d..66fe3c74e2c87 100644 --- a/src/test/ui/interior-mutability/interior-mutability.stderr +++ b/src/test/ui/interior-mutability/interior-mutability.stderr @@ -7,7 +7,11 @@ LL | catch_unwind(|| { x.set(23); }); = help: within `Cell`, the trait `RefUnwindSafe` is not implemented for `UnsafeCell` = note: required because it appears within the type `Cell` = note: required because of the requirements on the impl of `UnwindSafe` for `&Cell` - = note: required because it appears within the type `[closure@$DIR/interior-mutability.rs:5:18: 5:35]` +note: required because it's used within this closure + --> $DIR/interior-mutability.rs:5:18 + | +LL | catch_unwind(|| { x.set(23); }); + | ^^^^^^^^^^^^^^^^^ note: required by a bound in `catch_unwind` --> $SRC_DIR/std/src/panic.rs:LL:COL | diff --git a/src/test/ui/kindck/kindck-nonsendable-1.stderr b/src/test/ui/kindck/kindck-nonsendable-1.stderr index b3ebe7f5c7dc3..727573a0be4ed 100644 --- a/src/test/ui/kindck/kindck-nonsendable-1.stderr +++ b/src/test/ui/kindck/kindck-nonsendable-1.stderr @@ -7,7 +7,11 @@ LL | bar(move|| foo(x)); | `Rc` cannot be sent between threads safely | = help: within `[closure@$DIR/kindck-nonsendable-1.rs:9:9: 9:22]`, the trait `Send` is not implemented for `Rc` - = note: required because it appears within the type `[closure@$DIR/kindck-nonsendable-1.rs:9:9: 9:22]` +note: required because it's used within this closure + --> $DIR/kindck-nonsendable-1.rs:9:9 + | +LL | bar(move|| foo(x)); + | ^^^^^^^^^^^^^ note: required by a bound in `bar` --> $DIR/kindck-nonsendable-1.rs:5:21 | diff --git a/src/test/ui/no-send-res-ports.stderr b/src/test/ui/no-send-res-ports.stderr index 80708c989fcf7..e4c57c04e7269 100644 --- a/src/test/ui/no-send-res-ports.stderr +++ b/src/test/ui/no-send-res-ports.stderr @@ -22,7 +22,16 @@ note: required because it appears within the type `Foo` | LL | struct Foo { | ^^^ - = note: required because it appears within the type `[closure@$DIR/no-send-res-ports.rs:25:19: 29:6]` +note: required because it's used within this closure + --> $DIR/no-send-res-ports.rs:25:19 + | +LL | thread::spawn(move|| { + | ___________________^ +LL | | +LL | | let y = x; +LL | | println!("{:?}", y); +LL | | }); + | |_____^ note: required by a bound in `spawn` --> $SRC_DIR/std/src/thread/mod.rs:LL:COL | diff --git a/src/test/ui/not-clone-closure.stderr b/src/test/ui/not-clone-closure.stderr index 78b35569c2162..bebf561b120b5 100644 --- a/src/test/ui/not-clone-closure.stderr +++ b/src/test/ui/not-clone-closure.stderr @@ -10,7 +10,14 @@ LL | LL | let hello = hello.clone(); | ^^^^^ within `[closure@$DIR/not-clone-closure.rs:7:17: 9:6]`, the trait `Clone` is not implemented for `S` | - = note: required because it appears within the type `[closure@$DIR/not-clone-closure.rs:7:17: 9:6]` +note: required because it's used within this closure + --> $DIR/not-clone-closure.rs:7:17 + | +LL | let hello = move || { + | _________________^ +LL | | println!("Hello {}", a.0); +LL | | }; + | |_____^ help: consider annotating `S` with `#[derive(Clone)]` | LL | #[derive(Clone)] diff --git a/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.rs b/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.rs index 745379efa6df9..fc89b0e870e39 100644 --- a/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.rs +++ b/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.rs @@ -4,7 +4,9 @@ mod m { use std::rc::Rc; - type Foo = impl std::fmt::Debug; + type Foo = impl std::fmt::Debug; //~ NOTE appears within the type + //~^ within this `Foo` + //~| expansion of desugaring pub fn foo() -> Foo { Rc::new(22_u32) @@ -12,8 +14,12 @@ mod m { } fn is_send(_: T) {} +//~^ required by this bound +//~| required by a bound fn main() { is_send(m::foo()); //~^ ERROR: `Rc` cannot be sent between threads safely [E0277] + //~| NOTE cannot be sent + //~| NOTE required by a bound } diff --git a/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.stderr b/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.stderr index 0664275b2ad0a..d7247302dd1e0 100644 --- a/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.stderr +++ b/src/test/ui/type-alias-impl-trait/auto-trait-leakage2.stderr @@ -1,5 +1,5 @@ error[E0277]: `Rc` cannot be sent between threads safely - --> $DIR/auto-trait-leakage2.rs:17:13 + --> $DIR/auto-trait-leakage2.rs:21:13 | LL | type Foo = impl std::fmt::Debug; | -------------------- within this `Foo` @@ -10,9 +10,13 @@ LL | is_send(m::foo()); | required by a bound introduced by this call | = help: within `Foo`, the trait `Send` is not implemented for `Rc` - = note: required because it appears within the type `Foo` +note: required because it appears within the type `Foo` + --> $DIR/auto-trait-leakage2.rs:7:16 + | +LL | type Foo = impl std::fmt::Debug; + | ^^^^^^^^^^^^^^^^^^^^ note: required by a bound in `is_send` - --> $DIR/auto-trait-leakage2.rs:14:15 + --> $DIR/auto-trait-leakage2.rs:16:15 | LL | fn is_send(_: T) {} | ^^^^ required by this bound in `is_send` From b052d76586988040c6ae8aef609794ae16cc28dc Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 21 Jun 2022 19:43:46 -0500 Subject: [PATCH 2/2] Address review comments from #98259 It got merged so fast I didn't have time to make changes xD --- compiler/rustc_middle/src/ty/context.rs | 16 +++++++++++- .../src/traits/error_reporting/suggestions.rs | 26 +++---------------- src/test/ui/async-await/issue-68112.stderr | 4 +-- ...e-70935-complex-spans.drop_tracking.stderr | 4 +-- .../async-await/issue-70935-complex-spans.rs | 4 +-- .../partial-drop-partial-reinit.rs | 2 +- .../partial-drop-partial-reinit.stderr | 2 +- 7 files changed, 27 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index c43cf07b3ad0a..0765928e9b95c 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -74,7 +74,7 @@ use std::mem; use std::ops::{Bound, Deref}; use std::sync::Arc; -use super::RvalueScopes; +use super::{ImplPolarity, RvalueScopes}; pub trait OnDiskCache<'tcx>: rustc_data_structures::sync::Sync { /// Creates a new `OnDiskCache` instance from the serialized data in `data`. @@ -2224,6 +2224,20 @@ impl<'tcx> TyCtxt<'tcx> { }) } + /// Given a `ty`, return whether it's an `impl Future<...>`. + pub fn ty_is_opaque_future(self, ty: Ty<'_>) -> bool { + let ty::Opaque(def_id, _) = ty.kind() else { return false }; + let future_trait = self.lang_items().future_trait().unwrap(); + + self.explicit_item_bounds(def_id).iter().any(|(predicate, _)| { + let ty::PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder() else { + return false; + }; + trait_predicate.trait_ref.def_id == future_trait + && trait_predicate.polarity == ImplPolarity::Positive + }) + } + /// Computes the def-ids of the transitive supertraits of `trait_def_id`. This (intentionally) /// does not compute the full elaborated super-predicates but just the set of def-ids. It is used /// to identify which traits may define a given associated type to help avoid cycle errors. diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 159fcf932a1da..19318a85a0125 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -8,7 +8,6 @@ use crate::infer::InferCtxt; use crate::traits::normalize_to; use hir::HirId; -use rustc_ast::Movability; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{ @@ -2406,19 +2405,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } }; - let future_trait = self.tcx.lang_items().future_trait().unwrap(); - let opaque_ty_is_future = |def_id| { - self.tcx.explicit_item_bounds(def_id).iter().any(|(predicate, _)| { - if let ty::PredicateKind::Trait(trait_predicate) = - predicate.kind().skip_binder() - { - trait_predicate.trait_ref.def_id == future_trait - } else { - false - } - }) - }; - let from_generator = tcx.lang_items().from_generator_fn().unwrap(); // Don't print the tuple of capture types @@ -2444,13 +2430,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // If the previous type is `from_generator`, this is the future generated by the body of an async function. // Avoid printing it twice (it was already printed in the `ty::Generator` arm below). - let is_future = opaque_ty_is_future(def_id); + let is_future = tcx.ty_is_opaque_future(ty); debug!( ?obligated_types, ?is_future, "note_obligation_cause_code: check for async fn" ); - if opaque_ty_is_future(def_id) + if is_future && obligated_types.last().map_or(false, |ty| match ty.kind() { ty::Opaque(last_def_id, _) => { tcx.parent(*last_def_id) == from_generator @@ -2475,15 +2461,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } err.note(msg.trim_end_matches(", ")) } - ty::Generator(def_id, _, movability) => { + ty::Generator(def_id, _, _) => { let sp = self.tcx.def_span(def_id); // Special-case this to say "async block" instead of `[static generator]`. - let kind = if *movability == Movability::Static { - "async block" - } else { - "generator" - }; + let kind = tcx.generator_kind(def_id).unwrap(); err.span_note( sp, &format!("required because it's used within this {}", kind), diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index b8d3e1540d8a9..4285fbbeceb60 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -42,7 +42,7 @@ LL | require_send(send_fut); | = help: the trait `Sync` is not implemented for `RefCell` = note: required because of the requirements on the impl of `Send` for `Arc>` -note: required because it's used within this async block +note: required because it's used within this `async fn` body --> $DIR/issue-68112.rs:47:31 | LL | async fn ready2(t: T) -> T { t } @@ -53,7 +53,7 @@ note: required because it appears within the type `impl Future impl Future>> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: required because it captures the following types: `ResumeTy`, `impl Future>>`, `()`, `i32`, `Ready` -note: required because it's used within this async block +note: required because it's used within this `async` block --> $DIR/issue-68112.rs:55:26 | LL | let send_fut = async { diff --git a/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr b/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr index 19fd5eb7c73fb..43b7cb8cece36 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr +++ b/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr @@ -14,7 +14,7 @@ LL | baz(|| async{ LL | | foo(tx.clone()); LL | | }).await; | |_________^ -note: required because it's used within this async block +note: required because it's used within this `async fn` body --> $DIR/issue-70935-complex-spans.rs:9:67 | LL | async fn baz(_c: impl FnMut() -> T) where T: Future { @@ -23,7 +23,7 @@ LL | | LL | | } | |_^ = note: required because it captures the following types: `ResumeTy`, `impl Future`, `()` -note: required because it's used within this async block +note: required because it's used within this `async` block --> $DIR/issue-70935-complex-spans.rs:23:16 | LL | async move { diff --git a/src/test/ui/async-await/issue-70935-complex-spans.rs b/src/test/ui/async-await/issue-70935-complex-spans.rs index 4bf94fe342c1d..f45ce1f25efa0 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.rs +++ b/src/test/ui/async-await/issue-70935-complex-spans.rs @@ -7,7 +7,7 @@ use std::future::Future; async fn baz(_c: impl FnMut() -> T) where T: Future { -//[drop_tracking]~^ within this async block +//[drop_tracking]~^ within this `async fn` body } fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { @@ -21,7 +21,7 @@ fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { //[drop_tracking]~| NOTE: in this expansion //[drop_tracking]~| NOTE: in this expansion async move { - //[drop_tracking]~^ within this async block + //[drop_tracking]~^ within this `async` block baz(|| async{ //[drop_tracking]~ NOTE: used within this closure foo(tx.clone()); }).await; diff --git a/src/test/ui/async-await/partial-drop-partial-reinit.rs b/src/test/ui/async-await/partial-drop-partial-reinit.rs index 4fcfacea3f886..fe0fce7afd9f9 100644 --- a/src/test/ui/async-await/partial-drop-partial-reinit.rs +++ b/src/test/ui/async-await/partial-drop-partial-reinit.rs @@ -26,7 +26,7 @@ impl Drop for NotSend { impl !Send for NotSend {} async fn foo() { -//~^ NOTE used within this async block +//~^ NOTE used within this `async fn` body //~| NOTE within this `impl Future let mut x = (NotSend {},); drop(x.0); diff --git a/src/test/ui/async-await/partial-drop-partial-reinit.stderr b/src/test/ui/async-await/partial-drop-partial-reinit.stderr index 96d0c71f103fb..05f5358340a98 100644 --- a/src/test/ui/async-await/partial-drop-partial-reinit.stderr +++ b/src/test/ui/async-await/partial-drop-partial-reinit.stderr @@ -12,7 +12,7 @@ LL | async fn foo() { = help: within `impl Future`, the trait `Send` is not implemented for `NotSend` = note: required because it appears within the type `(NotSend,)` = note: required because it captures the following types: `ResumeTy`, `(NotSend,)`, `impl Future`, `()` -note: required because it's used within this async block +note: required because it's used within this `async fn` body --> $DIR/partial-drop-partial-reinit.rs:28:16 | LL | async fn foo() {