rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.5k stars 12.74k forks source link

Implement From<T> for Wrapping<T> and From<Wrapping<T>> for T #44937

Closed sportzer closed 6 years ago

sportzer commented 7 years ago

It would be nice if std::num::Wrapping supported From<T> and Into<T>, at least when T is a numeric primitive. I assumed these impls would exist and was surprised to find that they do not.

ketsuban commented 7 years ago

It doesn't explicitly list From, but I suspect this is at least spiritually a duplicate of #32463.

dtolnay commented 6 years ago

Can you point to an example of real code where these impls would be useful?

ketsuban commented 6 years ago

Any real code that uses Wrapping<T> benefits from it being more transparently like the underlying T, including From impls, because the purpose of Wrapping<T> and friends is being explicit about desired overflow behaviour in an ergonomic way while retaining T behaviour.

As a personal example, I dabbled with writing a Game Boy Advance emulator a while back; Wrapping<T> was the correct type for register values and similar, because I knew what the desired behaviour was and didn't need warnings about overflow in debug builds, but (if I recall correctly) the lack of implementations of common traits and methods like rotate_left made it less ergonomic than it could have been.

dtolnay commented 6 years ago

Sure, I agree that rotate_left and stuff would be good. That is tracked in #32463. Do you have an example from real code where converting T -> Wrapping\ or Wrapping\ to T using a From impl would have been preferable to not using a From impl?

dtolnay commented 6 years ago

I am going to close this for now. I would be happy to consider an RFC if someone finds concrete use cases that would benefit from From impls.

sportzer commented 6 years ago

I guess for me this was more of an initial impression sort of thing. Using wrapping.into() seemed like a more intuitive way of converting back into a T, although wrapping.0 wasn't exactly difficult to discover once I'd worked out that T doesn't implement From<Wrapping<T>>.

I think part of the problem was that when interacting with the standard library, syntax like Wrapping(0) is more commonly used for enum variants like Some, Ok, or Err, so being able to get the value back out via field access isn't as obvious as you might expect. The fact that wrapping.0 works also feels more like an implementation detail, albeit one which is part of the public API, but YMMV.

TheHashTableSlasher commented 6 years ago

Can you point to an example of real code where these impls would be useful?

Making porting any RNG not the worst experience ever, for starters. Or something like cyclic redundancy checks, which assumes twos-complement arithmetic and depends on a memoized array of coefficients. Basically any code where placing Wrapping() around every integer constant is tedious at best, and destructive of code transparency at worst.

dtolnay commented 6 years ago

@myconix could you link to real code?

TheHashTableSlasher commented 6 years ago

Something like http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/CODES/mt19937ar.c or https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp

TheHashTableSlasher commented 6 years ago

https://github.com/libtom/libtomcrypt/blob/develop/src/hashes/md5.c for a less trivial example

dtolnay commented 6 years ago

I am trying to see concretely what improvement there would be.

TheHashTableSlasher commented 6 years ago

Right now, Wrapping doesn't allow for operating with plain integers, even literals, That, combined with the fact that there's no succinct way to specify a Wrapping literal, means that EVERY integer literal in those code snippets would need a Wrapping() constructor around it. That's messy and time-consuming, in the very best case.

dtolnay commented 5 years ago

@myconix could you demonstrate a snippet of code ported from any one of those links into Rust,

TheHashTableSlasher commented 5 years ago

I can't do that in a reasonable time frame. The quicker explanation is that they can all be reduced to:

use std::num::Wrapping;

fn main() {
    let mut x = Wrapping(3);

    x *= 4;

    println!("{}", x);
}

Giving an error at compile time. To get this to work, you need to explicitly make the 4 wrapping as well:

use std::num::Wrapping;

fn main() {
    let mut x = Wrapping(3);

    x *= Wrapping(4);

    println!("{}", x);
}

This is a trivial example, but if I wanted to implement one of those examples in Rust, this problem gets worse proportional to the number of ops in the code.

dtolnay commented 5 years ago

@myconix I am going to continue to insist on seeing:

because I really don't believe that adding these impls would improve the use cases you are writing about.

I've locked the thread because I don't see the discussion here moving in a productive direction, but I would be willing to consider an RFC (however brief) that lays out concrete advantages of impl From<T> for Wrapping<T> and/or impl From<Wrapping<T>> for T.

If anyone is interested in spearheading an RFC, make sure to include: