Settling execution order for `=`, `+=`


#1

So I was going through old notifications (ah, the backlog!) and I was reminded that there are some unsettled questions as to Rust execution order that we really ought to get to the bottom of.

In particular, Rust generally prefers left-to-right execution order for all expressions – however, we do make some exceptions. Apparently, before MIR, we were actually quite inconsistent here, in that borrowck and trans disagreed on some of the particulars. MIR eliminated that inconsistency, but there are still some matters that we should try to resolve – and quick!

Case 1: assignments

As #27868 describes, it used to be that borrowck and trans disagreed about the execution order of a = b. In particular, borrowck treated the execution order as evaluating b first and then a – right-to-left. (Oddly, I have a distinct memory of trans working this way as well at the time when borrowck was first written, but perhaps I am wrong, or else perhaps trans drifted over time and borrowck failed to keep up.)

In any case, while LTR ordering is definitely more consistent overall, there is a strong reason to keep the ordering around the = operator somewhat different. Consider the expression vec[i] = vec[j]. Currently, this desugars to something like:

// vec[i] = vec[j]
let a = {
  let b = &vec[j]; // actually overloaded
  *b
};
let c = &mut vec[i];
*c = a;

This borrow checks just fine because the first borrow, of b = &vec[j], is confined to the rhs computation, and is complete before we take the second borrow (&mut vec[i]). (This error is similar to the limitations around self.foo(self.bar()) when foo or bar is an &mut self method; that is not (entirely) an accident and I’ll come to that later.)

When MIR landed, it agreed with borrowck. That means that we currently execute RHS before LHS. I don’t know that we are at liberty to change this: without significantly improving the type system, that would break a lot of code!

Case 2: Augmented assignment

There is a similar question of execution order around a += b. To make things a bit more interesting, when += is overloaded, the add_assign method takes an &mut borrow of a – in other words, it is not actually equivalent to a = a + b.

Unfortunately, MIR seems to be internally inconsistent here at present, as this example (from @eddyb) demonstrates. If the += is overloaded, we evaluate b then a. Otherwise, we evaluate a and then b! This seems no good.

Also, evaluating LTR is inconsistent with borrowck, which accepts things like this today:

fn foo(x: &mut [i32]) {
    x[0] += x[1];
}
    
fn main() { }

I believe that if we evaluate LTR this gets desugared to roughly the following, and hence would be an error, right?

let p = &mut x[0];
let q = x[1];
*p += q;

Where to go from here?

I think we should keep a = b evaluation order as it is now in MIR, because I don’t think we can make code like vec[i] = vec[j] stop compiling. (We had to make MIR/trans consistent, obviously, for soundness.) It seems like a similar argument implies a += b should be RTL. This is consistent in a certain way (if you see an =, RTL).

I haven’t done any crater runs to test how prevalent such patterns are.

Appendix: Method call evaluation order

It is well-known that nested method calls and &mut self methods don’t play nice. In particular, if you have self.set(self.get()), those arguments are evaluated LTR, which means that this line gets desugared to something like this:

let _0 = &mut *self;    // mutable borrow starts here
let _1 = &*self;        // borrow of `self` for `self.get()`
let _2 = Type::get(_1); 
Type::set(_0, _2);

You can see here that the borrow _0 is in scope when the first argument (self.get()) is evaluated. This is what causes the problem.

There are a couple of ways we could fix this. One really drastic thing we could which might help would be to change the evaluation order of arguments (note: I am not actually proposing this, just exploring a hypothetical). For example, we could imagine that we evaluate the arguments in reverse order, and hence we would have:

let _1 = &*self;        // borrow of `self` for `self.get()`
let _2 = Type::get(_1); 
let _0 = &mut *self;    // mutable borrow starts here
Type::set(_0, _2);

But this would be a massive breaking change.

I think we have to (and want to) keep the observable semantics of method calls to be evaluated left-to-right. But we could make the borrowck happy and improve everyday life by reordering those argument evaluations only when it is not observable. That would apply in this case, but would not apply in cases like self.foo().set(self.get()).

You can probably see the analogy to the vec[i] = vec[j] case. One important thing to note is that this would not be a legal case for re-ordering. This is because vec[i] is going through the Index and Deref traits for Vec, so the reordering would definitely be observable!

(There is also another way to think about making borrowck smarter without reordering; one could imagine that the _0 borrow is for a lifetime that includes only Type::set, and not the let itself. I don’t want to get distracted so I won’t go into detail here, but it works out pretty similar to the idea of reordering only when not observable in terms of what it can typecheck.)


Accepting nested method calls with an `&mut self` receiver
#2

So, a += b is not desugared into AddAssign::add_assign(&mut a, b)? Seems bad.


#3

Nope, rather into something like this, I think:

let _1 = b;
let _0 = &mut a;
AddAssign::add_assign(_0, _1)

#4

I thought @eddyb and @arielb1 had this all figured out?


#5

So I talked to @eddyb about the foo.bar() case on IRC. We were using different terms but it seems like we were both thinking about similar lines.

Basically, we can say that the evaluation order is LTR, but with an asterix that the borrowck sometimes accepts that wouldn’t work with a totally naive translation. You can think of this in many ways:

  • as reorderings in the MIR desugaring (as I kind of described it);
  • as borrowing for a future lifetime, namely the upcoming call (as I’ve often described it at other times);
  • as downgrading and upgrading permissions (as @eddyb was talking about it);

but however you do it you seem to wind up in a pretty similar place. We can make <x>.set(<x>.get()) work so long as the expression <x> consists of local variables and fields, but doesn’t contain traverse any boxes or downcasts into enums. IOW basically as long as it doesn’t have function calls (but that includes overloaded indices and derefs).

However, I think that the = cases are different. As I wrote on IRC, we’ve been borrow-checking method calls LTR, so we can loosen a bit and stay within that framework. But we’ve been borrow-checking = stuff as right-to-left, so switching to a LTR framework will cause a lot of errors I believe, even if we try to be a bit more accepting. Notably, stuff like vec[i] = vec[j] would not work.


#6

Sure. This is a dupe of Rust expression order of evaluation. Use the search!