Add WrappingNeg trait

[ocstl]

Closes #10

A quick search turned up that the extprim implements PrimInt for its u128 and i128, but, since both have their own wrapping_neg method, it's a non-issue.

That being said, I don't know for certain that it is the only project that implements PrimInt, so it might be wise to wait for a larger version bump before merging this.

[cuviper]

Thinking about #10 again -- we don't have any other "wrapping" methods on PrimInt, so I'm not sure why we would add wrapping_neg in particular. See also #95.

However, we do have WrappingAdd etc. -- so how about adding WrappingNeg?

[cuviper]

A quick search turned up that the extprim implements PrimInt for its u128 and i128, but, since both have their own wrapping_neg method, it's a non-issue.

Note that while they do have an inherent wrapping_neg method, they would still be broken by a new PrimInt with that method until they add it to their implementation explicitly.
(Rust doesn't use structural typing for traits, the way Go does for interfaces.)

[ocstl]

Note that while they do have an inherent wrapping_neg method, they would still be broken by a new PrimInt with that method until they add it to their implementation explicitly.
(Rust doesn't use structural typing for traits, the way Go does for interfaces.)

I thought I had tested for it, but you made me realise where I made a mistake. Thank you!

Thinking about #10 again -- we don't have any other "wrapping" methods on PrimInt, so I'm not sure why we would add wrapping_neg in particular. See also #95.

However, we do have WrappingAdd etc. -- so how about adding WrappingNeg?

It did feel a bit odd. And thanks for pointing out #95, it puts things in context.

The potential issue with adding WrappingNeg is that Neg was only implemented for Wrapping in Rust 1.10. Wouldn't this create a problem with regards to maintaining compatibility with 1.8?

[cuviper]

The potential issue with adding WrappingNeg is that Neg was only implemented for Wrapping in Rust 1.10. Wouldn't this create a problem with regards to maintaining compatibility with 1.8?

I think it will be OK if we follow the style of the others:

impl<T: WrappingNeg> WrappingNeg for Wrapping<T>
where
    Wrapping<T>: Neg<Output = Wrapping<T>>,
{
    fn wrapping_neg(&self) -> Self {
        Wrapping(self.0.wrapping_neg())
    }
}

That where constraint just won't match anything before Rust 1.10. As written, it's not actually using <Wrapping<T> as Neg>::neg at all, so this is overconstrained, but it's what we have on the others. 🤷‍♂️

[ocstl]

One thing about this implementation that bugs me a bit is that the WrappingNeg trait can't be bound on Neg, since that trait is not implemented for the unsigned types, for (in retrospect) obvious reasons. This breaks with the style for the other Wrapping traits.

That where constraint just won't match anything before Rust 1.10. As written, it's not actually using <Wrapping as Neg>::neg at all, so this is overconstrained, but it's what we have on the others.

That is good to know, thank you!

I didn't see it coming, but turns out the absence of the Neg trait for Wrapping in Rust 1.8 is causing issues for testing though. I guess a workaround would be to use Sub instead (Wrapping(0_u8) - Wrapping(...)), but this seems unsatisfactory.

While it would be great to be able to close this issue, if you feel any discomfort qualms at all about this, don't hesitate to say so.

[ocstl]

One thing about this implementation that bugs me a bit is that the WrappingNeg trait can't be bound on Neg, since that trait is not implemented for the unsigned types, for (in retrospect) obvious reasons. This breaks with the style for the other Wrapping traits.

Since CheckedNeg isn't bound by Neg either, this is fine. I tried to maintain the same order of operations as the Checked traits to make comparisons easier.

I also fixed a slight formatting issue.

The test for Wrapping falls back on Sub for the time being for lack of a better idea, although it does work for versions including 1.10 and above; I added a FIXME in case Neg becomes available following a MSRV change.

Thank you for your time, and let me know if there's anything. I was a bit too optimistic here.

[ocstl]

Sorry for the stream-of-thought like nature of the previous comments.

That where constraint just won't match anything before Rust 1.10. As written, it's not actually using <Wrapping as Neg>::neg at all, so this is overconstrained, but it's what we have on the others.

This creates a bit of a problem for the wrapping_is_wrappingneg() test function, since in Rust 1.8, Wrapping is not WrappingNeg, although it is once we reach 1.10. This bothered me a bit, but I guess that since Neg is not implemented for Wrapping over all covered versions, this is not something one can guarantee to users.

Also, Since Neg was only implemented in 1.10, we can't actually test the Neg op (-Wrapping(255u8)). I replaced both of these tests with a TODO in a separate commit. This way, if the MSRV reaches 1.10, it's a simple matter of reverting it to bring back the tests.

Alternatively, if you'd rather tabled implemented WrappingNeg if and when the MSRV reaches 1.10 so both tests can be included, I would understand completely.

This legacy business is tricky. So, thank you for your hard work. 🙇‍♂️

[cuviper]

I added a little bit to the doc example, but otherwise I think this is good, thanks!

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push