-
Notifications
You must be signed in to change notification settings - Fork 13.7k
implement Sum and Product for Saturating(u*) #144275
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
Could you add some tests to library/coretests/tests/iter/traits/accum.rs verifying the behavior? @rustbot label +I-libs-api-nominated |
FCP since this is insta-stable. @rfcbot merge |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
This comment has been minimized.
This comment has been minimized.
e37610c
to
d8d1462
Compare
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
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 with one nit and a squash
Cc @joshtriplett, in rust-lang/libs-team#604 (comment) you said:
We can settle that question in the FCP that we'll need for the PR (which will be insta-stable). Meanwhile, please go ahead with a PR adding this for unsigned Saturating types, and mark it as needs-fcp. Thank you!
This wasn't explicitly mentioned on this thread (outside of the8472's docs request), so I'm just clarifying here that the current implementation does not short circuit (which seems reasonable).
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
209c296
to
1a33d62
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Thank you! @bors r+ |
@bors rollup |
Rollup of 9 pull requests Successful merges: - #143713 (Add a mailmap entry for gnzlbg) - #144275 (implement Sum and Product for Saturating(u*)) - #144354 (fix(std): Fix undefined reference to __my_thread_exit on QNX 8.0) - #145387 (Remove TmpLayout in layout_of_enum) - #145793 (std library: use execinfo library also on NetBSD.) - #145884 (Test `instrument-mcount` codegen) - #145947 (Add more to the `[workspace.dependencies]` section in the top-level `Cargo.toml`) - #145972 (fix `core::marker::Destruct` doc) - #145977 (tests: Ignore basic-stepping.rs on riscv64) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144275 - Qelxiros:saturating-arithmetic, r=tgross35 implement Sum and Product for Saturating(u*) ACP: rust-lang/libs-team#604 `@rustbot` label +needs-fcp
Already merged. |
ACP: rust-lang/libs-team#604
@rustbot label +needs-fcp