Skip to content

Tracking Issue for negation methods on NonZeroI* #102443

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
4 tasks done
jmillikin opened this issue Sep 28, 2022 · 8 comments · Fixed by #111044
Closed
4 tasks done

Tracking Issue for negation methods on NonZeroI* #102443

jmillikin opened this issue Sep 28, 2022 · 8 comments · Fixed by #111044
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jmillikin
Copy link
Contributor

jmillikin commented Sep 28, 2022

Feature gate: #![feature(nonzero_negation_ops)]

This is a tracking issue for adding negation methods such as wrapping_neg() to the core::num::NonZeroI{8,16,32,64,128} types.

ACP: rust-lang/libs-team#105

Public API

// core::num

for $Ty in NonZeroI{8,16,32,64,128} {
	pub const fn is_positive(self) -> bool;
	pub const fn is_negative(self) -> bool;
	pub const fn checked_neg(self) -> Option<$Ty>;
	pub const fn overflowing_neg(self) -> ($Ty, bool);
	pub const fn saturating_neg(self) -> $Ty;
	pub const fn wrapping_neg(self) -> $Ty;
}

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@jmillikin jmillikin added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 28, 2022
@jmillikin
Copy link
Contributor Author

(pinging someone from the libs team, per instructions at https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html)

@joshtriplett I'd like to start the FCP for this feature. The new methods have been in nightly since v1.66, and the the stabilization PR for impl Neg is now merged (#102341).

@jmillikin
Copy link
Contributor Author

jmillikin commented May 1, 2023

Timed out waiting for someone to start the FCP -- since there's not much to discuss regarding the API (it just copies the primitive types), I sent in PR #111044 to stabilize.

@BurntSushi
Copy link
Member

@rfcbot fcp merge

This is a small collection of negation methods on the NonZeroI{8,16,32,64,128} types. Specifically, the is_negative predicate, along with checked, overflowing, saturating and wrapping versions of negation with appropriate return types. These are exact copies of the corresponding methods on the i[8,16,32,64,128} types. Since negation of a non-zero value yields a non-zero value itself, these methods are overall pretty trivial. I don't see any obvious footguns.

@rfcbot
Copy link
Collaborator

rfcbot commented May 2, 2023

Team member @BurntSushi 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.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 2, 2023
@rfcbot
Copy link
Collaborator

rfcbot commented May 2, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@Robbepop
Copy link
Contributor

Robbepop commented May 4, 2023

Sorry for being a bit late to the show but I just found this via TWIR.

What is the rational for leaving out an is_positive method? I know it is trivial to emulate via !n.is_negative() where n: NonZeroIxx but ...

  • Rust primitive signed integers such as i32 do have both methods.
  • Using the aforementioned !n.is_negative() instead of n.is_positive() when needed can be considered bad style.

@jmillikin
Copy link
Contributor Author

Thanks for the catch! I just didn't see that is_positive() existed. Created PR #111186 to add it.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 12, 2023
@rfcbot
Copy link
Collaborator

rfcbot commented May 12, 2023

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.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label May 12, 2023
@bors bors closed this as completed in 76e79ca May 16, 2023
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label May 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants