Incorporate feedback into diagnostics

tmandry · tmandry · commit df64c5d260d3 · 2020-04-13T18:58:17.000-07:00
diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs
@@ -1318,7 +1318,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 _ => false,
             })
             .unwrap_or(false);
-        let await_or_yield = if is_async { "await" } else { "yield" };
+        let (await_or_yield, an_await_or_yield) =
+            if is_async { ("await", "an await") } else { ("yield", "a yield") };
         let future_or_generator = if is_async { "future" } else { "generator" };
 
         // Special case the primary error message when send or sync is the trait that was
@@ -1364,38 +1365,26 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             span.push_span_label(original_span, message);
             err.set_span(span);
 
-            format!("{} is not {}", future_or_generator, trait_name)
+            format!("is not {}", trait_name)
         } else {
-            format!(
-                "{} does not implement `{}`",
-                future_or_generator,
-                trait_ref.print_only_trait_path()
-            )
-        };
-
-        let push_target_span_with_fallback = |span: &mut MultiSpan, fallback: &str| {
-            if target_ty.is_impl_trait() {
-                // It's not very useful to tell the user the type if it's opaque.
-                span.push_span_label(target_span, fallback.to_string());
-            } else {
-                span.push_span_label(target_span, format!("has type `{}`", target_ty));
-            }
+            format!("does not implement `{}`", trait_ref.print_only_trait_path())
         };
 
         if let Some(await_span) = from_awaited_ty {
             // The type causing this obligation is one being awaited at await_span.
             let mut span = MultiSpan::from_span(await_span);
 
-            if target_span == await_span {
-                push_target_span_with_fallback(&mut span, "await occurs here");
-            } else {
-                span.push_span_label(await_span, "await occurs here".to_string());
-                push_target_span_with_fallback(&mut span, "created here");
-            }
+            span.push_span_label(
+                await_span,
+                format!("await occurs here on type `{}`, which {}", target_ty, trait_explanation),
+            );
 
             err.span_note(
                 span,
-                &format!("{} as this value is used in an await", trait_explanation),
+                &format!(
+                    "future {not_trait} as it awaits another future which {not_trait}",
+                    not_trait = trait_explanation
+                ),
             );
         } else {
             // Look at the last interior type to get a span for the `.await`.
@@ -1410,7 +1399,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet),
             );
 
-            push_target_span_with_fallback(&mut span, "created here");
+            span.push_span_label(
+                target_span,
+                format!("has type `{}` which {}", target_ty, trait_explanation),
+            );
 
             // If available, use the scope span to annotate the drop location.
             if let Some(scope_span) = scope_span {
@@ -1423,8 +1415,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             err.span_note(
                 span,
                 &format!(
-                    "{} as this value is used across an {}",
-                    trait_explanation, await_or_yield
+                    "{} {} as this value is used across {}",
+                    future_or_generator, trait_explanation, an_await_or_yield
                 ),
             );
         }
diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr
@@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/async-fn-nonsend.rs:24:5
    |
 LL |     let x = non_send();
-   |         - created here
+   |         - has type `impl std::fmt::Debug` which is not `Send`
 LL |     drop(x);
 LL |     fut().await;
    |     ^^^^^^^^^^^ await occurs here, with `x` maybe used later
@@ -33,7 +33,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/async-fn-nonsend.rs:33:20
    |
 LL |     match Some(non_send()) {
-   |                ---------- created here
+   |                ---------- has type `impl std::fmt::Debug` which is not `Send`
 LL |         Some(_) => fut().await,
    |                    ^^^^^^^^^^^ await occurs here, with `non_send()` maybe used later
 ...
@@ -54,7 +54,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/async-fn-nonsend.rs:42:9
    |
 LL |     let f: &mut std::fmt::Formatter = panic!();
-   |         - has type `&mut std::fmt::Formatter<'_>`
+   |         - has type `&mut std::fmt::Formatter<'_>` which is not `Send`
 LL |     if non_sync().fmt(f).unwrap() == () {
 LL |         fut().await;
    |         ^^^^^^^^^^^ await occurs here, with `f` maybe used later
diff --git a/src/test/ui/async-await/issue-64130-1-sync.stderr b/src/test/ui/async-await/issue-64130-1-sync.stderr
@@ -12,7 +12,7 @@ note: future is not `Sync` as this value is used across an await
   --> $DIR/issue-64130-1-sync.rs:15:5
    |
 LL |     let x = Foo;
-   |         - has type `Foo`
+   |         - has type `Foo` which is not `Sync`
 LL |     baz().await;
    |     ^^^^^^^^^^^ await occurs here, with `x` maybe used later
 LL | }
diff --git a/src/test/ui/async-await/issue-64130-2-send.stderr b/src/test/ui/async-await/issue-64130-2-send.stderr
@@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/issue-64130-2-send.rs:15:5
    |
 LL |     let x = Foo;
-   |         - has type `Foo`
+   |         - has type `Foo` which is not `Send`
 LL |     baz().await;
    |     ^^^^^^^^^^^ await occurs here, with `x` maybe used later
 LL | }
diff --git a/src/test/ui/async-await/issue-64130-3-other.stderr b/src/test/ui/async-await/issue-64130-3-other.stderr
@@ -16,7 +16,7 @@ note: future does not implement `Qux` as this value is used across an await
   --> $DIR/issue-64130-3-other.rs:18:5
    |
 LL |     let x = Foo;
-   |         - has type `Foo`
+   |         - has type `Foo` which does not implement `Qux`
 LL |     baz().await;
    |     ^^^^^^^^^^^ await occurs here, with `x` maybe used later
 LL | }
diff --git a/src/test/ui/async-await/issue-64130-4-async-move.stderr b/src/test/ui/async-await/issue-64130-4-async-move.stderr
@@ -18,7 +18,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/issue-64130-4-async-move.rs:21:26
    |
 LL |         match client.status() {
-   |               ------ has type `&Client`
+   |               ------ has type `&Client` which is not `Send`
 LL |             200 => {
 LL |                 let _x = get().await;
    |                          ^^^^^^^^^^^ await occurs here, with `client` maybe used later
diff --git a/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr b/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr
@@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/issue-64130-non-send-future-diags.rs:15:5
    |
 LL |     let g = x.lock().unwrap();
-   |         - has type `std::sync::MutexGuard<'_, u32>`
+   |         - has type `std::sync::MutexGuard<'_, u32>` which is not `Send`
 LL |     baz().await;
    |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
 LL | }
diff --git a/src/test/ui/async-await/issue-67252-unnamed-future.stderr b/src/test/ui/async-await/issue-67252-unnamed-future.stderr
@@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
   --> $DIR/issue-67252-unnamed-future.rs:20:9
    |
 LL |         let _a = std::ptr::null_mut::<()>(); // `*mut ()` is not `Send`
-   |             -- has type `*mut ()`
+   |             -- has type `*mut ()` which is not `Send`
 LL |         AFuture.await;
    |         ^^^^^^^^^^^^^ await occurs here, with `_a` maybe used later
 LL |     });
diff --git a/src/test/ui/async-await/issue-68112.rs b/src/test/ui/async-await/issue-68112.rs
@@ -49,6 +49,8 @@ fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {
     ready2(Arc::new(RefCell::new(0)))
 }
 
+// Ideally this test would have diagnostics similar to the test above, but right
+// now it doesn't.
 fn test2() {
     let send_fut = async {
         let non_send_fut = make_non_send_future2();
diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr
@@ -8,13 +8,11 @@ LL |     require_send(send_fut);
    |     ^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
-note: future is not `Send` as this value is used in an await
+note: future is not `Send` as it awaits another future which is not `Send`
   --> $DIR/issue-68112.rs:31:17
    |
-LL |         let non_send_fut = make_non_send_future1();
-   |             ------------ created here
 LL |         let _ = non_send_fut.await;
-   |                 ^^^^^^^^^^^^ await occurs here
+   |                 ^^^^^^^^^^^^ await occurs here on type `impl std::future::Future`, which is not `Send`
 
 error: future cannot be sent between threads safely
   --> $DIR/issue-68112.rs:43:5
@@ -26,14 +24,14 @@ LL |     require_send(send_fut);
    |     ^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
-note: future is not `Send` as this value is used in an await
+note: future is not `Send` as it awaits another future which is not `Send`
   --> $DIR/issue-68112.rs:40:17
    |
 LL |         let _ = make_non_send_future1().await;
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^ await occurs here
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^ await occurs here on type `impl std::future::Future`, which is not `Send`
 
 error[E0277]: `std::cell::RefCell<i32>` cannot be shared between threads safely
-  --> $DIR/issue-68112.rs:58:5
+  --> $DIR/issue-68112.rs:60:5
    |
 LL | fn require_send(_: impl Send) {}
    |    ------------         ---- required by this bound in `require_send`
@@ -49,8 +47,8 @@ LL |     require_send(send_fut);
    = note: required because it appears within the type `impl std::future::Future`
    = note: required because it appears within the type `impl std::future::Future`
    = note: required because it appears within the type `{std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}`
-   = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:53:26: 57:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}]`
-   = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:53:26: 57:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}]>`
+   = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:55:26: 59:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}]`
+   = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:55:26: 59:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}]>`
    = note: required because it appears within the type `impl std::future::Future`
 
 error: aborting due to 3 previous errors
diff --git a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr
@@ -14,7 +14,7 @@ note: future is not `Send` as this value is used across an await
 LL |         bar(Foo(std::ptr::null())).await;
    |         ^^^^^^^^----------------^^^^^^^^- `std::ptr::null()` is later dropped here
    |         |       |
-   |         |       has type `*const u8`
+   |         |       has type `*const u8` which is not `Send`
    |         await occurs here, with `std::ptr::null()` maybe used later
 help: consider moving this into a `let` binding to create a shorter lived borrow
   --> $DIR/issue-65436-raw-ptr-not-send.rs:14:13
diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr
@@ -8,11 +8,11 @@ LL |     require_send(send_gen);
    |     ^^^^^^^^^^^^ generator is not `Send`
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
-note: generator is not `Send` as this value is used across an yield
+note: generator is not `Send` as this value is used across a yield
   --> $DIR/issue-68112.rs:31:9
    |
 LL |         let _non_send_gen = make_non_send_generator();
-   |             ------------- created here
+   |             ------------- has type `impl std::ops::Generator` which is not `Send`
 LL |         yield;
    |         ^^^^^ yield occurs here, with `_non_send_gen` maybe used later
 LL |     };
diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr
@@ -21,11 +21,11 @@ LL |     assert_sync(|| {
    |     ^^^^^^^^^^^ generator is not `Sync`
    |
    = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell<i32>, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<i32>`
-note: generator is not `Sync` as this value is used across an yield
+note: generator is not `Sync` as this value is used across a yield
   --> $DIR/not-send-sync.rs:12:9
    |
 LL |         let a = Cell::new(2);
-   |             - has type `std::cell::Cell<i32>`
+   |             - has type `std::cell::Cell<i32>` which is not `Sync`
 LL |         yield;
    |         ^^^^^ yield occurs here, with `a` maybe used later
 LL |     });

Original file line number	Diff line number	Diff line change
@@ -12,7 +12,7 @@ note: future is not `Sync` as this value is used across an await
`12`	`12`	`--> $DIR/issue-64130-1-sync.rs:15:5`
`13`	`13`	`\|`
`14`	`14`	`LL \| let x = Foo;`
`15`		- \| - has type `Foo`
	`15`	+ \| - has type `Foo` which is not `Sync`
`16`	`16`	`LL \| baz().await;`
`17`	`17`	\| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
`18`	`18`	`LL \| }`
Original file line number	Diff line number	Diff line change
@@ -16,7 +16,7 @@ note: future does not implement `Qux` as this value is used across an await
`16`	`16`	`--> $DIR/issue-64130-3-other.rs:18:5`
`17`	`17`	`\|`
`18`	`18`	`LL \| let x = Foo;`
`19`		- \| - has type `Foo`
	`19`	+ \| - has type `Foo` which does not implement `Qux`
`20`	`20`	`LL \| baz().await;`
`21`	`21`	\| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
`22`	`22`	`LL \| }`
Original file line number	Diff line number	Diff line change
@@ -18,7 +18,7 @@ note: future is not `Send` as this value is used across an await
`18`	`18`	`--> $DIR/issue-64130-4-async-move.rs:21:26`
`19`	`19`	`\|`
`20`	`20`	`LL \| match client.status() {`
`21`		- \| ------ has type `&Client`
	`21`	+ \| ------ has type `&Client` which is not `Send`
`22`	`22`	`LL \| 200 => {`
`23`	`23`	`LL \| let _x = get().await;`
`24`	`24`	\| ^^^^^^^^^^^ await occurs here, with `client` maybe used later
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,8 @@ fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {`
`49`	`49`	`ready2(Arc::new(RefCell::new(0)))`
`50`	`50`	`}`
`51`	`51`
	`52`	`+// Ideally this test would have diagnostics similar to the test above, but right`
	`53`	`+// now it doesn't.`
`52`	`54`	`fn test2() {`
`53`	`55`	`let send_fut = async {`
`54`	`56`	`let non_send_fut = make_non_send_future2();`
Original file line number	Diff line number	Diff line change
`@@ -8,11 +8,11 @@ LL \| require_send(send_gen);`
`8`	`8`	\| ^^^^^^^^^^^^ generator is not `Send`
`9`	`9`	`\|`
`10`	`10`	= help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
`11`		-note: generator is not `Send` as this value is used across an yield
	`11`	+note: generator is not `Send` as this value is used across a yield
`12`	`12`	`--> $DIR/issue-68112.rs:31:9`
`13`	`13`	`\|`
`14`	`14`	`LL \| let _non_send_gen = make_non_send_generator();`
`15`		`- \| ------------- created here`
	`15`	+ \| ------------- has type `impl std::ops::Generator` which is not `Send`
`16`	`16`	`LL \| yield;`
`17`	`17`	\| ^^^^^ yield occurs here, with `_non_send_gen` maybe used later
`18`	`18`	`LL \| };`
Original file line number	Diff line number	Diff line change
`@@ -21,11 +21,11 @@ LL \| assert_sync(\|\| {`
`21`	`21`	\| ^^^^^^^^^^^ generator is not `Sync`
`22`	`22`	`\|`
`23`	`23`	= help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell<i32>, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<i32>`
`24`		-note: generator is not `Sync` as this value is used across an yield
	`24`	+note: generator is not `Sync` as this value is used across a yield
`25`	`25`	`--> $DIR/not-send-sync.rs:12:9`
`26`	`26`	`\|`
`27`	`27`	`LL \| let a = Cell::new(2);`
`28`		- \| - has type `std::cell::Cell<i32>`
	`28`	+ \| - has type `std::cell::Cell<i32>` which is not `Sync`
`29`	`29`	`LL \| yield;`
`30`	`30`	\| ^^^^^ yield occurs here, with `a` maybe used later
`31`	`31`	`LL \| });`