Explicitly define the evaluation order/"sequence points" etc.

[huonw]

From this stackoverflow question.

I may just not be searching for the right terms, but I don't believe this is mentioned in the manual. It would be good to have this clearly defined.

There is possibly some subtlety with issues like #6268, where it may make sense to evaluate the receiver last.

[huonw]

@chris-morgan's investigation on that SO question turned up this:

fn foo(_: (), x: int) -> int {
    x
}

fn main() {
    let mut a = 1;
    {
        a * foo(a = 3, a)
    };
}

The block evaluates to 9, while I was expecting 3 with a ltr evaluation order.

[chris-morgan]

For reference, some playing around with things: http://is.gd/ynMTY7

trait Actually {
    fn actually<U: ::std::fmt::Show>(self, U) -> U;
}

impl<T: ::std::fmt::Show> Actually for T {
    fn actually<U: ::std::fmt::Show>(self, u: U) -> U {
        println!("{} actually {}", self, u);
        u
    }
}

fn main() {
    let mut a;
    // -13, 5, -13, 5, -23, 5, -23, 5, -8, 5.
    println!("{}", {a = 2i; a} - {a = 3; a} * {a = 5; a});
    println!("{}", a);
    println!("{}", (a = 2i).actually(a) - (a = 3).actually(a) * (a = 5).actually(a));
    println!("{}", a);
    println!("{}", {a = 2i; a} - a * {a = 5; a});
    println!("{}", a);
    println!("{}", (a = 2i).actually(a) - a * (a = 5).actually(a));
    println!("{}", a);
    println!("{}", (a = 2i).actually(a) - ().actually(a) * (a = 5).actually(a));
    println!("{}", a);

    let mut a;
    // These ones always end up as `false`
    println!("{}", {a = true; a} && {a = false; a});
    println!("{}", a);
    println!("{}", {a = false; a} && {a = true; a});
    println!("{}", a);
    println!("{}", (a = true).actually(a) && (a = false).actually(a));
    println!("{}", a);
    println!("{}", (a = false).actually(a) && (a = true).actually(a));
    println!("{}", a);

    // These ones always end up as `true`
    println!("{}", {a = true; a} || {a = false; a});
    println!("{}", a);
    println!("{}", {a = false; a} || {a = true; a});
    println!("{}", a);
    println!("{}", (a = true).actually(a) || (a = false).actually(a));
    println!("{}", a);
    println!("{}", (a = false).actually(a) || (a = true).actually(a));
    println!("{}", a);
}

[huonw]

Ok, to expand on my comment above (inspired by the last two expressions in the numeric part of @chris-morgan's example), these two differ in evaluation order, even though the only difference is the introduction of the noop foo((), ...) around the a on the LHS:

fn foo(_: (), x: int) -> int { x }

fn main() {
    let mut a = 2i;
    println!("{}", a * foo(a = 5, a)); // 25
    a = 2i;
    println!("{}", foo((), a) * foo(a = 5, a)); // 10
}

http://is.gd/zVdDoh

Nominating.

[pnkfelix]

cc me

[petrochenkov]

While experimenting with evaluation order I (author of SO question) noticed some strange interaction with borrow checker. Consider the next code:

fn my_add(lhs : &int, rhs : &int) -> int { *lhs + *rhs }; // the same as built-in add

let mut a = 1i;
let b1 = my_add({a = 10; &mut a}, {a = 100; &mut a}); // custom add, does't compile, borrow checker is not pleased
let b2 = *{a = 10; &mut a} + *{a = 100; &mut a}; // built-in add, suspiciously compiles, modifying and borrowing "a" while already borrowed

But built-in add is supposed to behave as if it was a function call to the corresponding method of Add trait, right?

[pnkfelix]

@petrochenkov I suspect the reason for the discrepancy you are observing is two-fold.

First, I do not think you are 100% correctly translating the infix-+ to a direct call to the function, at least in terms of how the compiler internally sees it. I think the correct translation probably looks more like this:

    let b3 = {
        let tmp1 = *{a = 10; &mut a};
        let tmp2 = *{a = 100; &mut a};
        my_add(&tmp1, &tmp2)
    };

(Or at least, the above does correspond to to the *{a = 10; &mut a} + *{a = 100; &mut a} in my head, and it successfully compiles in rustc.)

Note in particular that I am dereferencing the &mut a and then reborrowing it immutably in the bindings of tmp1 and tmp2; this is because I have just taken the input expressions you fed into your infix-add (which had derefence operations) and then explicitly added the borrows that the infix-add will put on itself when converting to the calls to the fn add method.

---

Second, one might think, "okay, if what Felix said above works, then why didn't he write the desugaring this way:"

    let b4 = my_add(&*{a = 10; &mut a}, &*{a = 100; &mut a});

And indeed, I did sidestep that possible desugaring. Or rather, I did that one first, myself, but it did not compile. That might be related to the problem documented in #6268. (Or it might not be; I thought it was at first, but on further examination I am not sure if it is or should be.)

[petrochenkov]

I think the correct translation probably looks more like this:
let b3 = {
let tmp1 = *{a = 10; &mut a};
let tmp2 = *{a = 100; &mut a};
my_add(&tmp1, &tmp2)
};

Aha, it means that built-in add does "lvalue to rvalue conversion" (in C++ terms), then borrow checker is correct. But in this case built-in add doesn't implement Add trait, but something slightly different:
fn add<int, int>(self, rhs: int) -> int; // arguments by value
i.e. the same distinction between built-in and overloaded operators as in C++ takes place. Correct?

[pnkfelix]

@petrochenkov no, the infix-+ operator definitely implements the Add trait as specified. (There is an on-going discussion about changing the definition of Add so that it takes its inputs by value instead of by reference, but that is not the current state of things.)

The difference is that the infix-+ operator automatically adds the & to the inputs that you feed it, which is part of what I was trying to show you in my desugaring: note that I added & to each of the two input arguments when I passed them as tmp1 and tmp2.

[pnkfelix]

@petrochenkov Let me put this a different way: Whatever you do to translate <A> + <B> to my_add(<C>, <D>), the expression <A> should appear directly within <C>, and likewise <B> should appear directly within <D>.

But in your in your original comparison, you had something that looked like:

let b1 = my_add(E1, E2);
let b2 = *E1 + *E2;

Note that *E1 (the <A>) does not appear within E1 (the <C>), and likewise *E2 does not appear within E2.

This is why I re-wrote your example instead as something similar to:

let b3 = my_add(&*E1, &*E2);

so that now the *E1 does appear directly within &*E1. And likewise *E2 within &*E2.

This may seem like a minor detail, but the difference between { &mut foo } and &*{ &mut foo } is quite significant.

[petrochenkov]

@pnkfelix

the difference between { &mut foo } and &*{ &mut foo } is quite significant.

I understand - the mutability is different. But I'm still not convinced :)
I'll try to follow the abstract evaluation step by step as I understand it. My assumptions are:

• all the evaluations are sequenced, including built-in operations
• int behaves as if it implemented Add trait exactly

1. my_add({a = 10; &mut a}, {a = 100; &mut a});
   a) the expression {a = 10; &mut a} (first argument) is evaluated, the result has type &mut int (let's cal it b)
   b) b is implicitly converted to &int (c), because my_add has &int as a first parameter
   c) c is saved to be later passed to my_add, i.e. a is borrowed as immutable now
   d) we can't evaluate {a = 100; &mut a}, because a is borrowed as immutable

2. *{a = 10; &mut a} + *{a = 100; &mut a}
   a) the expression *{a = 10; &mut a} is evaluated, the result is a mutable lvalue of type int (b)
   b) operator + under the cover apply operator & to b, the result is of type &int (c)
   c) c is already has type &int as required by Add::add, so no implicit conversion happens
   d) c is saved to be later passed to Add::add, i.e. a is borrowed as immutable now
   e) we can't evaluate {a = 100; &mut a}, because a is borrowed as immutable... wait, we can!?

[pnkfelix]

We are not going to attempt to do this exercise of formally defining the evaluation order for 1.0. Therefore, not assigning this to the 1.0 milestone.

Specific suprises that are currently exhibited by the rustc compiler should be filed as their own bugs.

[pnkfelix]

Having said the above ...

Having the evaluation order actually written up and specified is something that someone should do eventually. Therefore, we will keep this bug open.

Tagging as P-low, (not 1.0 milestone).

[chris-morgan]

But does that not mean that we have undefined behaviour? I thought fixing that sort of thing was deemed important for 1.0.

[pnkfelix]

@chris-morgan I think the underlying claim here is that there is a defined, deterministic order of evaluation. It just is currently only specified via the rustc compiler, but we hope to actually write it down somewhere concretely eventually, and not have it just be something that is "implementation dependant."

[chris-morgan]

@pnkfelix I retract my claim of undefined behaviour. But what if, when it is finally addressed, it is realised that what is there was inconsistent and undesirable (as appears may be the case), possibly even buggy? Then we will not be able to alter it until the next major release.

[pnkfelix]

@chris-morgan First off, I agree that what you describe is an undesirable scenario. I certainly do not like the semantics of order-of-evaluation that is present in @huonw 's examples above.

But, under the assumption that we will not be able to be 100% validate our current implementation before 1.0, I'm not sure we have a better option available than what you describe. After all, if we are going to claim that code in the 1.x series will enjoy some sort of backwards-compatibility guarantee, but we also attempt to fix such "bugs" during the 1.x series, then we would need to support those clients who inadvertently end up relying on the "bad" order-of-evaluation. (By "support", I mean provide a really pain-free way to keep one's code compiling and runnning the same way in the presence of "fixes" to the order of evaluation; perhaps via an attribute with a Rust version number that one attaches to one's module, fn, or closure body.)

But maybe you have a suggestion for a different path? (Keep in mind that I do not expect to be able to validate the implementation against a spec, so merely specifying the order of evaluation separately helps little here, except in identified bugs that still would not be fixed unless you want to solve the problem I outlined above.)

[chris-morgan]

I have no better suggestions.

[alexcrichton]

cc @steveklabnik, could this move to the RFCs repo?

[oli-obk]

Wouldn't it force more readable code if such constructs were forbidden? I mean, the borrow checker already forbids simultaneous mutable and immutable references. Why not also forbid simultaneous mutable and immutable usage?

I have to carefully lay out the steps happening in my mind, even if the eval-order is specified. I don't think anyone considers the following to be readable.

fn main() {
    let mut a = 5;
    let b = (a, a = 6, a, a * (a, a = 7).0);
    println!("{:?}", b);
}

or logical (even the compiler is confused about what's going on...):

<anon>:3:35: 3:36 warning: value assigned to `a` is never read, #[warn(unused_assignments)] on by default
<anon>:3     let b = (a, a = 6, a, a * (a, a = 7).0);
                                           ^
(5, (), 6, 42)

Sequence points are a major issue in c/c++, not just because they cause undefined behavior, but also because not everyone is an expert on the evaluation order. If you give a piece of code to a group of non-experts on sequence points, you get a quite a number of different suggestions of what the outcome should be. There won't be less confusion just because the evaluation order is defined. Most likely there'll just be a number of "good practices" about how to write readable expressions.

Things like v.truncate(v.len() - 1) are a different issue, as that is hierarchical an therefor there cannot be a confusion about the order.

[steveklabnik]

After talking with @nikomatsakis , we're not quite ready to define these things yet. Given that that will take a lot of work, I'm going to be giving this issue a close, as it's really more of an RFC type of thing, but

I think we will be I think we will be nailing down pretty precisely as part of the MIR construction work,
nailing down pretty precisely as part of the MIR construction work,

and so documenting all that would fall under that work anyway.

[bluss]

The borrow checker thinks it's left to right:

let mut data = vec![1, 2, 3];

anyfunction(data.remove(0), &mut data);  // Ok
anyfunction(&mut data, data.remove(0));  // Not Ok