Split delayed bugs into has-errored and will-error halves. #121016
Conversation
rustbot
added
A-query-system
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Type relation code was changed Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor Some changes occurred in cc @BoxyUwU
cc @davidtwco, @compiler-errors, @TaKO8Ki rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer Some changes might have occurred in exhaustiveness checking cc @Nadrieril Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
| @@ -1089,7 +1089,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { | |||
| ); | |||
|
|
|||
| if result.is_err() { | |||
There was a problem hiding this comment.
just take the ErrorGuaranteed that is already there
| tcx.dcx() | ||
| .span_delayed_bug(attr.span, "this attribute can only be applied to functions"); | ||
| tcx.dcx().span_assert_has_errors( | ||
| attr.span, | ||
| "this attribute can only be applied to functions", | ||
| ); |
There was a problem hiding this comment.
The comment above talks about delayed bug, and it's not clear to me this will not be reachable by some random reordering of queries
There was a problem hiding this comment.
While I definitely want this split, I also think it's too messy to do this change in a (semi-)mechanical manner. Lots of these cases could just pass an ErrorGuaranteed along, and most others aren't clearly ICE free to me.
I think we should just add the scheme and use it to replace our existing has_errors().unwrap() calls, and then keep expanding its scope as we do cleanups.
|
If we go with this as proposed, this seems worth some sort of MCP or so -- mostly for making as many people as possible aware that this happens so that when inevitably gaps in our test suite are discovered and more of these need to be converted to "will-error" cases, people already know that this is what happens (skipping potentially lengthy investigations) and can ping the right folks. |
My plan was the opposite 😅 check that a delay_span_bug is followed by an error, even if there were errors beforehand |
|
r? oli-obk |
|
Assignment is not allowed on a closed PR. |
|
whoops! sorry, misclick r? oli-obk |
|
☔ The latest upstream changes (presumably #121036) made this pull request unmergeable. Please resolve the merge conflicts. |
Once we have emitted at least one error, delayed bugs won't be used. So we can (a) we can (a) discard any existing delayed bugs, and (b) stop recording any new delayed bugs. This eliminates a longstanding `FIXME` comment. There should be no soundness issues because it's not possible to un-emit an error.
This commit adds new `{span_,}assert_has_errors` methods implementing
the simpler has-errored cases, and leaves the existing
`{span_,}delayed_bug` methods for the will-error cases.
It also converts as many cases as possible to has-errored. I did this by
converting every case to has-errored, then running tests and converting
back to will-error every case that caused an assertion in the test
suite. The test suite doesn't have perfect coverage so it's possible
that there are a few more has-errored cases that will need conversion
back to will-error. Also, some of the will-error cases might actually be
a mixture of has-errored and will-error. But it's hard to do this
perfectly without carefully going through every individual case, which
would be painful, and an imperfect split still gets most of the
benefits.
nnethercote
force-pushed
the
split-delayed_bug
branch
from
36f93cb
to
33a6833
Compare
|
There are about 200 delayed bug uses and about 2/3 of them can be converted to Nonetheless, I've filed #121071. It doesn't add any new |
|
My worry is that when we make rustc more parallel we'll end up with lots of ICEs where we previously had a guaranteed order. |
|
☔ The latest upstream changes (presumably #121018) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Closing in favour of #121071. |
Split delayed bugs into has-errored and will-error halves.
This commit adds new
{span_,}assert_has_errorsmethods implementing the simpler has-errored cases, and leaves the existing{span_,}delayed_bugmethods for the will-error cases.It also converts as many cases as possible to has-errored. I did this by converting every case to has-errored, then running tests and converting back to will-error every case that caused an assertion in the test suite. The test suite doesn't have perfect coverage so it's possible that there are a few more has-errored cases that will need conversion back to will-error. Also, some of the will-error cases might actually be a mixture of has-errored and will-error. But it's hard to do this perfectly without carefully going through every individual case, which would be painful, and an imperfect split still gets most of the benefits.
r? @compiler-errors