Impl {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}Assign<$t> for Wrapping<$...

m-ou-se commented 26 days ago

I feel a bit conflicted about these. I agree Wrapping can be annoying to work with, and this is a big improvement to that. But this also adds some inconcistencies that we might not be able to fix:

• This adds Add<i32> for Wrapping<i32> (etc.), but do we also want Add<Wrapping<i32>> for i32? Having w + 1 work but not 1 + w seems bad. But adding even more implementations on i32 doesn't seem great either.
• Should we also add Add<&i32> for Wrapping<i32> (etc.), just like we have Add<&Wrapping> for Wrapping? It would be consistent, but it would technically be a breaking change since Wrapping(1) + &Default::default() would no longer compile.
• How about PartialEq/PartialOrd? We can't add those as it'd be a breaking change, but having w + 1 work, while still requiring an explicit w == Wrapping(1) also doesn't feel great.
• What if someone wants a non-wrapping result type? E.g. they write let b: i32 = w + 1; and forget the .0 in w.0 to get the non-wrapping type. Today, they get an error that they can't add i32 to Wrapping<i32> and they need to pick either the wrapping behaviour or the non-wrapping behaviour explicitly, by wrapping the 1 in Wrapping(1) or by unwrapping the w with .0. But with this change, the addition would happen in a wrapping way, and then give the an error about assigning a Wrapping<i32> to a i32. Or not give any error if : i32 wasn't explicit.

This makes me think it might be better if we only do this for the *Assign traits. wrapping_i32 += 1; clearly follows the behaviour of the wrapping_i32 variable, and some_i32 += Wrapping(1); won't compile.

These are just some initial thoughts after thinking about this for a few minutes. I'd love to hear some input from the rest of @rust-lang/libs-api.

This means - if I understood this correctly - the new impls have to be stabilized instantly. Which in turn means, this PR has to kick of an FCP on the tracking issue as well?

In such cases we usually do the FCP on the PR itself.