diagnostics: Point closure trait errors at captured values - #161624
rust-bors[bot] merged 3 commits into
Conversation
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@rustbot label +A-closures +A-auto-traits +C-enhancement +D-terse |
| Some(typeck_results) if typeck_results.hir_owner.to_def_id() == typeck_root => { | ||
| typeck_results.closure_min_captures_flattened(closure_def_id).collect() | ||
| } | ||
| _ => self.tcx.closure_captures(closure_def_id).to_vec(), |
There was a problem hiding this comment.
This condition would override the existing error unintentionally, like:
#![recursion_limit = "2"]
fn require_send<T: Send>(_: T) {}
fn main() {
let x = (1u8,);
require_send(move || { drop(x); });
}This should return a recursive error but it now returns a cyclic one, maybe tyctx or something else would be overwritten somewhere.
There was a problem hiding this comment.
Yep, makes sense, I missed this, ty.
What happens is that fallback calls tcx.closure_captures, which goes through closure_typeinfo and asks for typeck again. Overflow gets reported from inside trait selection with a plain infcx.err_ctxt(), so there are no typeck results around, and it takes the fallback while typeck of the same body is still on the stack. That's your cycle. I ran your snippet with -Znext-solver=no and got E0391: cycle detected when type-checking main before the fix, E0275 after.
The fix is small and kind of dumb...only look at the typeck results the error context is already carrying, and only if they belong to the closure's own root. If they aren't there, we print the old note like before.
Two tests go back to the old note because of that, auto-trait-leak2 and issue-70935-complex-spans. Those errors come after typeck, from the opaque type side, so nothing is carrying results by then. I spent a while trying to keep the query for cases like those, but from error reporting I can't really tell whether typeck for that body already finished or is still running, and guessing wrong is what got us here in the first place. It bugs me a bit, the async one is exactly where naming the capture pays off, but I'd rather print a vague note than eat the real error.
Also stole your snippet for a test. It needs -Znext-solver=no because on the default solver the recursion limit is the recursion_depth_exceeding_limit lint now, and that path doesn't walk the obligation chain, so it never reaches this note at all.
Reaching for `tcx.closure_captures` asks for `typeck` again. When the error is reported while typeck of the closure's root is still running, that turns the reported error into a query cycle, so restrict the note to the results that are already at hand.
46d2ba5 to
5332fa8
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. |
Rollup of 14 pull requests Successful merges: - #162404 (`rust-analyzer` subtree update) - #161624 (diagnostics: Point closure trait errors at captured values) - #161697 (make `Complex` ABI-compatible on sparc64 and powerpc64) - #162182 (delay unexpected successful goal during ambiguity reporting) - #162328 (Allow overriding filecheck even if LLVM is built or downloaded) - #162367 (Use `reason` for tracked item diagnostics from `cfg_select!`) - #162381 (fix bare urls split text) - #162388 (std: fix set_permissions_nofollow on espidf and horizon) - #162319 (docs(core): correct ARMv8-M Baseline atomic CAS support) - #162341 (add regression test for packus_epi16 issue) - #162383 (Add a hint for using `nolimit` to the limiting error message) - #162384 (remove EnumSizeOpt) - #162390 (remove outdated comment in `UnsafeCell::raw_get` source) - #162397 (docs: Ask for ABI documentation in the platform support template)
Rollup merge of #161624 - Dnreikronos:diagnostics/closure_capture_source, r=JohnTitor diagnostics: Point closure trait errors at captured values Fixes #161371 Right now this error knows that something captured by a closure doesn't meet a trait bound, but the note points at the whole closure. That's fine in a tiny example. Once the closure has a bunch of captures, it's pretty annoying because you still have to hunt down the value that caused it. The useful type is already in the obligation chain, so the diagnostic keeps it while walking back to the closure and matches it with the capture list. The next solver puts an internal tuple in the middle, which is a little awkward, so the diagnostic skips that part. Then the note can name the capture and point at where it was used. If the match doesn't work, rustc keeps the old note. I first thought about carrying capture info from the trait solver itself, but that felt like a lot of plumbing for one diagnostic. Keeping it in error reporting felt smaller and easier to follow. I added a test with one bad capture next to an unrelated one, then updated the existing UI output that now gets the more useful span.
Rollup of 14 pull requests Successful merges: - rust-lang/rust#162404 (`rust-analyzer` subtree update) - rust-lang/rust#161624 (diagnostics: Point closure trait errors at captured values) - rust-lang/rust#161697 (make `Complex` ABI-compatible on sparc64 and powerpc64) - rust-lang/rust#162182 (delay unexpected successful goal during ambiguity reporting) - rust-lang/rust#162328 (Allow overriding filecheck even if LLVM is built or downloaded) - rust-lang/rust#162367 (Use `reason` for tracked item diagnostics from `cfg_select!`) - rust-lang/rust#162381 (fix bare urls split text) - rust-lang/rust#162388 (std: fix set_permissions_nofollow on espidf and horizon) - rust-lang/rust#162319 (docs(core): correct ARMv8-M Baseline atomic CAS support) - rust-lang/rust#162341 (add regression test for packus_epi16 issue) - rust-lang/rust#162383 (Add a hint for using `nolimit` to the limiting error message) - rust-lang/rust#162384 (remove EnumSizeOpt) - rust-lang/rust#162390 (remove outdated comment in `UnsafeCell::raw_get` source) - rust-lang/rust#162397 (docs: Ask for ABI documentation in the platform support template)
Fixes #161371
Right now this error knows that something captured by a closure doesn't meet a trait bound, but the note points at the whole closure. That's fine in a tiny example. Once the closure has a bunch of captures, it's pretty annoying because you still have to hunt down the value that caused it.
The useful type is already in the obligation chain, so the diagnostic keeps it while walking back to the closure and matches it with the capture list. The next solver puts an internal tuple in the middle, which is a little awkward, so the diagnostic skips that part. Then the note can name the capture and point at where it was used. If the match doesn't work, rustc keeps the old note.
I first thought about carrying capture info from the trait solver itself, but that felt like a lot of plumbing for one diagnostic. Keeping it in error reporting felt smaller and easier to follow. I added a test with one bad capture next to an unrelated one, then updated the existing UI output that now gets the more useful span.