-
Notifications
You must be signed in to change notification settings - Fork 14k
make coerce-lub order independent #147565
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
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
70c3cf5 to
0d38d86
Compare
This comment has been minimized.
This comment has been minimized.
0d38d86 to
1375123
Compare
This comment has been minimized.
This comment has been minimized.
1375123 to
0b58522
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bce0933 to
eb6434f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
e5524ca to
d8775a4
Compare
This comment has been minimized.
This comment has been minimized.
d8775a4 to
612a766
Compare
This comment has been minimized.
This comment has been minimized.
612a766 to
1184da6
Compare
This comment has been minimized.
This comment has been minimized.
797201d to
3ecd1a8
Compare
This comment has been minimized.
This comment has been minimized.
6179239 to
2958ef0
Compare
This comment has been minimized.
This comment has been minimized.
2958ef0 to
8ebe57f
Compare
644496d to
7b5f145
Compare
|
🎉 Experiment
Footnotes
|
|
r? lcnr |
|
Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a Some changes occurred to the CTFE machinery Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred to constck cc @fee1-dead Some changes occurred in src/tools/clippy cc @rust-lang/clippy changes to the core type system Some changes occurred in compiler/rustc_codegen_ssa This PR changes a file inside |
7b5f145 to
e8753d4
Compare
|
This PR was rebased onto a different master 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. |
e8753d4 to
f810d18
Compare
- leak checking the lub for fndef<->fndef coerce-lubs - start lubbing closure<->closure coerce-lubs and leak check it
f810d18 to
3ae3c8c
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
blocked on deciding what to do about breakage :) |
based on #147565
make coerce-lubs order independent
cc #73154 #97206
lubcurrently equates binders and then just returns the lhs without ever leak checking to ensure the binders are actually equal. this can lead to coerce-lub operations being order dependent, i.e. thata coerce-lub bhas differing behaviour tob coerce-lub a.we also cannot rely on borrow checking actually checking the constraints from equating the binders as
lubis a hir-typeck coercion concept that is not relevant once MIR has been built. In the MIR we can simply subtype all the types of match arms with the final type of the match.it is a forwards compatibility hazard to have coerce-lub be order dependent as it results in an effectively arbitrary inference choice which may or may not actually be correct or compatible with hypothetical future behaviours of
lubthat actually try to compute a proper lub'd type.on stable we attempt to make coerce-lubs order independent with two checks:
fndef->fnptrcoercionsfnptr->fnptrcoercionson stable we do this regardless of whether the coercion is occuring as part of a
coerce-lubor not.under this PR we have the following checks:
fnptr/fndef->fnptrcoercionsclosure->fnptrcoercions performed as part of a coerce-lubclosure/fndef<->closure/fndefcoerce-lubs that coerce both sides to fnptrsluboperation when a coerce-lub does not actually coercethis is theoretically breaking in two cases:
the second case actually is encountered by a few crates in practice which has been somewhat mitigated by the next change.
improving binder lub
this PR also implements a slight improvement to
luboperations on higher ranked types to avoid some regressions: #147565 (comment)when lubbing two higher ranked types, if one of the binders is empty then we now instantiate the other binder with inference variables and lub the instantiated type with the empty-binder type.
concretely:
for<'a> fn(&'a ()) lub fn(&'static ())fn(&'?a ()) lub fn(&'static ())'?a eq 'staticpasses leak checkwhereas the previous behaviour was:
for<'a> fn(&'a ()) lub fn(&'static ())fn(&'!a ()) eq fn(&'static ())'!a eq 'staticfails leak checkthis change is just theoretically incorrect :3