Primitive vs struct antipattern?

[sjep]

This is coming from a stack overflow question.

My answer didn't sit well with me. While it's true that the add_assign isn't called in the primitive case, the fact that it's called shouldn't cause a compilation failure because the Vec isn't being passed in.

The below fails to compile:

use std::ops::AddAssign;

#[derive(Clone, Copy, PartialEq, Debug, Default)]
struct MyNum(i32);

impl AddAssign for MyNum {
    fn add_assign(&mut self, rhs: MyNum) {
        *self = MyNum(self.0 + rhs.0)
    }
}

fn main() {
    let mut b = vec![MyNum(0), MyNum(1)];
    b[0] += b[1];
}

With this message:

error[E0502]: cannot borrow `b` as immutable because it is also borrowed as mutable
  --> src/main.rs:14:13
   |
14 |     b[0] += b[1];
   |     --------^---
   |     |       |
   |     |       immutable borrow occurs here
   |     mutable borrow occurs here
   |     mutable borrow later used here

However this works:

fn main() {
    let mut b = vec![1, 2];
    b[0] += b[1];
}

Looking at the MIR, the only difference I can see is in the primitive case Index is called before IndexMut whereas in the struct case that is reversed. I don't see why the struct needs to behave differently.

I expected to see this happen: the primitive case and the struct case should either both fail or both succeed. In my mind, they should both succeed as the dereference for b[1] can complete before b[0].

If the mutable struct needs to exist first for some reason then the slice version should also fail, but this works:

use std::ops::AddAssign;

#[derive(Clone, Copy, PartialEq, Debug, Default)]
struct MyNum(i32);

impl AddAssign for MyNum {
    fn add_assign(&mut self, rhs: MyNum) {
        *self = MyNum(self.0 + rhs.0)
    }
}

fn main() {
    let mut b = vec![MyNum(0), MyNum(1)];
    let slice = b.as_mut_slice();
    slice[0] += slice[1];
}

Meta

rustc --version --verbose:

rustc 1.45.0-nightly (75e1463c5 2020-05-13)
binary: rustc
commit-hash: 75e1463c52aaea25bd32ed53c73797357e561cce
commit-date: 2020-05-13
host: x86_64-apple-darwin
release: 1.45.0-nightly
LLVM version: 9.0

[sjep]

The slice version does mut dereference first. So the struct behavior between slice and vec both deref the left-hand side then the right-hand side. Slice doesn't call Index or IndexMut though, it dereferences directly from the slice.

I'm sure there is some desugaring going on to cause the behavior, but I'm not sure it should work this way?

[steffahn]

Someone in an IRLO discussion pointed here. Here’s a full explanation.

First, as (thankfully documented) by the reference, there is a special case with regards to order-of-evaluation for operators like +=

Evaluation of compound assignment expressions depends on the types of the operators.

If both types are primitives, then the modifying operand will be evaluated first followed by the assigned operand. It will then set the value of the assigned operand's place to the value of performing the operation of the operator with the values of the assigned operand and modifying operand.

Note: This is different than other expressions in that the right operand is evaluated before the left one.

Otherwise, this expression is syntactic sugar for calling the function of the overloading compound assignment trait of the operator (see the table earlier in this chapter). A mutable borrow of the assigned operand is automatically taken.

The second piece to the equation is two-phase borrows. I couldn’t find anything about them in the reference, but here’s an explanation in the rustc development guide.

Two-phase borrows are a more permissive version of mutable borrows that allow nested method calls such as vec.push(vec.len()). Such borrows first act as shared borrows in a "reservation" phase and can later be "activated" into a full mutable borrow.

Only certain implicit mutable borrows can be two-phase, any &mut or ref mut in the source code is never a two-phase borrow. The cases where we generate a two-phase borrow are:

1. The autoref borrow when calling a method with a mutable reference receiver.
2. A mutable reborrow in function arguments.
3. The implicit mutable borrow in an overloaded compound assignment operator.

So the reference is slightly inaccurate about the += being just syntactic sugar.

The third piece of information is that slice’s indexing operation is compiler-implemented while Vec’s goes through Index[Mut] traits. So the former allows more precise analysis by the borrow checker, because it gets to make more assumptions about what the implementation works like. Or rather: the latter doesn’t get handled directly by the borrow checker at all, as far as I’m aware, but instead desugared first to a call to Index[Mut] methods. Same effect: the behavior is opaque to the borrow checker, whereas it natively supports reasoning about built-in indexing.

Now, to explain the three cases.

• b[0] += b[1] on Vec<i32> works because the RHS is evaluated before the LHS

• b[0] += b[1] on &mut [MyNum] will evaluate the LHS before the RHS, but the indexing is a primitive operation.
  As such, it supports access via two-phase borrows. Evaluating the expression b[0] will check whether the index is in-bounds and only borrows the slice immutably at first; then the RHS is evaluated, requiring immutable access to b for a short time to read the value, and after that, the LHS’s borrow is upgraded to a mutable borrow.

• b[0] += b[1] on Vec<MyNum> will evaluate the LHS before the RHS, and the indexing works via IndexMut.
  Since IndexMut::index_mut already gets full mutable access to the Vec while the indexing happens, it would not be safe to allow any form of “do the indexing without any mutable access first, upgrade later” shenanigans. So the code fails to compile. Or put differently: The indexing isn’t primitive but (effectively) desugars to something like *IndexMut::index_mut(&mut b, 0). Evaluating this expression, even immutably borrowed at first for the first step of two-phase borrows, will already borrow b itself mutably, because of the explicit &mut b.