Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

std::num::Checked{Add,Sub,Mul,Div} should have type parameters #15

Closed
cuviper opened this issue Dec 19, 2017 · 6 comments
Closed

std::num::Checked{Add,Sub,Mul,Div} should have type parameters #15

cuviper opened this issue Dec 19, 2017 · 6 comments

Comments

@cuviper
Copy link
Member

cuviper commented Dec 19, 2017

From @steveklabnik on February 11, 2015 1:35

Issue by lifthrasiir
Monday Apr 14, 2014 at 12:33 GMT

For earlier discussion, see rust-lang/rust#13510

This issue was labelled with: A-libs in the Rust repository


The corresponding std::ops traits have both the type of right hand side operand and the type of result, while Checked* traits do not (and always assume that both operands and result have the same type). There is no real reason to make Checked* traits behave differently from std::ops traits, and having them as is hampers advanced uses of operator overloading: for example, if the scalar type (e.g. DateTime) and difference type (e.g. Duration) are distinct from each other then the current scheme wouldn't work at all.

Copied from original issue: rust-num/num#59

@cuviper
Copy link
Member Author

cuviper commented Dec 19, 2017

From @clarcharr on September 30, 2017 21:40

Adding onto this, there should be checked versions of Rem, Shl, and Shr as well.

@clarfonthey
Copy link
Contributor

clarfonthey commented Jan 10, 2018

IMHO these traits should also take things by value for consistency with the original traits. Object safety is already broken because they still rely on the original not-object-safe traits.

@cuviper
Copy link
Member Author

cuviper commented Mar 5, 2018

I've decided to limit the clutter of breaking change issues, so they will now be tracked collectively in #47. I'm closing this issue for now as postponed but we can still discuss its merits here as needed.

@cuviper cuviper closed this as completed Mar 5, 2018
@cuviper cuviper removed this from the num-traits-0.x milestone Mar 5, 2018
@regexident
Copy link
Contributor

regexident commented Mar 25, 2021

For sake of completeness it's worth mentioning that not just the Checked{Add,Sub,Mul} set of traits should have type parameters, but all of these, too:

  • Wrapping{Add,Sub,Mul}
  • Saturating{Add,Sub,Mul}
  • Overflowing{Add,Sub,Mul}

Same for Unchecked{Add,Sub,Mul}, should they get added to num-traits in the future.

@regexident
Copy link
Contributor

To add another motivation for adding type parameters consider vector-scaling arithmetic:

impl CheckedMul<Scalar> for Vector {
    fn checked_mul(&self, v: &Self) -> Option<Self> {
                           // ~~~~
                           // not possible as we would require `Scalar` here,
                           // not `Self` (problem caused by `Self: Mul<Self, Output = _>`)
                                                                       ~~~~
    }
}
impl CheckedMul<Vector> for Scalar {
    fn checked_mul(&self, v: &Self) -> Option<Self> {
                           // ~~~~            ~~~~
                           // not possible either as we would require `Vector` here,
                           // not `Self` (problem caused by `Self: Mul<Self, Output = Self>`)
                                                                       ~~~~           ~~~~
    }
}

@regexident
Copy link
Contributor

Since this issue doesn't actually mention what the desired re-definitions would actually look like I'd like to take the opportunity to provide one such recommendation here:

Checked{Add,Sub,Mul,Div}:

pub trait Checked…<Rhs = Self> {
    type Output;

    #[must_use]
    fn checked_(self, rhs: Rhs) -> Option<Self::Output>;
}

Unchecked{Add,Sub,Mul,Div}:

pub trait UncheckedAdd<Rhs = Self> {
    type Output;

    #[must_use]
    fn unchecked_add(self, rhs: Rhs) -> Self::Output;
}

Saturating{Add,Sub,Mul,Div}:

pub trait Saturating…<Rhs = Self> {
    type Output;

    #[must_use]
    fn saturating_(self, rhs: Rhs) -> Self::Output;
}

Wrapping{Add,Sub,Mul,Div}:

pub trait Wrapping…<Rhs = Self> {
    type Output;

    #[must_use]
    fn wrapping_(self, rhs: Rhs) -> Self::Output;
}

Overflowing{Add,Sub,Mul,Div}:

pub trait Overflowing…<Rhs = Self> {
    type Output;

    #[must_use]
    fn overflowing_(self, rhs: Rhs) -> (Self::Output, bool);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants