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

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

Closed
sportzer opened this issue Sep 30, 2017 · 15 comments
Closed

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

sportzer opened this issue Sep 30, 2017 · 15 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@sportzer
Copy link

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
Copy link
Contributor

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

@TimNN TimNN added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 1, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 17, 2017

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

@ketsuban
Copy link
Contributor

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
Copy link
Member

dtolnay commented Nov 17, 2017

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<T> or Wrapping<T> to T using a From impl would have been preferable to not using a From impl?

@dtolnay
Copy link
Member

dtolnay commented Nov 19, 2017

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.

@dtolnay dtolnay closed this as completed Nov 19, 2017
@sportzer
Copy link
Author

sportzer commented Nov 19, 2017

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
Copy link

TheHashTableSlasher commented Nov 16, 2018

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
Copy link
Member

dtolnay commented Nov 16, 2018

@myconix could you link to real code?

@TheHashTableSlasher
Copy link

@dtolnay
Copy link
Member

dtolnay commented Nov 16, 2018

  • If you were porting any of that code today without From impls, what would it look like?
  • If we were to add From impls then what would the ported code look like?

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

@TheHashTableSlasher
Copy link

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
Copy link
Member

dtolnay commented Nov 16, 2018

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

  • not using From impls, and then
  • using at least one of the From impls requested in this issue?

@TheHashTableSlasher
Copy link

TheHashTableSlasher commented Nov 16, 2018

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
Copy link
Member

dtolnay commented Nov 16, 2018

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

  • real Rust code that compiles today without From impls, and
  • the same code written using the From impls requested in the subject of this issue

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:

  • real Rust code that compiles today without From impls, and
  • the same code written using the requested From impls, demonstrating the improvement.

@rust-lang rust-lang locked as resolved and limited conversation to collaborators Nov 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants