-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Continue to get rid of ty::Const::{try_}eval*
#130950
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
Conversation
Some changes occurred in src/tools/clippy
cc @rust-lang/clippy
Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.
Some changes occurred to MIR optimizations
cc @rust-lang/wg-mir-opt
Some changes occurred to the CTFE / Miri interpreter
cc @rust-lang/miri
Some changes occurred in compiler/rustc_codegen_gcc
Some changes occurred in compiler/rustc_codegen_cranelift
cc @bjorn3
Some changes occurred in compiler/rustc_sanitizers
cc @rust-lang/project-exploit-mitigations, @rcvalle
The Miri subtree was changed
cc @rust-lang/miri
try::Const::{try_}eval* (追記ここまで)
This comment has been minimized.
This comment has been minimized.
ah debuginfo
627a151 to
98b5f4f
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This possibly may need to be normalized w/ lazy norm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this so we can structurally normalize a const in consider_builtin_transmute_candidate. That structurally matches on an Assume struct which contains a bunch of bools for configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best reviewed without whitespace. We added a probe so we can normalize below without side-effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was sketchy. If a const was failing to eval, we just returned "whatever". 🤔
try::Const::{try_}eval* (削除ここまで)ty::Const::{try_}eval* (追記ここまで)
@rustbot author
some doctest is failing fml
☔ The latest upstream changes (presumably #131036) made this pull request unmergeable. Please resolve the merge conflicts.
98b5f4f to
66760f4
Compare
@rustbot ready
☔ The latest upstream changes (presumably #131892) made this pull request unmergeable. Please resolve the merge conflicts.
I resolved the large threads since they were making it hard to review the actual code changes themselves and it also seemed like @RalfJung had a better understanding of what's going on now? Apologies if this was over-eager ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you :3 Looks good to me, sorry for taking so long to review. Once you resolve the nit, rebase, and maybe squash some of these commits? r=me
66760f4 to
5cf8107
Compare
@bors r+ rollup=never
☀️ Test successful - checks-actions
Approved by: BoxyUwU
Pushing f2ba411 to master...
|
Finished benchmarking commit (f2ba411): 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)Results (secondary -1.2%)This 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: 781.459s -> 781.789s (0.04%) |
Uh oh!
There was an error while loading. Please reload this page.
This PR mostly does:
try_eval_*andeval_*helpers fromty::Const, and replace their usages withtry_to_*.ty::Const::eval.ty::Const::normalizetoty::Const::normalize_internal. This function is still used in the normalization code itself.TransmuteFromgoal.I'm happy to split it out further; for example, I could probably land the first part which removes the helpers, or the changes to codegen which are more obvious than the changes to tools.
r? BoxyUwU
Part of #130704