-
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
Use box syntax everywhere #87804
Use box syntax everywhere #87804
Conversation
Result of running: rg -l "Box::new" compiler/ | rg '.rs$' | rg -v 'cranelift' | xargs sed -i 's/Box::new(/box (/'
Some changes occured to the CTFE / Miri engine cc @rust-lang/miri |
(rust-highfive has picked a reviewer for you, use r? to override) |
@bors try |
⌛ Trying commit cbd8fcd with merge 69c94e269ddbe56fb5388e19bcd469b6500fa5ea... |
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.
4818909
to
d5d2fb6
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit d5d2fb6 with merge 97d3a002940f685cc3fc98436ceaaa36eb1fb802... |
☀️ Try build successful - checks-actions |
Queued 97d3a002940f685cc3fc98436ceaaa36eb1fb802 with parent 1f94abc, future comparison URL. |
@@ -49,7 +49,7 @@ macro_rules! err_exhaust { | |||
#[macro_export] | |||
macro_rules! err_machine_stop { | |||
($($tt:tt)*) => { | |||
$crate::mir::interpret::InterpError::MachineStop(Box::new($($tt)*)) | |||
$crate::mir::interpret::InterpError::MachineStop(box ($($tt)*)) |
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 will mean other crates using the macro (such as standalone Miri) need to also enable the box
feature, right? I think I'd prefer to avoid that.
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.
There exists #[allow_internal_unstable]
to prevent that.
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.
Example macro that does this for box syntax is the vec macro. As @petrochenkov correctly assumed, this PR was a test run though to explore the implications of using box syntax everywhere.
Finished benchmarking try commit (97d3a002940f685cc3fc98436ceaaa36eb1fb802): comparison url. Summary: This benchmark run did not return any significant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. @bors rollup=never |
This is a slight regression of up to 0.3%. No improvements. |
I assume this was opened only to measure performance, without intent to actually merge the changes, so closing. |
Alternative PR to #87781 .
As suggested by @leonardo-m :#87781 (comment)