-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Make typeck aware of uninhabited types #100288
Conversation
I'm not entirely confident in the test changes tbh. Maybe a few corner cases need some ironing. |
This comment has been minimized.
This comment has been minimized.
026ab44
to
df073c7
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #100304) made this pull request unmergeable. Please resolve the merge conflicts. |
df073c7
to
21fbdf7
Compare
21fbdf7
to
46ec791
Compare
This comment has been minimized.
This comment has been minimized.
46ec791
to
7d40885
Compare
This comment has been minimized.
This comment has been minimized.
7d40885
to
3ad87e0
Compare
This comment has been minimized.
This comment has been minimized.
6af9e16
to
2d81d20
Compare
☔ The latest upstream changes (presumably #98655) made this pull request unmergeable. Please resolve the merge conflicts. |
2d81d20
to
360c63f
Compare
This comment has been minimized.
This comment has been minimized.
360c63f
to
d0a27f1
Compare
How does this PR articulate with #100720? |
There are a number of queries that are susceptible to cycle errors if they run on an infinitely recursive (unrepresentable) ADT. adt_sized_constraint and type_uninhabited_from are a couple such queries. There are multiple ways to solve the cycle error problem for a given query:
So we can either be conservative and try to guard queries from unrepresentable types as much as possible (halt rustc early), or go the other direction and try to recover-and-continue as much as possible for unrepresentable types. Or maybe the right balance is somewhere in the middle. I'm not quite sure. In any case, it might make sense to try and merge #100720 first on its own, and then we can think about how other parts of the compiler may or may not use the new |
d0a27f1
to
ad74cc0
Compare
Removed the early exit and instead added |
Avoid deriving PartialOrd on Diverges since it includes fields which should not affect ordering.
8dea84f
to
b71f85d
Compare
This comment has been minimized.
This comment has been minimized.
Bumped into #103220. I may need help with this one. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
As lifetimes are not meaningful in this check, you can call |
Then I bump into |
I'm not too familiar with this PR so take my advice with a grain of salt, but if you're performing a query on a type that has inference variables, and then returning a response that relates to those inference variables, you really should be canonicalizing: https://doc.rust-lang.org/stable/nightly-rustc/rustc_infer/infer/canonical/canonicalizer/struct.Canonicalizer.html#method.canonicalize |
I think that ICE saved us from an hard-to-debug issue. There are 2 cases:
IIUC, the goal of the call to |
If it's just for the |
After thinking a bit more about what happens, I agree that using Splitting the |
Is this really okay? I'm worried that such cases are not well defined. Like if we make
Agreed. |
resolve_vars_if_possible probably won't get smarter, but it's results depend on the unification state of the type checker. Notably, on the order in which expressions are checked for type inference. I'll have to read the surrounding code more precisely to see how that order interferes with the uninhabited check. |
☔ The latest upstream changes (presumably #103904) made this pull request unmergeable. Please resolve the merge conflicts. |
Unfortunately I haven't had much time for rust lately. @cjgillot PM'ed me and offered to take over here. I'm just going to close this PR. |
It was previously assumed in #85556 that we can't check if an expression has an uninhabited type in typeck. So an
unreachable_code
case was added to the liveness pass instead. This PR moves that case back into typeck. It just requires addingcycle_delay_bug
to the uninhabited query.Incidentally this leads to some new cases for
unreachable_code
. But it should simply be consistent with how!
is currently handled.My original motivation is to remove a blocker to doing some refactoring on liveness.
r? rust-lang/compiler