Implement Neg for signed non-zero integers.#102341
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. Please see the contribution instructions for more information. |
|
@rustbot label +T-libs-api -T-libs |
library/core/tests/nonzero.rs
Outdated
There was a problem hiding this comment.
Please also add a test for overflow behavior: what happens if you negate the smallest possible value (e.g. NonZeroI64::new(i64::MIN))?
There was a problem hiding this comment.
Done.
The test asserts that for MIN and MAX, the behavior on overflow is the same as for the underlying primitve.
Edit: per discussion, replaced with a UI test to verify correct panic message on overflow.
e0bf824 to
fa7742f
Compare
|
@rustbot label -S-waiting-on-author +S-waiting-on-review |
|
That test is going to fail when overflow panics. I think you need to add a copy of |
a7bedfc to
60cf899
Compare
Ah, that's perfect, thanks! I got notified of the failing test, but was having trouble getting it to test with debug assertions locally. |
60cf899 to
2846afb
Compare
|
Gentle ping -- is the current state good enough to start the FCP, or is further work needed? |
|
What's the next step for me here? I assume there needs to be an FCP, but since there's no unstable version to merge I'm not sure what the trigger would be. |
|
@joshtriplett Sorry for the direct ping, but could you please let me know what I should be doing to move this PR along? |
|
r? libs-api (Also, there is a FIXME in |
2846afb to
7e8b265
Compare
|
@fee1-dead Thanks for pointing out the |
|
Can someone with appropriate permissions please push the merge button for this PR? |
|
@bors r+ |
|
📌 Commit 7e8b265a810090e29ccc4cd141949362a5680d98 has been approved by It is now in the queue for this repository. |
|
⌛ Testing commit 7e8b265a810090e29ccc4cd141949362a5680d98 with merge 797a2c85e5aee048fd3aae9a0213686f182f26aa... |
|
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
Negating a non-zero integer currently requires unpacking to a
primitive and re-wrapping. Since negation of non-zero signed
integers always produces a non-zero result, it is safe to
implement `Neg` for `NonZeroI{N}`.
The new `impl` is marked as stable because trait implementations
for two stable types can't be marked unstable.
7e8b265 to
4e2797d
Compare
|
There was a conflict with PR #110393 (removing const traits from libcore), merged 14 hours ago. I've rebased to current master HEAD. |
|
@bors r=dtolnay |
Rollup of 7 pull requests Successful merges: - rust-lang#102341 (Implement `Neg` for signed non-zero integers.) - rust-lang#110424 (Spelling misc) - rust-lang#110448 (cmp doc examples improvements) - rust-lang#110516 (bootstrap: Update linux-raw-sys to 0.3.2) - rust-lang#110548 (Make `impl Debug for Span` not panic on not having session globals.) - rust-lang#110554 (`deny(unsafe_op_in_unsafe_fn)` in `rustc_data_structures`) - rust-lang#110575 (fix lint regression in `non_upper_case_globals`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Negating a non-zero integer currently requires unpacking to a primitive and re-wrapping. Since negation of non-zero signed integers always produces a non-zero result, it is safe to implement
NegforNonZeroI{N}.The new
implis marked as stable because trait impls for two stable types can't be marked unstable.See discussion on rust-lang/libs-team#105 for additional context.