-
Notifications
You must be signed in to change notification settings - Fork 13.9k
coercions reviews/misc whatevers #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
Draft
BoxyUwU
wants to merge
9
commits into
rust-lang:master
Choose a base branch
from
BoxyUwU:coercion_cleanup
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
coercions reviews/misc whatevers #147565
+1,115
−477
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
3ecd1a8 to
f23f16b
Compare
6179239 to
2958ef0
Compare
This comment has been minimized.
This comment has been minimized.
This has three main effects: 1. we now leak check for closure->fnptr and fnptr->closure coercions 2. we now leak check normal subtyping in HIR typeck when going through coercions 3. we now leak check normal lubbing when there is no coercion
8ebe57f to
957898a
Compare
This comment has been minimized.
This comment has been minimized.
957898a to
b6db8f4
Compare
This comment has been minimized.
This comment has been minimized.
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit f5ebf2e with merge 84addd3… To cancel the try build, run the command Workflow: https://github.com/rust-lang/rust/actions/runs/18955853581 |
rust-bors bot
added a commit
that referenced
this pull request
Oct 30, 2025
coercions reviews/misc whatevers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
S-waiting-on-perf
Status: Waiting on a perf run to be completed.
T-clippy
Relevant to the Clippy team.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
"remove normalize call"
Fixes #132765 in theory breaking as we might now treat some previously-ambig hr aliases as rigid when we shouldn't. we may want to explicitly normalize the coercion target in
fcx.coercelike the new solver does"leak check and lub for closure<->closure coerce-lubs"
Fixes rust-lang/trait-system-refactor-initiative#233
the
|x| x + 1expr has a type ofClosure(?31t)which we wind up inferring the RPIT to. TheCoerceManyret_coercionfor the wholepeculiartypeck has an expected type ofRPIT(unnormalized). When we type check thereturn |x| x + 1expr we go from the never type toClosure(?31t)which then participates in theret_coerciongiving us acoerce-lub(RPIT, Closure(?31t)).Normalizing
RPITgives us someClosure(?50t)where?31tand?50thave been unified with?31tas the root var.resolve_vars_if_possibledoesn't resolve infer vars to their roots so these wind up with different structural identities so the fast path doesn't apply and we fall back to coercing to afnptr. cc #147193 which also fixes thisNew solver probably just gets more inference variables here because canonicalization + generally different approach to normalization of opaques. Idk :3
This technically allows more code to compile (see the test using TAIT in the commit). I don't think this can be observed on stable though.
In theory this could break existing code that relied on coercing a closure to itself resulting in a fnptr. I don't expect this to really happen in practice and this is an "obvious" bug fix.
misc additional leak checks :)
We now leak check in a lot more places:
let a = b;will coerceb->abut if there's no coercion it just falls back to subtyping which is now leak checked.closure->fnptrnow leak checksfndef<->fndefnow leak checksfndef<->closureorclosure<->fndefnow leak checksFor 1 and 2 this should in theory only affect dead code as inside of livecode the borrow checker will wind up checking these constraints anyway. For 2 specifically it's worth noting that we already leak check
fndef->fnptrandfnptr->fnptrcoercions on stable.3 allows more code to compile. Previously by not leak checking we could wind up thinking two fndefs were equal and so avoid coercing to a fnptr. Then borrowck would wind up emitting an error due to incompatible binders. See
tests/ui/coercion/leak_check_fndef_lub.rswhich failed to compile before this PR.3 and 4 both stop some code from compiling in cases where the coerce-lub can only succeed due to
lubarbitrarily returning thelhsfor binders instead of a true "lub". See theorder_dependence_xtests intests/ui/coercion/leak_check_lub_to_fnptr.rs. Note that forfnptr<->fnptrcoerce-lubs we already leak check on stable preventing equivalent code from compilingsimilar to 1/2, both 3 and 4 should in theory also cause some code to stop compiling as we will be leak checking in dead code where borrowck is unable to emit errors.
TODO: I need to add some more tests here and properly link to them I think
"account for safe target features in fndef<->closure and fndef<->fndef coerce-lubs"
We previously did not take safe target features into account when creating the fn sig for fndefs during
fndef<->closureorfndef<->fndefcoerce-lubs. We now do allowingcoerce-lubto produce a safe fn pointer when fndefs with safe target features are involved. This is consistent with existing (non lub) coercion logic forfndef->fnptrwhich allows coercing to a safe fn pointer.r? ghost