-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
only assemble alias bound candidates for rigid aliases #119744
Conversation
This comment has been minimized.
This comment has been minimized.
This PR ensures that the code I extracted from gimli works:
Plz add it as a test, whether you want to minimize it further or not is up to you. This has to do with taking |
@rustbot author until it's ready for review |
523abe3
to
f005f7a
Compare
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
always normalize `LoweredTy` in the new solver I currently expect us to stop using alias bound candidates of normalizable aliases due to rust-lang/trait-system-refactor-initiative#77 by landing rust-lang#119744. At this point it mostly doesn't matter whether we eagerly normalize (and replace with infer vars in case of ambiguity). cc rust-lang#113473 previous attempt The infer var replacement for ambiguous projections can in very rare cases: - weaken inference rust-lang/trait-system-refactor-initiative#81 - strengthen inference rust-lang/trait-system-refactor-initiative#7 I do not expect this impact on inference to significantly affect real crates. r? `@compiler-errors`
always normalize `LoweredTy` in the new solver I currently expect us to stop using alias bound candidates of normalizable aliases due to rust-lang/trait-system-refactor-initiative#77 by landing rust-lang#119744. At this point it mostly doesn't matter whether we eagerly normalize (and replace with infer vars in case of ambiguity). cc rust-lang#113473 previous attempt The infer var replacement for ambiguous projections can in very rare cases: - weaken inference rust-lang/trait-system-refactor-initiative#81 - strengthen inference rust-lang/trait-system-refactor-initiative#7 I do not expect this impact on inference to significantly affect real crates. r? ``@compiler-errors``
Rollup merge of rust-lang#120378 - lcnr:normalize-ast, r=compiler-errors always normalize `LoweredTy` in the new solver I currently expect us to stop using alias bound candidates of normalizable aliases due to rust-lang/trait-system-refactor-initiative#77 by landing rust-lang#119744. At this point it mostly doesn't matter whether we eagerly normalize (and replace with infer vars in case of ambiguity). cc rust-lang#113473 previous attempt The infer var replacement for ambiguous projections can in very rare cases: - weaken inference rust-lang/trait-system-refactor-initiative#81 - strengthen inference rust-lang/trait-system-refactor-initiative#7 I do not expect this impact on inference to significantly affect real crates. r? ``@compiler-errors``
☔ The latest upstream changes (presumably #120401) made this pull request unmergeable. Please resolve the merge conflicts. |
f005f7a
to
7f09bb6
Compare
7f09bb6
to
ea4e5b8
Compare
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (c401f09): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 662.576s -> 660.86s (-0.26%) |
fixes rust-lang/trait-system-refactor-initiative#77
This also causes
<Wrapper<?0> as Trait>::Unwrap: Trait
to always be ambig, as we now normalize the self type before checking whether it is an inference variable.I cannot think of an approach to the underlying issues here which does not require the "may-define means must-define" restriction for opaque types. Going to go ahead with this and added this restriction to the tracking issue for the new solver to make sure we don't stabilize it without getting types + lang signoff here.
r? @compiler-errors