Implement `Neg` for signed non-zero integers. #102341

jmillikin · 2022-09-27T02:57:54Z

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 impls for two stable types can't be marked unstable.

See discussion on rust-lang/libs-team#105 for additional context.

rust-highfive · 2022-09-27T02:57:57Z

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.

jmillikin · 2022-09-27T03:19:03Z

@rustbot label +T-libs-api -T-libs

joshtriplett · 2022-10-03T11:50:34Z

library/core/tests/nonzero.rs

+
+    assert_eq!((-NonZeroI128::new(1).unwrap()).get(), -1);
+    assert_eq!((-NonZeroI128::new(-1).unwrap()).get(), 1);
+}


Please also add a test for overflow behavior: what happens if you negate the smallest possible value (e.g. NonZeroI64::new(i64::MIN))?

~~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.

jmillikin · 2022-10-03T12:25:46Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

joshtriplett · 2022-10-03T12:51:07Z

That test is going to fail when overflow panics.

I think you need to add a copy of src/test/ui/numbers-arithmetic/overflowing-neg.rs for a NonZero type, instead.

jmillikin · 2022-10-03T13:06:46Z

That test is going to fail when overflow panics.

I think you need to add a copy of src/test/ui/numbers-arithmetic/overflowing-neg.rs for a NonZero type, instead.

Ah, that's perfect, thanks! I got notified of the failing test, but was having trouble getting it to test with debug assertions locally.

jmillikin · 2022-10-06T02:42:12Z

Gentle ping -- is the current state good enough to start the FCP, or is further work needed?

jmillikin · 2022-10-18T10:19:29Z

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.

jmillikin · 2023-01-23T14:44:21Z

@joshtriplett Sorry for the direct ping, but could you please let me know what I should be doing to move this PR along?

fee1-dead · 2023-02-19T14:10:56Z

r? libs-api

(Also, there is a FIXME in core/src/num/nonzero.rs that should be removed if signed non-zero integers implement Neg)

jmillikin · 2023-02-21T00:22:32Z

@fee1-dead Thanks for pointing out the FIXME. I've updated my change to resolve it.

jmillikin · 2023-04-20T00:41:05Z

Can someone with appropriate permissions please push the merge button for this PR?

dtolnay · 2023-04-20T00:48:47Z

@bors r+

bors · 2023-04-20T00:48:50Z

📌 Commit 7e8b265a810090e29ccc4cd141949362a5680d98 has been approved by dtolnay

It is now in the queue for this repository.

bors · 2023-04-20T04:04:04Z

⌛ Testing commit 7e8b265a810090e29ccc4cd141949362a5680d98 with merge 797a2c85e5aee048fd3aae9a0213686f182f26aa...

bors · 2023-04-20T04:07:56Z

💔 Test failed - checks-actions

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.

jmillikin · 2023-04-20T05:33:35Z

There was a conflict with PR #110393 (removing const traits from libcore), merged 14 hours ago. I've rebased to current master HEAD.

workingjubilee · 2023-04-20T06:18:08Z

@bors r=dtolnay

bors · 2023-04-20T06:18:10Z

📌 Commit 4e2797d has been approved by dtolnay

It is now in the queue for this repository.

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

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 27, 2022

rust-highfive assigned joshtriplett Sep 27, 2022

This comment was marked as resolved.

Sign in to view

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2022

jmillikin mentioned this pull request Sep 27, 2022

ACP: Negation functions for NonZeroI* rust-lang/libs-team#105

Closed

rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 27, 2022

scottmcm added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Sep 30, 2022

joshtriplett reviewed Oct 3, 2022

View reviewed changes

joshtriplett added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2022

jmillikin force-pushed the nonzero-impl-neg branch from e0bf824 to fa7742f Compare October 3, 2022 12:24

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 3, 2022

jmillikin force-pushed the nonzero-impl-neg branch 2 times, most recently from a7bedfc to 60cf899 Compare October 3, 2022 13:05

jmillikin force-pushed the nonzero-impl-neg branch from 60cf899 to 2846afb Compare October 3, 2022 13:29

rustbot assigned m-ou-se and unassigned joshtriplett Feb 19, 2023

jmillikin force-pushed the nonzero-impl-neg branch from 2846afb to 7e8b265 Compare February 20, 2023 23:45

dtolnay assigned dtolnay and unassigned m-ou-se Apr 20, 2023

dtolnay approved these changes Apr 20, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 20, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 20, 2023

This comment has been minimized.

Sign in to view

jmillikin force-pushed the nonzero-impl-neg branch from 7e8b265 to 4e2797d Compare April 20, 2023 05:28

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 20, 2023

JohnTitor mentioned this pull request Apr 20, 2023

Rollup of 7 pull requests #110585

Merged

bors merged commit df3aca2 into rust-lang:master Apr 20, 2023

rustbot added this to the 1.71.0 milestone Apr 20, 2023

jmillikin deleted the nonzero-impl-neg branch April 20, 2023 12:49

jmillikin mentioned this pull request Apr 20, 2023

Tracking Issue for negation methods on NonZeroI* #102443

Closed

4 tasks

apiraino removed the to-announce Announce this issue on triage meeting label Apr 20, 2023

jmillikin mentioned this pull request May 5, 2023

impl Neg for NonZeroI* #97907

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `Neg` for signed non-zero integers. #102341

Implement `Neg` for signed non-zero integers. #102341

jmillikin commented Sep 27, 2022

This comment was marked as resolved.

rust-highfive commented Sep 27, 2022

jmillikin commented Sep 27, 2022

joshtriplett Oct 3, 2022

jmillikin Oct 3, 2022 •

edited

Loading

jmillikin commented Oct 3, 2022

joshtriplett commented Oct 3, 2022

jmillikin commented Oct 3, 2022

jmillikin commented Oct 6, 2022

jmillikin commented Oct 18, 2022

jmillikin commented Jan 23, 2023

fee1-dead commented Feb 19, 2023 •

edited

Loading

jmillikin commented Feb 21, 2023

jmillikin commented Apr 20, 2023

dtolnay commented Apr 20, 2023

bors commented Apr 20, 2023

bors commented Apr 20, 2023

bors commented Apr 20, 2023

This comment has been minimized.

jmillikin commented Apr 20, 2023

workingjubilee commented Apr 20, 2023

bors commented Apr 20, 2023

Implement Neg for signed non-zero integers. #102341

Implement Neg for signed non-zero integers. #102341

Conversation

jmillikin commented Sep 27, 2022

This comment was marked as resolved.

rust-highfive commented Sep 27, 2022

jmillikin commented Sep 27, 2022

joshtriplett Oct 3, 2022

Choose a reason for hiding this comment

jmillikin Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

jmillikin commented Oct 3, 2022

joshtriplett commented Oct 3, 2022

jmillikin commented Oct 3, 2022

jmillikin commented Oct 6, 2022

jmillikin commented Oct 18, 2022

jmillikin commented Jan 23, 2023

fee1-dead commented Feb 19, 2023 • edited Loading

jmillikin commented Feb 21, 2023

jmillikin commented Apr 20, 2023

dtolnay commented Apr 20, 2023

bors commented Apr 20, 2023

bors commented Apr 20, 2023

bors commented Apr 20, 2023

This comment has been minimized.

jmillikin commented Apr 20, 2023

workingjubilee commented Apr 20, 2023

bors commented Apr 20, 2023

Implement `Neg` for signed non-zero integers. #102341

Implement `Neg` for signed non-zero integers. #102341

jmillikin Oct 3, 2022 •

edited

Loading

fee1-dead commented Feb 19, 2023 •

edited

Loading