-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Stabilize if_let_rescope #131984
Stabilize if_let_rescope #131984
Conversation
@rustbot labels +A-edition-2024 |
This comment has been minimized.
This comment has been minimized.
@@ -244,6 +244,8 @@ declare_features! ( | |||
(accepted, irrefutable_let_patterns, "1.33.0", Some(44495)), | |||
/// Allows `#[instruction_set(_)]` attribute. | |||
(accepted, isa_attribute, "1.67.0", Some(74727)), | |||
/// Rescoping temporaries in `if let` to align with Rust 2024. | |||
(accepted, if_let_rescope, "1.83.0", Some(124085)), |
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.
Aside from being sorted, this should also probably use CURRENT_RUSTC_VERSION
.
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.
Applied
r? @lcnr Given that you had a look at the related... ...it may make sense for you to have a look at this also. |
8daa523
to
807eb2f
Compare
☔ The latest upstream changes (presumably #132027) made this pull request unmergeable. Please resolve the merge conflicts. |
807eb2f
to
6d569f7
Compare
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.
r=me on the impl, not gonna actually approve this myself while I am arguing about the current design in #131154 :>
@bors r=traviscross,lcnr |
…rescope, r=traviscross,lcnr Stabilize if_let_rescope Close rust-lang#131154 Tracked by rust-lang#124085
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#131375 (compiler: apply clippy::clone_on_ref_ptr for CI) - rust-lang#131984 (Stabilize if_let_rescope) - rust-lang#132151 (Ensure that resume arg outlives region bound for coroutines) - rust-lang#132161 ([StableMIR] A few fixes to pretty printing) - rust-lang#132194 (Collect item bounds for RPITITs from trait where clauses just like associated types) - rust-lang#132233 (Split `boxed.rs` into a few modules) - rust-lang#132270 (clarified doc for `std::fs::OpenOptions.truncate()`) - rust-lang#132284 (Remove my ping for rustdoc/clean/types.rs) r? `@ghost` `@rustbot` modify labels: rollup
…rescope, r=traviscross,lcnr Stabilize if_let_rescope Close rust-lang#131154 Tracked by rust-lang#124085
…rescope, r=traviscross,lcnr Stabilize if_let_rescope Close rust-lang#131154 Tracked by rust-lang#124085
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#131984 (Stabilize if_let_rescope) - rust-lang#132151 (Ensure that resume arg outlives region bound for coroutines) - rust-lang#132157 (Remove detail from label/note that is already available in other note) - rust-lang#132274 (Cleanup op lookup in HIR typeck) - rust-lang#132319 (cg_llvm: Clean up FFI calls for setting module flags) - rust-lang#132321 (xous: sync: remove `rustc_const_stable` attribute on Condvar and Mutex new()) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#131984 - dingxiangfei2009:stabilize-if-let-rescope, r=traviscross,lcnr Stabilize if_let_rescope Close rust-lang#131154 Tracked by rust-lang#124085
@rust-timer build 701efc4 Checking if this might be the source of the regression in #132326. |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (701efc4): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -2.9%, secondary -1.8%)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.
CyclesResults (secondary 2.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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 784.999s -> 785.94s (0.12%) |
Yeah so this is quite the regression on instruction counts. Doesn't look so terrible on cycles though. The detailed results show the |
Close #131154
Tracked by #124085