Skip to content

Commit 1deca04

Browse files
jyn514jyn514
committed
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.
1 parent cdcc53b commit 1deca04

21 files changed

+389
-72
lines changed

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs‎

Lines changed: 93 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::infer::InferCtxt;
88
use crate::traits::normalize_to;
99

1010
use hir::HirId;
11+
use rustc_ast::Movability;
1112
use rustc_data_structures::fx::FxHashSet;
1213
use rustc_data_structures::stack::ensure_sufficient_stack;
1314
use rustc_errors::{
@@ -2397,24 +2398,104 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
23972398
{
23982399
let parent_trait_ref =
23992400
self.resolve_vars_if_possible(data.parent_trait_pred);
2400-
let ty = parent_trait_ref.skip_binder().self_ty();
2401-
matches!(ty.kind(), ty::Generator(..))
2402-
|| matches!(ty.kind(), ty::Closure(..))
2401+
let nested_ty = parent_trait_ref.skip_binder().self_ty();
2402+
matches!(nested_ty.kind(), ty::Generator(..))
2403+
|| matches!(nested_ty.kind(), ty::Closure(..))
24032404
} else {
24042405
false
24052406
}
24062407
};
24072408

2409+
let future_trait = self.tcx.lang_items().future_trait().unwrap();
2410+
let opaque_ty_is_future = |def_id| {
2411+
self.tcx.explicit_item_bounds(def_id).iter().any(|(predicate, _)| {
2412+
if let ty::PredicateKind::Trait(trait_predicate) =
2413+
predicate.kind().skip_binder()
2414+
{
2415+
trait_predicate.trait_ref.def_id == future_trait
2416+
} else {
2417+
false
2418+
}
2419+
})
2420+
};
2421+
2422+
let from_generator = tcx.lang_items().from_generator_fn().unwrap();
2423+
24082424
// Don't print the tuple of capture types
2409-
if !is_upvar_tys_infer_tuple {
2410-
let msg = format!("required because it appears within the type `{}`", ty);
2411-
match ty.kind() {
2412-
ty::Adt(def, _) => match self.tcx.opt_item_ident(def.did()) {
2413-
Some(ident) => err.span_note(ident.span, &msg),
2414-
None => err.note(&msg),
2415-
},
2416-
_ => err.note(&msg),
2417-
};
2425+
'print: {
2426+
if !is_upvar_tys_infer_tuple {
2427+
let msg = format!("required because it appears within the type `{}`", ty);
2428+
match ty.kind() {
2429+
ty::Adt(def, _) => {
2430+
// `gen_future` is used in all async functions; it doesn't add any additional info.
2431+
if self.tcx.is_diagnostic_item(sym::gen_future, def.did()) {
2432+
break 'print;
2433+
}
2434+
match self.tcx.opt_item_ident(def.did()) {
2435+
Some(ident) => err.span_note(ident.span, &msg),
2436+
None => err.note(&msg),
2437+
}
2438+
}
2439+
ty::Opaque(def_id, _) => {
2440+
// Avoid printing the future from `core::future::from_generator`, it's not helpful
2441+
if tcx.parent(*def_id) == from_generator {
2442+
break 'print;
2443+
}
2444+
2445+
// If the previous type is `from_generator`, this is the future generated by the body of an async function.
2446+
// Avoid printing it twice (it was already printed in the `ty::Generator` arm below).
2447+
let is_future = opaque_ty_is_future(def_id);
2448+
debug!(
2449+
?obligated_types,
2450+
?is_future,
2451+
"note_obligation_cause_code: check for async fn"
2452+
);
2453+
if opaque_ty_is_future(def_id)
2454+
&& obligated_types.last().map_or(false, |ty| match ty.kind() {
2455+
ty::Opaque(last_def_id, _) => {
2456+
tcx.parent(*last_def_id) == from_generator
2457+
}
2458+
_ => false,
2459+
})
2460+
{
2461+
break 'print;
2462+
}
2463+
err.span_note(self.tcx.def_span(def_id), &msg)
2464+
}
2465+
ty::GeneratorWitness(bound_tys) => {
2466+
use std::fmt::Write;
2467+
2468+
// FIXME: this is kind of an unusual format for rustc, can we make it more clear?
2469+
// Maybe we should just remove this note altogether?
2470+
// FIXME: only print types which don't meet the trait requirement
2471+
let mut msg =
2472+
"required because it captures the following types: ".to_owned();
2473+
for ty in bound_tys.skip_binder() {
2474+
write!(msg, "`{}`, ", ty).unwrap();
2475+
}
2476+
err.note(msg.trim_end_matches(", "))
2477+
}
2478+
ty::Generator(def_id, _, movability) => {
2479+
let sp = self.tcx.def_span(def_id);
2480+
2481+
// Special-case this to say "async block" instead of `[static generator]`.
2482+
let kind = if *movability == Movability::Static {
2483+
"async block"
2484+
} else {
2485+
"generator"
2486+
};
2487+
err.span_note(
2488+
sp,
2489+
&format!("required because it's used within this {}", kind),
2490+
)
2491+
}
2492+
ty::Closure(def_id, _) => err.span_note(
2493+
self.tcx.def_span(def_id),
2494+
&format!("required because it's used within this closure"),
2495+
),
2496+
_ => err.note(&msg),
2497+
};
2498+
}
24182499
}
24192500

24202501
obligated_types.push(ty);

‎src/test/ui/async-await/issue-68112.stderr‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,27 @@ LL | require_send(send_fut);
4242
|
4343
= help: the trait `Sync` is not implemented for `RefCell<i32>`
4444
= note: required because of the requirements on the impl of `Send` for `Arc<RefCell<i32>>`
45-
= note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:47:31: 47:36]`
46-
= note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:47:31: 47:36]>`
47-
= note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
48-
= note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
49-
= note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
50-
= note: required because it appears within the type `{ResumeTy, impl Future<Output = Arc<RefCell<i32>>>, (), i32, Ready<i32>}`
51-
= note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:55:26: 59:6]`
52-
= note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:55:26: 59:6]>`
53-
= note: required because it appears within the type `impl Future<Output = ()>`
45+
note: required because it's used within this async block
46+
--> $DIR/issue-68112.rs:47:31
47+
|
48+
LL | async fn ready2<T>(t: T) -> T { t }
49+
| ^^^^^
50+
note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
51+
--> $DIR/issue-68112.rs:48:31
52+
|
53+
LL | fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {
54+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
55+
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = Arc<RefCell<i32>>>`, `()`, `i32`, `Ready<i32>`
56+
note: required because it's used within this async block
57+
--> $DIR/issue-68112.rs:55:26
58+
|
59+
LL | let send_fut = async {
60+
| __________________________^
61+
LL | | let non_send_fut = make_non_send_future2();
62+
LL | | let _ = non_send_fut.await;
63+
LL | | ready(0).await;
64+
LL | | };
65+
| |_____^
5466
note: required by a bound in `require_send`
5567
--> $DIR/issue-68112.rs:11:25
5668
|

‎src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error[E0277]: `Sender<i32>` cannot be shared between threads safely
2+
--> $DIR/issue-70935-complex-spans.rs:13:45
3+
|
4+
LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
5+
| ^^^^^^^^^^^^^^^^^^ `Sender<i32>` cannot be shared between threads safely
6+
|
7+
= help: the trait `Sync` is not implemented for `Sender<i32>`
8+
= note: required because of the requirements on the impl of `Send` for `&Sender<i32>`
9+
note: required because it's used within this closure
10+
--> $DIR/issue-70935-complex-spans.rs:25:13
11+
|
12+
LL | baz(|| async{
13+
| _____________^
14+
LL | | foo(tx.clone());
15+
LL | | }).await;
16+
| |_________^
17+
note: required because it's used within this async block
18+
--> $DIR/issue-70935-complex-spans.rs:9:67
19+
|
20+
LL | async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
21+
| ___________________________________________________________________^
22+
LL | |
23+
LL | | }
24+
| |_^
25+
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
26+
note: required because it's used within this async block
27+
--> $DIR/issue-70935-complex-spans.rs:23:16
28+
|
29+
LL | async move {
30+
| ________________^
31+
LL | |
32+
LL | | baz(|| async{
33+
LL | | foo(tx.clone());
34+
LL | | }).await;
35+
LL | | }
36+
| |_____^
37+
38+
error: aborting due to previous error
39+
40+
For more information about this error, try `rustc --explain E0277`.

‎src/test/ui/async-await/issue-70935-complex-spans.stderr‎ renamed to ‎src/test/ui/async-await/issue-70935-complex-spans.normal.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
error: future cannot be sent between threads safely
2-
--> $DIR/issue-70935-complex-spans.rs:10:45
2+
--> $DIR/issue-70935-complex-spans.rs:13:45
33
|
44
LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
55
| ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
66
|
77
= help: the trait `Sync` is not implemented for `Sender<i32>`
88
note: future is not `Send` as this value is used across an await
9-
--> $DIR/issue-70935-complex-spans.rs:15:11
9+
--> $DIR/issue-70935-complex-spans.rs:27:11
1010
|
1111
LL | baz(|| async{
1212
| _____________-
1313
LL | | foo(tx.clone());
1414
LL | | }).await;
1515
| | - ^^^^^^ await occurs here, with the value maybe used later
1616
| |_________|
17-
| has type `[closure@$DIR/issue-70935-complex-spans.rs:13:13: 15:10]` which is not `Send`
17+
| has type `[closure@$DIR/issue-70935-complex-spans.rs:25:13: 27:10]` which is not `Send`
1818
note: the value is later dropped here
19-
--> $DIR/issue-70935-complex-spans.rs:15:17
19+
--> $DIR/issue-70935-complex-spans.rs:27:17
2020
|
2121
LL | }).await;
2222
| ^

‎src/test/ui/async-await/issue-70935-complex-spans.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,28 @@
11
// edition:2018
2+
// revisions: normal drop_tracking
3+
// [drop_tracking]compile-flags:-Zdrop-tracking
24
// #70935: Check if we do not emit snippet
35
// with newlines which lead complex diagnostics.
46

57
use std::future::Future;
68

79
async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
10+
//[drop_tracking]~^ within this async block
811
}
912

1013
fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
11-
//~^ ERROR: future cannot be sent between threads safely
14+
//[normal]~^ ERROR: future cannot be sent between threads safely
15+
//[drop_tracking]~^^ ERROR: `Sender<i32>` cannot be shared
16+
//[drop_tracking]~| NOTE: cannot be shared
17+
//[drop_tracking]~| NOTE: requirements on the impl of `Send`
18+
//[drop_tracking]~| NOTE: captures the following types
19+
//[drop_tracking]~| NOTE: in this expansion
20+
//[drop_tracking]~| NOTE: in this expansion
21+
//[drop_tracking]~| NOTE: in this expansion
22+
//[drop_tracking]~| NOTE: in this expansion
1223
async move {
13-
baz(|| async{
24+
//[drop_tracking]~^ within this async block
25+
baz(|| async{ //[drop_tracking]~ NOTE: used within this closure
1426
foo(tx.clone());
1527
}).await;
1628
}

‎src/test/ui/async-await/partial-drop-partial-reinit.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@
55
fn main() {
66
gimme_send(foo());
77
//~^ ERROR cannot be sent between threads safely
8+
//~| NOTE cannot be sent
9+
//~| NOTE bound introduced by
10+
//~| NOTE appears within the type
11+
//~| NOTE captures the following types
812
}
913

1014
fn gimme_send<T: Send>(t: T) {
15+
//~^ NOTE required by this bound
16+
//~| NOTE required by a bound
1117
drop(t);
1218
}
1319

@@ -20,6 +26,8 @@ impl Drop for NotSend {
2026
impl !Send for NotSend {}
2127

2228
async fn foo() {
29+
//~^ NOTE used within this async block
30+
//~| NOTE within this `impl Future
2331
let mut x = (NotSend {},);
2432
drop(x.0);
2533
x.0 = NotSend {};

‎src/test/ui/async-await/partial-drop-partial-reinit.stderr‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,21 @@ LL | async fn foo() {
1111
|
1212
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `NotSend`
1313
= note: required because it appears within the type `(NotSend,)`
14-
= note: required because it appears within the type `{ResumeTy, (NotSend,), impl Future<Output = ()>, ()}`
15-
= note: required because it appears within the type `[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]`
16-
= 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]>`
17-
= note: required because it appears within the type `impl Future<Output = ()>`
18-
= note: required because it appears within the type `impl Future<Output = ()>`
14+
= note: required because it captures the following types: `ResumeTy`, `(NotSend,)`, `impl Future<Output = ()>`, `()`
15+
note: required because it's used within this async block
16+
--> $DIR/partial-drop-partial-reinit.rs:28:16
17+
|
18+
LL | async fn foo() {
19+
| ________________^
20+
LL | |
21+
LL | |
22+
LL | | let mut x = (NotSend {},);
23+
... |
24+
LL | | bar().await;
25+
LL | | }
26+
| |_^
1927
note: required by a bound in `gimme_send`
20-
--> $DIR/partial-drop-partial-reinit.rs:10:18
28+
--> $DIR/partial-drop-partial-reinit.rs:14:18
2129
|
2230
LL | fn gimme_send<T: Send>(t: T) {
2331
| ^^^^ required by this bound in `gimme_send`

‎src/test/ui/closures/closure-move-sync.stderr‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,15 @@ LL | let t = thread::spawn(|| {
66
|
77
= help: the trait `Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
88
= note: required because of the requirements on the impl of `Send` for `&std::sync::mpsc::Receiver<()>`
9-
= note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6]`
9+
note: required because it's used within this closure
10+
--> $DIR/closure-move-sync.rs:6:27
11+
|
12+
LL | let t = thread::spawn(|| {
13+
| ___________________________^
14+
LL | | recv.recv().unwrap();
15+
LL | |
16+
LL | | });
17+
| |_____^
1018
note: required by a bound in `spawn`
1119
--> $SRC_DIR/std/src/thread/mod.rs:LL:COL
1220
|
@@ -21,7 +29,11 @@ LL | thread::spawn(|| tx.send(()).unwrap());
2129
|
2230
= help: the trait `Sync` is not implemented for `Sender<()>`
2331
= note: required because of the requirements on the impl of `Send` for `&Sender<()>`
24-
= note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:18:19: 18:42]`
32+
note: required because it's used within this closure
33+
--> $DIR/closure-move-sync.rs:18:19
34+
|
35+
LL | thread::spawn(|| tx.send(()).unwrap());
36+
| ^^^^^^^^^^^^^^^^^^^^^^^
2537
note: required by a bound in `spawn`
2638
--> $SRC_DIR/std/src/thread/mod.rs:LL:COL
2739
|

‎src/test/ui/generator/issue-68112.rs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ pub fn make_gen1<T>(t: T) -> Ready<T> {
2020
}
2121

2222
fn require_send(_: impl Send) {}
23+
//~^ NOTE required by a bound
24+
//~| NOTE required by a bound
25+
//~| NOTE required by this bound
26+
//~| NOTE required by this bound
2327

2428
fn make_non_send_generator() -> impl Generator<Return = Arc<RefCell<i32>>> {
2529
make_gen1(Arc::new(RefCell::new(0)))
@@ -28,29 +32,39 @@ fn make_non_send_generator() -> impl Generator<Return = Arc<RefCell<i32>>> {
2832
fn test1() {
2933
let send_gen = || {
3034
let _non_send_gen = make_non_send_generator();
35+
//~^ NOTE not `Send`
3136
yield;
32-
};
37+
//~^ NOTE yield occurs here
38+
//~| NOTE value is used across a yield
39+
}; //~ NOTE later dropped here
3340
require_send(send_gen);
3441
//~^ ERROR generator cannot be sent between threads
42+
//~| NOTE not `Send`
3543
}
3644

3745
pub fn make_gen2<T>(t: T) -> impl Generator<Return = T> {
38-
|| {
46+
//~^ NOTE appears within the type
47+
//~| NOTE expansion of desugaring
48+
|| { //~ NOTE used within this generator
3949
yield;
4050
t
4151
}
4252
}
43-
fn make_non_send_generator2() -> impl Generator<Return = Arc<RefCell<i32>>> {
53+
fn make_non_send_generator2() -> impl Generator<Return = Arc<RefCell<i32>>> { //~ NOTE appears within the type
54+
//~^ NOTE expansion of desugaring
4455
make_gen2(Arc::new(RefCell::new(0)))
4556
}
4657

4758
fn test2() {
48-
let send_gen = || {
59+
let send_gen = || { //~ NOTE used within this generator
4960
let _non_send_gen = make_non_send_generator2();
5061
yield;
5162
};
5263
require_send(send_gen);
5364
//~^ ERROR `RefCell<i32>` cannot be shared between threads safely
65+
//~| NOTE `RefCell<i32>` cannot be shared between threads safely
66+
//~| NOTE requirements on the impl
67+
//~| NOTE captures the following types
5468
}
5569

5670
fn main() {}

0 commit comments

Comments
 (0)