-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Print nicer async/await trait errors for generators in any place in the error 'stack' #67116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5381ddf
b135976
17d6d11
151bae9
3960826
da0d053
416d7d3
10a7633
25c3035
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ use crate::ty::fast_reject; | |
use crate::ty::fold::TypeFolder; | ||
use crate::ty::subst::Subst; | ||
use crate::ty::SubtypePredicate; | ||
use crate::ty::TraitPredicate; | ||
use crate::util::nodemap::{FxHashMap, FxHashSet}; | ||
|
||
use errors::{Applicability, DiagnosticBuilder, pluralize, Style}; | ||
|
@@ -2103,12 +2104,28 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
err: &mut DiagnosticBuilder<'_>, | ||
obligation: &PredicateObligation<'tcx>, | ||
) { | ||
|
||
debug!("note_obligation_cause: obligation.predicate={:?} \ | ||
obligation.cause.span={:?}", obligation.predicate, obligation.cause.span); | ||
|
||
// First, attempt to add note to this error with an async-await-specific | ||
// message, and fall back to regular note otherwise. | ||
if !self.maybe_note_obligation_cause_for_async_await(err, obligation) { | ||
self.note_obligation_cause_code(err, &obligation.predicate, &obligation.cause.code, | ||
&mut vec![]); | ||
if let ty::Predicate::Trait(trait_predicate) = obligation.predicate { | ||
if self.maybe_note_obligation_cause_for_async_await( | ||
err, | ||
trait_predicate.skip_binder(), | ||
&obligation.cause.code | ||
) { | ||
return; | ||
} | ||
} | ||
|
||
self.note_obligation_cause_code( | ||
err, | ||
&obligation.predicate, | ||
&obligation.cause.code, | ||
&mut vec![] | ||
); | ||
} | ||
|
||
/// Adds an async-await specific note to the diagnostic when the future does not implement | ||
|
@@ -2156,12 +2173,17 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
fn maybe_note_obligation_cause_for_async_await( | ||
&self, | ||
err: &mut DiagnosticBuilder<'_>, | ||
obligation: &PredicateObligation<'tcx>, | ||
trait_predicate: &TraitPredicate<'tcx>, | ||
code: &ObligationCauseCode<'tcx>, | ||
) -> bool { | ||
debug!("maybe_note_obligation_cause_for_async_await: obligation.predicate={:?} \ | ||
obligation.cause.span={:?}", obligation.predicate, obligation.cause.span); | ||
debug!("note_obligation_cause_for_async_await: trait_predicate={:?} \ | ||
code={:?}", trait_predicate, code); | ||
let source_map = self.tcx.sess.source_map(); | ||
|
||
let (trait_ref, target_ty) = (trait_predicate.trait_ref, trait_predicate.self_ty()); | ||
|
||
debug!("note_obligation_cause_for_async_await: target_ty={:?}", target_ty); | ||
|
||
// Attempt to detect an async-await error by looking at the obligation causes, looking | ||
// for a generator to be present. | ||
// | ||
|
@@ -2187,51 +2209,99 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
// The first obligation in the chain is the most useful and has the generator that captured | ||
// the type. The last generator has information about where the bound was introduced. At | ||
// least one generator should be present for this diagnostic to be modified. | ||
let (mut trait_ref, mut target_ty) = match obligation.predicate { | ||
ty::Predicate::Trait(p) => | ||
(Some(p.skip_binder().trait_ref), Some(p.skip_binder().self_ty())), | ||
_ => (None, None), | ||
}; | ||
let mut generator = None; | ||
let mut last_generator = None; | ||
let mut next_code = Some(&obligation.cause.code); | ||
let mut next_code = Some(code); | ||
let mut next_predicate = None; | ||
|
||
// First, find an `ObligationCause` code referencing a generator. | ||
// Skip over intermediate causes generated by `.await` lowering | ||
while let Some(code) = next_code { | ||
debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code); | ||
debug!("note_obligation_cause_for_async_await: code={:?}", code); | ||
match code { | ||
ObligationCauseCode::BuiltinDerivedObligation(derived_obligation) | | ||
ObligationCauseCode::ImplDerivedObligation(derived_obligation) => { | ||
let ty = derived_obligation.parent_trait_ref.self_ty(); | ||
debug!("maybe_note_obligation_cause_for_async_await: \ | ||
parent_trait_ref={:?} self_ty.kind={:?}", | ||
derived_obligation.parent_trait_ref, ty.kind); | ||
debug!("note_obligation_cause_for_async_await: self_ty.kind={:?}", | ||
derived_obligation.parent_trait_ref.self_ty().kind); | ||
|
||
next_code = Some(derived_obligation.parent_code.as_ref()); | ||
next_predicate = Some( | ||
self.resolve_vars_if_possible(&derived_obligation.parent_trait_ref) | ||
.to_predicate() | ||
); | ||
|
||
match ty.kind { | ||
match derived_obligation.parent_trait_ref.self_ty().kind { | ||
ty::Adt(ty::AdtDef { did, .. }, ..) if | ||
self.tcx.is_diagnostic_item(sym::gen_future, *did) => {}, | ||
ty::Generator(did, ..) => { | ||
generator = generator.or(Some(did)); | ||
last_generator = Some(did); | ||
generator = Some(did); | ||
debug!("note_obligation_cause_for_async_await: found generator {:?}", | ||
generator); | ||
break; | ||
}, | ||
ty::GeneratorWitness(..) => {}, | ||
_ if generator.is_none() => { | ||
trait_ref = Some(*derived_obligation.parent_trait_ref.skip_binder()); | ||
target_ty = Some(ty); | ||
}, | ||
_ => {}, | ||
ty::GeneratorWitness(_) | ty::Opaque(..) => {}, | ||
_ => return false, | ||
} | ||
|
||
}, | ||
_ => return false, | ||
} | ||
}; | ||
|
||
// Now that we've found a generator, try to determine if we should | ||
// skip over any subsequence causes. | ||
while let Some(code) = next_code { | ||
debug!("note_obligation_cause_for_async_await: inspecting code={:?}", code); | ||
match code { | ||
ObligationCauseCode::BuiltinDerivedObligation(derived_obligation) | | ||
ObligationCauseCode::ImplDerivedObligation(derived_obligation) => { | ||
debug!("note_obligation_cause_for_async_await: inspecting self_ty.kind={:?}", | ||
derived_obligation.parent_trait_ref.self_ty().kind); | ||
|
||
|
||
match derived_obligation.parent_trait_ref.self_ty().kind { | ||
ty::Adt(ty::AdtDef { did, .. }, ..) if | ||
self.tcx.is_diagnostic_item(sym::gen_future, *did) => {}, | ||
ty::Opaque(did, _) if | ||
self.tcx.parent(did).map(|parent| { | ||
self.tcx.is_diagnostic_item(sym::from_generator, parent) | ||
}).unwrap_or(false) => {} | ||
_ => break, | ||
} | ||
|
||
next_code = Some(derived_obligation.parent_code.as_ref()); | ||
next_predicate = Some( | ||
self.resolve_vars_if_possible(&derived_obligation.parent_trait_ref) | ||
.to_predicate() | ||
) | ||
}, | ||
_ => break, | ||
_ => break | ||
} | ||
} | ||
|
||
let generator_did = generator.expect("can only reach this if there was a generator"); | ||
|
||
// Finally, see if there are any generator types remaining in the 'error stack'. | ||
// If there aren't, then treat the current generator as the 'last generator', | ||
// which will make `note_obligation_cause_for_async_await` emit an extra | ||
// message for it | ||
let mut target_code = next_code; | ||
let mut last_generator = Some(generator_did); | ||
while let Some(code) = target_code { | ||
debug!("note_obligation_cause_for_async_await: last_generator code={:?}", code); | ||
if let ObligationCauseCode::BuiltinDerivedObligation(obligation) | | ||
ObligationCauseCode::ImplDerivedObligation(obligation) = code { | ||
if let ty::Generator(..) = obligation.parent_trait_ref.self_ty().kind { | ||
// We found another generator, so our current generator can't be | ||
// the last one | ||
last_generator = None; | ||
break; | ||
} | ||
target_code = Some(obligation.parent_code.as_ref()); | ||
} else { | ||
break; | ||
} | ||
} | ||
|
||
// Only continue if a generator was found. | ||
debug!("maybe_note_obligation_cause_for_async_await: generator={:?} trait_ref={:?} \ | ||
target_ty={:?}", generator, trait_ref, target_ty); | ||
let (generator_did, trait_ref, target_ty) = match (generator, trait_ref, target_ty) { | ||
(Some(generator_did), Some(trait_ref), Some(target_ty)) => | ||
(generator_did, trait_ref, target_ty), | ||
_ => return false, | ||
}; | ||
|
||
let span = self.tcx.def_span(generator_did); | ||
|
||
|
@@ -2290,7 +2360,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
if let Some((target_span, Ok(snippet), scope_span)) = target_span { | ||
self.note_obligation_cause_for_async_await( | ||
err, *target_span, scope_span, snippet, generator_did, last_generator, | ||
trait_ref, target_ty, tables, obligation, next_code, | ||
trait_ref, target_ty, tables, &next_predicate.unwrap(), next_code, | ||
); | ||
true | ||
} else { | ||
|
@@ -2311,7 +2381,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
trait_ref: ty::TraitRef<'_>, | ||
target_ty: Ty<'tcx>, | ||
tables: &ty::TypeckTables<'_>, | ||
obligation: &PredicateObligation<'tcx>, | ||
predicate: &ty::Predicate<'tcx>, | ||
next_code: Option<&ObligationCauseCode<'tcx>>, | ||
) { | ||
let source_map = self.tcx.sess.source_map(); | ||
|
@@ -2342,9 +2412,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
("`Sync`", "shared") | ||
}; | ||
|
||
err.clear_code(); | ||
err.set_primary_message( | ||
format!("future cannot be {} between threads safely", trait_verb) | ||
&format!("future cannot be {} between threads safely", trait_verb) | ||
); | ||
|
||
let original_span = err.span.primary_span().unwrap(); | ||
|
@@ -2396,7 +2465,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
debug!("note_obligation_cause_for_async_await: next_code={:?}", next_code); | ||
self.note_obligation_cause_code( | ||
err, | ||
&obligation.predicate, | ||
&predicate, | ||
next_code.unwrap(), | ||
&mut Vec::new(), | ||
); | ||
|
@@ -2410,6 +2479,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
where T: fmt::Display | ||
{ | ||
let tcx = self.tcx; | ||
|
||
|
||
let mut handle_async_await = |trait_ref, parent_code| { | ||
self.maybe_note_obligation_cause_for_async_await( | ||
err, | ||
&TraitPredicate { trait_ref }, | ||
parent_code | ||
) | ||
}; | ||
|
||
match *cause_code { | ||
ObligationCauseCode::ExprAssignable | | ||
ObligationCauseCode::MatchExpressionArm { .. } | | ||
|
@@ -2550,6 +2629,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
} | ||
ObligationCauseCode::BuiltinDerivedObligation(ref data) => { | ||
let parent_trait_ref = self.resolve_vars_if_possible(&data.parent_trait_ref); | ||
|
||
if handle_async_await(*parent_trait_ref.skip_binder(), &data.parent_code) { | ||
return; | ||
} | ||
|
||
let ty = parent_trait_ref.skip_binder().self_ty(); | ||
err.note(&format!("required because it appears within the type `{}`", ty)); | ||
obligated_types.push(ty); | ||
|
@@ -2564,6 +2648,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |
} | ||
ObligationCauseCode::ImplDerivedObligation(ref data) => { | ||
let parent_trait_ref = self.resolve_vars_if_possible(&data.parent_trait_ref); | ||
|
||
if handle_async_await(*parent_trait_ref.skip_binder(), &data.parent_code) { | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you elaborate on which cases are supported by adding |
||
|
||
err.note( | ||
&format!("required because of the requirements on the impl of `{}` for `{}`", | ||
parent_trait_ref.print_only_trait_path(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
error: future cannot be sent between threads safely | ||
error[E0277]: future cannot be sent between threads safely | ||
--> $DIR/async-fn-nonsend.rs:50:5 | ||
| | ||
LL | fn assert_send(_: impl Send) {} | ||
|
@@ -18,8 +18,9 @@ LL | fut().await; | |
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later | ||
LL | } | ||
| - `x` is later dropped here | ||
= note: required because it appears within the type `impl std::future::Future` | ||
|
||
error: future cannot be sent between threads safely | ||
error[E0277]: future cannot be sent between threads safely | ||
--> $DIR/async-fn-nonsend.rs:52:5 | ||
| | ||
LL | fn assert_send(_: impl Send) {} | ||
|
@@ -39,8 +40,9 @@ LL | Some(_) => fut().await, | |
... | ||
LL | } | ||
| - `non_send()` is later dropped here | ||
= note: required because it appears within the type `impl std::future::Future` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the notes that just cite |
||
|
||
error: future cannot be sent between threads safely | ||
error[E0277]: future cannot be sent between threads safely | ||
--> $DIR/async-fn-nonsend.rs:54:5 | ||
| | ||
LL | fn assert_send(_: impl Send) {} | ||
|
@@ -50,6 +52,8 @@ LL | assert_send(non_sync_with_method_call()); | |
| ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send` | ||
| | ||
= help: the trait `std::marker::Send` is not implemented for `dyn std::fmt::Write` | ||
= note: required because of the requirements on the impl of `std::marker::Send` for `&mut dyn std::fmt::Write` | ||
= note: required because it appears within the type `std::fmt::Formatter<'_>` | ||
note: future is not `Send` as this value is used across an await | ||
--> $DIR/async-fn-nonsend.rs:43:9 | ||
| | ||
|
@@ -61,8 +65,9 @@ LL | fut().await; | |
LL | } | ||
LL | } | ||
| - `f` is later dropped here | ||
= note: required because it appears within the type `impl std::future::Future` | ||
|
||
error: future cannot be sent between threads safely | ||
error[E0277]: future cannot be sent between threads safely | ||
--> $DIR/async-fn-nonsend.rs:54:5 | ||
| | ||
LL | fn assert_send(_: impl Send) {} | ||
|
@@ -72,6 +77,12 @@ LL | assert_send(non_sync_with_method_call()); | |
| ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send` | ||
| | ||
= help: within `std::fmt::ArgumentV1<'_>`, the trait `std::marker::Sync` is not implemented for `*mut (dyn std::ops::Fn() + 'static)` | ||
= note: required because it appears within the type `std::marker::PhantomData<*mut (dyn std::ops::Fn() + 'static)>` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have to say that I'm not sure how much value this provides either. Based on the error message, at least, I admit I can't really make heads or tails of these types. At best, it seems sort of like they're appearing in the wrong place, as they seem to be explaining something about the variable I guess I feel like in practice if I saw this error I'd just skip over all those notes and I'd probably tell users to do the same. That said, I can imagine that in some very confusing cases notes like this could be helpful -- but I'd sort of like an example of such a case. |
||
= note: required because it appears within the type `core::fmt::Void` | ||
= note: required because it appears within the type `&core::fmt::Void` | ||
= note: required because it appears within the type `std::fmt::ArgumentV1<'_>` | ||
= note: required because of the requirements on the impl of `std::marker::Send` for `std::slice::Iter<'_, std::fmt::ArgumentV1<'_>>` | ||
= note: required because it appears within the type `std::fmt::Formatter<'_>` | ||
note: future is not `Send` as this value is used across an await | ||
--> $DIR/async-fn-nonsend.rs:43:9 | ||
| | ||
|
@@ -83,6 +94,8 @@ LL | fut().await; | |
LL | } | ||
LL | } | ||
| - `f` is later dropped here | ||
= note: required because it appears within the type `impl std::future::Future` | ||
|
||
error: aborting due to 4 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0277`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any strong feelings about undoing this change from #65345. My intent in clearing the error code was avoiding having
E0277
have multiple different primary messages:#65345 introduced two special-cased error messages:
Send
)Sync
)E0277
otherwise has the following message:If we don't mind having
E0277
be the error code for all of these cases, then we should remove this line.cc @rust-lang/wg-diagnostics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do find E0277 to be a too generic error that happens all over the place, and we have precedence of changing error codes for more specific cases (type errors are not just E0308). We should add new error codes for these cases so that we can add explanations in the index.