Abort after representability errors#153317
Abort after representability errors#153317nnethercote wants to merge 2 commits intorust-lang:mainfrom
representability errors#153317Conversation
Currently, `Representability::from_cycle_error` prints an "infinite size" error and then returns `Representability::Infinite`, which lets analysis continue. This commit changes it so it just aborts after printing the error. This has two benefits. First, the error messages are better. The error messages we get after continuing are mostly bad -- we usually get another cycle error, e.g. about drop checking or layout, which is not much use to the user, and then abort after that. The only exception is `issue-105231.rs` where a "conflicting implementations" error is now omitted, but there are three other errors before that one so it's no great loss. Second, it allows some simplifications: see the next commit.
rustbot
added
A-query-system
|
|
|
This overlaps with #153028 by @Zoxc. (I wrote this independently a few days ago after writing #153169 and then subsequently discovered the overlap.) That PR changes the |
This comment has been minimized.
This comment has been minimized.
This variant was a fallback value used to continue analysis after a `Representability` error, but it's no longer needed thanks to the previous commit. This means `Representability` can now be reduced to a unit type that exists just so `FromCycleError` can exist for the `representability` query. (There is potential for additional cleanups here, but those are beyond the scope of this PR.) This means the `representability`/`representability_adt_ty` queries no longer have a meaningful return value, i.e. they are check-only queries. So they now use `ensure_ok()` (which required removing the `anon` modifier), and have a `check_` prefix added. And the `rtry!` macro is no longer needed.
nnethercote
force-pushed
the
abort-after-infinite-errors
branch
from
44bc328 to
8675222
Compare
Turns out this doesn't work for the reason because it requires dropping |
| Representable, | ||
| Infinite(ErrorGuaranteed), | ||
| } | ||
| pub struct Representability; |
There was a problem hiding this comment.
I like that you haven't got rid of this type, although I would have made it pub struct Representability(()) in order to make its construction a private detail for a query implementation.
There was a problem hiding this comment.
Hmm, but we cannot do that as query implementation is in another crate. I guess I would've done new_unchecked unsafe fn instead then if rustc was my project, but that's kinda a far too theoretical concern for an open project.
The second commit message still mentions ensure_ok and anon |
6 participants
Doing so results in better error messages and makes the code a bit simpler. Details in individual commits.
r? @oli-obk