-
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
Add {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}{,Assign}<$t> to Saturat… #92356
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
3ebfaa5
to
1c0dc18
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 1c0dc18 with merge 500ba85ad4d49bc3023225770346fae0ddfabab6... |
☀️ Try build successful - checks-actions |
Queued 500ba85ad4d49bc3023225770346fae0ddfabab6 with parent 5883b87, future comparison URL. |
Finished benchmarking commit (500ba85ad4d49bc3023225770346fae0ddfabab6): comparison url. Summary: This change led to large relevant regressions 😿 in compiler performance.
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. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led 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 |
Regression is in doc builds, which is probably to be expected -- rustdoc is dependent on the whole all trait impls for each compilation rather than anything more local today, and this is adding a lot of trait impls. r? @dtolnay as this is a fairly extensive expansion of library surface area. The impl itself seems OK, I think, but could use another set of eyes to to confirm. I felt particularly unsure about the Rem impl (I think it does the right thing though). |
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 am on board with this. Thanks!
@bors r+ |
📌 Commit 1c0dc18 has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (26c06cf): comparison url. Summary: This change led to large relevant regressions 😿 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
@kellerkindt for next time: When you're changing/expanding an existing unstable feature (like |
Alright. I wasn't sure whether the stabilization of Thank you for the info. |
…l, r=m-ou-se Impl {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}Assign<$t> for Wrapping<$t> for rust 1.60.0 Tracking issue rust-lang#93204 This is about adding basic integer operations to the `Wrapping` type: ```rust let mut value = Wrapping(2u8); value += 3u8; value -= 1u8; value *= 2u8; value /= 2u8; value %= 2u8; value ^= 255u8; value |= 123u8; value &= 2u8; ``` Because this adds stable impls on a stable type, it runs into the following issue if an `#[unstable(...)]` attribute is used: ``` an `#[unstable]` annotation here has no effect note: see issue rust-lang#55436 <rust-lang#55436> for more information ``` This means - if I understood this correctly - the new impls have to be stabilized instantly. Which in turn means, this PR has to kick of an FCP on the tracking issue as well? This impl is analog to 1c0dc18 rust-lang#92356 for the `Saturating` type `@dtolnay` `@Mark-Simulacrum`
…l, r=m-ou-se Impl {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}Assign<$t> for Wrapping<$t> for rust 1.60.0 Tracking issue rust-lang#93204 This is about adding basic integer operations to the `Wrapping` type: ```rust let mut value = Wrapping(2u8); value += 3u8; value -= 1u8; value *= 2u8; value /= 2u8; value %= 2u8; value ^= 255u8; value |= 123u8; value &= 2u8; ``` Because this adds stable impls on a stable type, it runs into the following issue if an `#[unstable(...)]` attribute is used: ``` an `#[unstable]` annotation here has no effect note: see issue rust-lang#55436 <rust-lang#55436> for more information ``` This means - if I understood this correctly - the new impls have to be stabilized instantly. Which in turn means, this PR has to kick of an FCP on the tracking issue as well? This impl is analog to 1c0dc18 rust-lang#92356 for the `Saturating` type ``@dtolnay`` ``@Mark-Simulacrum``
Tracking issue #92354