rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

Add Wrapping/Saturating::from_mut reference conversions #327

Open afetisov opened 6 months ago

afetisov commented 6 months ago

Proposal

Problem statement

The Wrapping and Saturating types have stable layout, defined as equivalent to the inner public numeric type. This means that it's easy to convert owned values of T and Wrapping<T>. However, conversion of references (&/&mut) is also sound, since the wrapper imposes no extra invariants on its contents, but it's impossible to do safely. The stable solution is to do mem::transmute::<&mut Wrapping<T>, &mut T>(ref_mut), which is unsafe and a mouthful to write.

Motivating examples or use cases

The primary motivation is to be able to use op-assign operators with wrapping/saturating semantics.

In my experience using &Wrapping<T>/&mut Wrapping<T> in API is extremely uncommon (personally I struggle to think of any examples beyond the implementations of general traits, like PartialEq or AddAssign, for those types). In my view, this is due to a combination of issues:

For this reason I think that conversions &Wrapping<T> <=> &T (similarly for &mut and Saturating) are typically unnecessary. A primary counterexample is op-assign operators (+=, -= etc). Currently it is impossible to easily use them to implement wrapping/saturating operations on integer types. The wrapping analogue of a += b; currently looks like

a = a.wrapping_add(b);

It's more verbose, since it requires to repeat the expression a twice (note that it could be some complex place expression, like *foo.bar[5].baz()). It's also more error-prone, since the place expression may contain operators and function calls (e.g. indexing and .baz() in the previous example), and there is no guarantee that those operations are cheap or idempotent for the relevant types.

With this proposal, the wrapping/saturating op-assign operators could be implemented as simply as

*Wrapping::from_mut(&mut a) += b;

without introducing any new traits, or code duplication.

The above example is particularly common when translating C/C++ code into Rust, since all operations on unsigned integers in those languages have wrapping semantics.

One of the benefits of the proposed approach is that the use of wrapping operations on ordinary integer types is obvious at use site (due to the presence of Wrapping::from_mut), and it is easy to refactor this code into a non-wrapping arithmetic operation if need be.

Solution sketch

Add the following functions:

impl<T> Wrapping<T> {
    /// Converts the referent from `T` to `Wrapping<T>`.
    ///
    /// This allows to pass `&mut T` where `&mut Wrapping<T>` is expected, and allows to
    /// use wrapping operations on `T` with the natural operator syntax.
    ///
    /// # Example
    /// ```
    /// # use core::num::Wrapping;
    /// let mut val = 0_usize;
    /// *Wrapping::from_mut(&mut val) -= 1;
    /// assert_eq!(val, usize::MAX);
    /// ```
    pub fn from_mut(r: &mut T) -> &mut Wrapping<T> {
        // SAFETY: the layouts of `T` and `Wrapping<T>` are defined as equivalent,
        //      and `Wrapping<T>` imposes no extra invariants on its contents.
        unsafe { mem::transmute(r) }
    }
}

impl<T> AsMut<T> for Wrapping<T> {
    fn as_mut(&mut self) -> &mut T {
        // SAFETY: the layouts of `T` and `Wrapping<T>` are defined as equivalent,
        //      and `Wrapping<T>` imposes no extra invariants on its contents.
        unsafe { mem::transmute(self) }
    }
}

impl<T> Saturating<T> {
    /// Converts the referent from `T` to `Saturating<T>`.
    ///
    /// This allows to pass `&mut T` where `&mut Saturating<T>` is expected, and allows to
    /// use saturating operations on `T` with the natural operator syntax.
    ///
    /// # Example
    /// ```
    /// # use core::num::Saturating;
    /// let mut val = 0_usize;
    /// *Saturating::from_mut(&mut val) -= 1;
    /// assert_eq!(val, 0);
    /// ```
    pub fn from_mut(r: &mut T) -> &mut Saturating<T> {
        // SAFETY: the layouts of `T` and `Saturating<T>` are defined as equivalent,
        //      and `Saturating<T>` imposes no extra invariants on its contents.
        unsafe { mem::transmute(r) }
    }
}

impl<T> AsMut<T> for Saturating<T> {
    fn as_mut(&mut self) -> &mut T {
        // SAFETY: the layouts of `T` and `Saturating<T>` are defined as equivalent,
        //      and `Saturating<T>` imposes no extra invariants on its contents.
        unsafe { mem::transmute(self) }
    }
}

As seen above, I also propose to add the inverse conversions &mut Wrapping<T>, &mut Saturating<T> => &mut T. We could add it as a separate method on the types, but the signature and semantics of those methods fit the existing AsMut trait perfectly, so imho it's best to use it. The API to remove the wrapper is mostly included for completeness. If one works with wrapping types instead of raw integers, it allows to easily apply non-wrapping ordinary (potentially panicking) operations to the contents. I don't think it's a common need, foremost because using the wrappers is itself not super common, but it's nice to have the symmetry.

Note that I do not propose to add the matching API for conversions &T <=> &Wrapping<T>. We could add them with similar implementation and reasoning, using the AsRef trait for the inverse conversion. However, I don't know of a good use case for those APIs, since the operator traits don't use them, and the PartialOrd, PartialEq etc traits have the same behaviour for both the wrappers and the wrapped types. Since we can always add those methods later if need be, I omit them from this proposal, but I'm open to add them if anyone has a use case where by-value operations on dereferenced expressions are insufficient.

Alternatives

Given the already existing similar methods on atomic types(unstable), UnsafeCell(unstable), arrays and slices, there is already prior art for this kind of conversions, which makes them natural to expect for the users, and thus more discoverable.

Links and related work

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution: