-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Specialize try_destructure_mir_constant
for its sole user (pretty printing)
#113291
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
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @RalfJung (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
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.
We can't remove the query, as we need to invoke it from rustc_middle, but can only implement it in mir interpretation/const eval.
Ah, that is unfortunate, but it makes sense.
Can you change the name and docs of the query to reflect that it is to be used for diagnostic purposes only?
21c62f5
to
e1e04a8
Compare
This comment has been minimized.
This comment has been minimized.
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
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.
LGTM. Feel free to r=me, or get someone else's review as well.
(I assume we have tests ensuring that the printed output is not affected.)
no_hash | ||
eval_always |
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.
I have no idea what these do so I will just trust they make sense. ;)
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.
Mostly turning the query into a glorified function pointer (caching disabled). I think we have a better way nowadays, but I need to look it up
@bors r=RalfJung |
☀️ Test successful - checks-actions |
Finished benchmarking commit (bd8aabe): 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)This benchmark run did not return any relevant results for this metric. 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: 658.504s -> 657.658s (-0.13%) |
…lfJung Specialize `try_destructure_mir_constant` for its sole user (pretty printing) We can't remove the query, as we need to invoke it from rustc_middle, but can only implement it in mir interpretation/const eval. r? `@RalfJung` for a first round. While we could move all the logic into pretty printing, that would end up duplicating a bit of code with const eval, which doesn't seem great either.
We can't remove the query, as we need to invoke it from rustc_middle, but can only implement it in mir interpretation/const eval.
r? @RalfJung for a first round.
While we could move all the logic into pretty printing, that would end up duplicating a bit of code with const eval, which doesn't seem great either.