diagnostics: point to coroutine body on higher-ranked auto trait errors#156226
diagnostics: point to coroutine body on higher-ranked auto trait errors#156226shivendra02467 wants to merge 1 commit into
Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Seems plausible but I don't know enough about the underlying issue to review effectively. Let's try someone else: r? @lcnr |
|
Hi @lcnr, just a gentle ping on this whenever you have a moment! Since you opened the original issue, I'd love to get your thoughts on this approach. |
|
|
||
| loop { | ||
| match current_code { | ||
| ObligationCauseCode::MatchImpl(inner_cause, _) => { | ||
| current_code = inner_cause.code(); | ||
| } | ||
| ObligationCauseCode::BuiltinDerived(derived) => { | ||
| let self_ty = derived.parent_trait_pred.skip_binder().self_ty(); | ||
|
|
||
| if let ty::Coroutine(def_id, _) | ty::CoroutineWitness(def_id, _) = | ||
| self_ty.kind() | ||
| { | ||
| span = self.tcx().def_span(*def_id); | ||
| break; | ||
| } | ||
|
|
||
| current_code = &derived.parent_code; | ||
| } | ||
| _ => break, | ||
| } |
There was a problem hiding this comment.
it's good to see that this information is already accessible. We should also keep the original span, we want to know "where do we require things to be Send and error" and "why do we error".
So instead of replacing the span, it would be good to add an additional note, so sth like
error: implementation of `Send` is not general enough
--> src/main.rs:25:5
|
25 | is_send(outer())
| ^^^^^^^^^^^^^^^^ implementation of `Send` is not general enough
--> src/main.rs:13:74
|
13 | async fn inner<'a, T: Trait + 'a>(_: T, x: T::Assoc<'a>) -> T::Assoc<'a> {
| __________________________________________________________________________^
14 | | std::future::ready(x).await
15 | | }
| |_^ this async block has a field of type `&'0 u32` which is not known to be `Send`
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`There was a problem hiding this comment.
Makes total sense, I'll get that updated. also
In your example, you mention extracting the specific interior field type (&'0 u32). Dynamically extracting the exact interior type that caused the failure from the placeholder error seems like a much larger/more complex change.
Would a generic note like "this coroutine or async block is not general enough" be acceptable for the scope of this PR, or were you hoping to get the specific interior type extraction implemented here as well?
There was a problem hiding this comment.
"this async block captures a value whose type is not Send" or sth?
an async block being not general enough doesn't really make sense to me. But ye, having some more general "this async block needs to impl Send seems fine. I think you can get the type of the affected field by looking at the self type of its child obligation.
There was a problem hiding this comment.
I don't know how we talk about the fields of coroutine witnesses in diagnostics, cc @rust-lang/wg-diagnostics or maybe @rust-lang/lang-docs
There was a problem hiding this comment.
you can compute this def_id at the end of the error here? No longer have to do it at the start of the function
There was a problem hiding this comment.
that makes total sense, i've updated the PR, thanks!
There was a problem hiding this comment.
"this async block captures a value whose type is not
Send" or sth?
Sounds right to me.
Thanks! and no problem. |
8c5f8d8 to
d561ece
Compare
This comment has been minimized.
This comment has been minimized.
d561ece to
ba78256
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| | __________________________________________________________________________- | ||
| LL | | std::future::ready(x).await | ||
| LL | | } | ||
| | |_- this async fn captures a value with an unsatisfied trait bound |
There was a problem hiding this comment.
cc @rust-lang/types I don't know how to best word this annotation. Thoughts?
I feel like "captures a value" is not completely ideal
There was a problem hiding this comment.
I'll hold off on changing the "captures a value..." text until the types team weighs in on what terminology they prefer here!
ba78256 to
0606c3e
Compare
0606c3e to
6789fc0
Compare
|
Awesome, thanks for confirming that phrasing, @traviscross! @lcnr I've pushed an update that makes the label dynamically match the phrasing traviscross approved. It now grabs the specific trait name, so it will output "this async fn captures a value whose type is not |
Fixes #155880
When encountering a higher-ranked auto trait bound error involving a coroutine or async function,the trait solver previously used the span of the outermost cause (eg.
spawn(...)oris_send(...)).This PR modifies the
TraitPlaceholderMismatchformatting logic to walk down theObligationCauseCodechain.If the obligation originates from aty::Coroutineorty::CoroutineWitness, it extracts that specific span, providing a much more accurate underline for the user.Before:
After: