Skip to content

ACP: Negation functions for NonZeroI* #105

Closed

Description

@jmillikin

Proposal

Problem statement

Integer primitives have functions such as overflowing_neg() for numeric negation with controlled overflow. These functions are currently missing from the core::num::NonZeroI{8,16,32,64} types.

Motivation, use-cases

Non-zero integers are frequently used to represent error codes from low-level OS APIs, because these APIs use 0 to indicate non-error. Some APIs use signed error codes as a form of namespacing, such as internal/public (FreeBSD) or kernel/userspace (Linux).

For example, the FUSE protocol uses userspace error codes (positive non-zero) but negates them when sending an error frame to the kernel.

// old
let (neg, _) = err.get().overflowing_neg();
let wire_neg = unsafe { NonZeroI32::new_unchecked(neg) };

// new
let (wire_neg, _) = err.overflowing_neg();

Solution sketches

Since the primitive integer types already have *_neg() functions, and negation can't return zero for non-zero inputs, writing wrappers for the non-zero integers is straightforward.

pub const fn is_negative(self) -> bool {
    self.get().is_negative()
}

pub const fn checked_neg(self) -> Option<$Ty> {
    if let Some(result) = self.get().checked_neg() {
        return Some(unsafe { $Ty::new_unchecked(result) });
    }
    None
}

pub const fn overflowing_neg(self) -> ($Ty, bool) {
    let (result, overflow) = self.get().overflowing_neg();
    ((unsafe { $Ty::new_unchecked(result) }), overflow)
}

pub const fn saturating_neg(self) -> $Ty {
    if let Some(result) = self.checked_neg() {
        return result;
    }
    unsafe { $Ty::new_unchecked(<$Int>::MAX) }
}

pub const fn wrapping_neg(self) -> $Ty {
    let result = self.get().wrapping_neg();
    unsafe { $Ty::new_unchecked(result) }
}

Links and related work

rust-lang/rust#89065
rust-lang/rust#84186

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

Activity

  1. jmillikin commented on Sep 18, 2022

    @jmillikin
    Author

    Proposed implementation: jmillikin/upstream__rust@a807a2d

    Docs screenshots:

    nonzero-negation-1

    nonzero-negation-2

    nonzero-negation-3

    nonzero-negation-4

    nonzero-negation-5

  2. cuviper commented on Sep 18, 2022

    @cuviper
    Member

    How about trait Neg for the operator?

  3. jmillikin commented on Sep 18, 2022

    @jmillikin
    Author

    How about trait Neg for the operator?

    It's easy to implement, but my understanding is that trait impls for two stable types can't be marked unstable due to rust-lang/rust#55436.

    #[unstable(feature = "nonzero_negation_ops", issue = "none")]
    #[rustc_const_unstable(feature = "const_ops", issue = "90080")]
    impl const Neg for $Ty {
        type Output = Self;
    
        #[inline]
        fn neg(self) -> Self {
            unsafe { $Ty::new_unchecked(Neg::neg(self.get())) }
        }    
    }
    error: an `#[unstable]` annotation here has no effect
       --> library/core/src/num/nonzero.rs:877:13
    [...]
        = note: `#[deny(ineffective_unstable_trait_impl)]` on by default
        = note: see issue #55436 <https://github.com/rust-lang/rust/issues/55436> for more information
    
  4. thomcc commented on Sep 18, 2022

    @thomcc
    Member

    That's not a deal-breaker if the trait implementation is sensible/desirable.

  5. jmillikin commented on Sep 18, 2022

    @jmillikin
    Author

    I agree that Neg would be useful, have no strong opinion on the exact details of the stabilization process, and would be happy to do whatever the Rust libs team recommends in this case.

  6. scottmcm commented on Sep 26, 2022

    @scottmcm
    Member

    Given that the signed nonzero types already have things like checked_abs and overflowing_abs and saturating_mul and such, adding things like checked_neg and saturating_neg seem well covered by existing precedent, so if you want to send a PR for the methods as unstable, you can r? @scottmcm on that and I'll approve it. (They probably just weren't done originally because they don't make sense on the unsigned versions.)

    Trait impls can't go in unstable, though, so that at least should be a different PR, and I'll leave it for a real libs-api member to comment on it.

  7. jmillikin commented on Sep 27, 2022

    @jmillikin
    Author

    OK, I've filed PR rust-lang/rust#102341 to implement Neg for NonZeroI*.

    To avoid merge conflicts, I'll send a second PR for the negation functions once that first one lands.

  8. scottmcm commented on Sep 27, 2022

    @scottmcm
    Member

    Traits take at least 10 days for FCP, so that PR will take much longer to land than one for unstable inherent methods.

  9. jmillikin commented on Sep 27, 2022

    @jmillikin
    Author

    Traits take at least 10 days for FCP, so that PR will take much longer to land than one for unstable inherent methods.

    Interesting. OK, filed rust-lang/rust#102342 to add the new methods. Once one of them merges I'll rebase the second one.

  10. added a commit that references this issue on Sep 29, 2022
  11. added a commit that references this issue on Sep 29, 2022
  12. added a commit that references this issue on Apr 20, 2023
  13. added a commit that references this issue on May 4, 2023
  14. added a commit that references this issue on May 16, 2023
  15. added
    ACP-acceptedAPI Change Proposal is accepted (seconded with no objections)
    on Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ACP-acceptedAPI Change Proposal is accepted (seconded with no objections)api-change-proposalA proposal to add or alter unstable APIs in the standard libraries

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions