-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
new solver normalization improvements #117278
Conversation
275f894
to
2b2c4af
Compare
This comment has been minimized.
This comment has been minimized.
8315095
to
c2c6400
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…er-errors prepopulate opaque ty storage before using it doesn't have any significant impact rn afaict, as we freely define new opaque types during MIR typeck. It will be relevant with rust-lang#117278 and once we stop allowing the definition of new opaques in MIR typeck r? `@compiler-errors`
Rollup merge of rust-lang#117439 - lcnr:prepopulate-earlier, r=compiler-errors prepopulate opaque ty storage before using it doesn't have any significant impact rn afaict, as we freely define new opaque types during MIR typeck. It will be relevant with rust-lang#117278 and once we stop allowing the definition of new opaques in MIR typeck r? `@compiler-errors`
☔ The latest upstream changes (presumably #117459) made this pull request unmergeable. Please resolve the merge conflicts. |
e4a289b
to
f5f5ae0
Compare
This comment has been minimized.
This comment has been minimized.
f5f5ae0
to
51debd6
Compare
This comment has been minimized.
This comment has been minimized.
still draft so marking author so it gets out of my review queue 😆 @rustbot author |
d65c990
to
d8b7f24
Compare
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
d8b7f24
to
8ed725a
Compare
8ed725a
to
fce71ad
Compare
@rustbot ready |
🆒 @bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (78efca8): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
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.
CyclesResultsThis 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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 675.386s -> 676.982s (0.24%) |
Note for next week's triage if it comes up in the results: |
Add some comments, add `can_define_opaque_ty` check to `try_normalize_ty_recur` Follow-up from rust-lang#117278, since I was recently re-reviewing this code.
Rollup merge of rust-lang#118915 - compiler-errors:alias-nits, r=lcnr Add some comments, add `can_define_opaque_ty` check to `try_normalize_ty_recur` Follow-up from rust-lang#117278, since I was recently re-reviewing this code.
cool beans
At the core of this PR is a
try_normalize_ty
which stops for rigid aliases by usingcommit_if_ok
.Reworks alias-relate to fully normalize both the lhs and rhs and then equate the resulting rigid (or inference) types. This fixes rust-lang/trait-system-refactor-initiative#68 by avoiding the exponential blowup. Also supersedes #116369 by only defining opaque types if the hidden type is rigid.
I removed the stability check in
EvalCtxt::evaluate_goal
due to rust-lang/trait-system-refactor-initiative#75. While I personally have opinions on how to fix it, that still requires further t-types/@nikomatsakis buy-in, so I removed that for now. Once we've decided on our approach there, we can revert this commit.r? @compiler-errors