Implement std::ops::*Assign traits for slices

[newpavlov]

Quite often I need to xor two slices, but I can't write buf1 ^= buf2; as BitXorAssign is not implemented for &[u8] and it's quite an unpleasant papercut for me. I couldn't find a good reason why it was not done, so I would like to suggest to add generic implementations of ops::*Assign traits.

It can be done as follows:

Click to expand

macro_rules! impl_assign_trait {
    ($trait:ident, $method:ident) => {
        impl<'a, T, Rhs> $trait<&'a [Rhs]> for [T]
            where T: $trait<Rhs>, Rhs: Copy
        {
            fn $method(&mut self, rhs: &[Rhs]) {
                for (a, b) in self.iter_mut().zip(rhs) {
                    a.$method(*b);
                }
            }
        }

        impl<T> $trait<T> for [T] where T: $trait + Copy {
            fn $method(&mut self, rhs: T) {
                for v in self.iter_mut() {
                    v.$method(rhs);
                }
            }
        }
    }
}

impl_assign_trait!(AddAssign, add_assign);
impl_assign_trait!(BitAndAssign, bitand_assign);
impl_assign_trait!(BitOrAssign, bitor_assign);
impl_assign_trait!(BitXorAssign, bitxor_assign);
impl_assign_trait!(DivAssign, div_assign);
impl_assign_trait!(MulAssign, mul_assign);
impl_assign_trait!(RemAssign, rem_assign);
impl_assign_trait!(SubAssign, sub_assign);

impl<T, Rhs> ShrAssign<Rhs> for [T] where T: ShrAssign<Rhs>, Rhs: Copy {
    fn shr_assign(&mut self, rhs: Rhs) {
        for v in self.iter_mut() {
            v.shr_assign(rhs);
        }
    }
}

impl<T, Rhs> ShlAssign<Rhs> for [T] where T: ShlAssign<Rhs>, Rhs: Copy {
    fn shl_assign(&mut self, rhs: Rhs) {
        for v in self.iter_mut() {
            v.shl_assign(rhs);
        }
    }
}

I am not sure about second impl in the macro, but I guess in some cases it could be convenient to write buf += 4; instead of an explicit loop.

I can create PR if there is no arguments against this addition.

[eternaleye]

On the one hand, I can definitely see the usefulness. On the other hand, there are some questions that I think need considered.

In a ^= b:

• What should happen if a.length() < b.length()?
  • Panic?
  • Only operate on the common prefix?
• What about the other way around?
• With a: &mut [T] and b: &[U]
  • Should this delegate to <T as BitXor<U, Output=T>> or <T as BitXorAssign<U>>?
• Should this require U: Copy?
• Should this require T: Copy?
• How is this affected by RFC 1909?
  • Should BitXorAssign<[U]> be implemented in addition to BitXorAssign<&[U]>?
  • Should that not require U: Copy, if BitXorAssign<&[U]> does?

[newpavlov]

What should happen if a.length() < b.length()?

I personally think it should simply panic if a.length() != b.length(). Probably we should add assert_eq with an appropriate panic message.

With a: &mut [T] and b: &[U] Should this delegate to <T as BitXor<U, Output=T>> or <T as BitXorAssign>?

As I wrote in the implementation I think we should use BitXorAssign, as it will be the less surprising compared to BitXor, for using which I don't see much reason.

Should this require U: Copy?

Yes, because Assign traits take rhs by value and not by reference, so we have to copy value from the slice. Maybe we could extend it to Clone, but I am not sure if we should.

Should this require T: Copy?

It should not, and in the implementation which I presented it does not. It's a fully working code, copy pasted from my local copy of the repository.

How is this affected by RFC 1909?

Unfortunately I can't answer this question.

[eternaleye]

Yes, because Assign traits take rhs by value and not by reference

That's the main reason that using Op rather than OpAssign might be worthwhile - those are implemented both for references and values of the primitive integer types. Another option might be to do the same for OpAssign.

Either of these would allow dropping the U: Copy requirement.

[newpavlov]

It could be definitely nice to remove Copy constraint, but I am not sure if there is enough merit to justify added complexity.

Also if I am not mistaken Op and OpAssign traits are currently completely independent from each other, and there is no blank implementations of one in terms of another. So I don't think it's the right place to introduce this kind of coupling between traits.

After some thought, it seems that proposed implementation could brake 3rd party code, e.g. in case if crate defines some type T, implements OpAssign not only for it, but also for &[T]. If it's indeed true, then we'll have to implement OpAssign only for primitive types from the standard library.

[dtolnay]

The behavior of panicking on a size mismatch makes me uneasy about adding these impls. In any case, it seems like there is some design work to figure out still, and plenty of tradeoffs. I would prefer for this feature to go through the RFC process if anyone feels motivated to put together a writeup of the tradeoffs, alternative designs, and concrete proposal.