Auto merge of #65608 - matthewjasper:mir-eval-order, r=pnkfelix

Fix MIR lowering evaluation order and soundness bug

* Fixes a soundness issue with built-in index operations
* Ensures correct evaluation order of assignment expressions where the RHS is a FRU or is a use of a local of reference type.
* Removes an unnecessary symbol to string conversion

closes #65909
closes #65910

8 files changed, 201 insertions, 17 deletions

diff --git a/src/test/mir-opt/storage_live_dead_in_statics.rs b/src/test/mir-opt/storage_live_dead_in_statics.rs
index 2ed34ecfad2..5dc15286bab 100644
--- a/src/test/mir-opt/storage_live_dead_in_statics.rs
+++ b/src/test/mir-opt/storage_live_dead_in_statics.rs
@@ -36,11 +36,11 @@ fn main() {
 // END RUST SOURCE
 // START rustc.XXX.mir_map.0.mir
 //    let mut _0: &'static Foo;
-//    let mut _1: &'static Foo;
+//    let _1: &'static Foo;
 //    let _2: Foo;
 //    let mut _3: &'static [(u32, u32)];
 //    let mut _4: &'static [(u32, u32); 42];
-//    let mut _5: &'static [(u32, u32); 42];
+//    let _5: &'static [(u32, u32); 42];
 //    let _6: [(u32, u32); 42];
 //    let mut _7: (u32, u32);
 //    let mut _8: (u32, u32);
diff --git a/src/test/mir-opt/uninhabited_enum_branching.rs b/src/test/mir-opt/uninhabited_enum_branching.rs
index 1f37ff1498d..aa56918a9b8 100644
--- a/src/test/mir-opt/uninhabited_enum_branching.rs
+++ b/src/test/mir-opt/uninhabited_enum_branching.rs
@@ -34,12 +34,12 @@ fn main() {
 // let _1: &str;
 // let mut _2: Test1;
 // let mut _3: isize;
-// let mut _4: &str;
-// let mut _5: &str;
+// let _4: &str;
+// let _5: &str;
 // let _6: &str;
 // let mut _7: Test2;
 // let mut _8: isize;
-// let mut _9: &str;
+// let _9: &str;
 // bb0: {
 //   StorageLive(_1);
 //   StorageLive(_2);
@@ -103,12 +103,12 @@ fn main() {
 // let _1: &str;
 // let mut _2: Test1;
 // let mut _3: isize;
-// let mut _4: &str;
-// let mut _5: &str;
+// let _4: &str;
+// let _5: &str;
 // let _6: &str;
 // let mut _7: Test2;
 // let mut _8: isize;
-// let mut _9: &str;
+// let _9: &str;
 // bb0: {
 //   StorageLive(_1);
 //   StorageLive(_2);
@@ -172,12 +172,12 @@ fn main() {
 // let _1: &str;
 // let mut _2: Test1;
 // let mut _3: isize;
-// let mut _4: &str;
-// let mut _5: &str;
+// let _4: &str;
+// let _5: &str;
 // let _6: &str;
 // let mut _7: Test2;
 // let mut _8: isize;
-// let mut _9: &str;
+// let _9: &str;
 // bb0: {
 //   StorageLive(_1);
 //   StorageLive(_2);
diff --git a/src/test/ui/borrowck/borrowck-init-in-fru.stderr b/src/test/ui/borrowck/borrowck-init-in-fru.stderr
index a4c042d1c12..f01afe1466a 100644
--- a/src/test/ui/borrowck/borrowck-init-in-fru.stderr
+++ b/src/test/ui/borrowck/borrowck-init-in-fru.stderr
@@ -1,8 +1,8 @@
 error[E0381]: use of possibly-uninitialized variable: `origin`
-  --> $DIR/borrowck-init-in-fru.rs:9:5
+  --> $DIR/borrowck-init-in-fru.rs:9:14
    |
 LL |     origin = Point { x: 10, ..origin };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of possibly-uninitialized `origin.y`
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^ use of possibly-uninitialized `origin.y`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs
new file mode 100644
index 00000000000..0e0e3cda6e2
--- /dev/null
+++ b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs
@@ -0,0 +1,82 @@
+// Test that we error if a slice is modified after it has been bounds checked
+// and before we actually index it.
+
+fn modify_before_assert_slice_slice(x: &[&[i32]]) -> i32 {
+    let mut x = x;
+    let z: &[i32] = &[1, 2, 3];
+    let y: &[&[i32]] = &[z];
+    x[{ x = y; 0 }][2]              // OK we haven't checked any bounds before we index `x`.
+}
+
+fn modify_before_assert_array_slice(x: &[&[i32]; 3]) -> i32 {
+    let mut x = x;
+    let z: &[i32] = &[1, 2, 3];
+    let y: &[&[i32]; 3] = &[z, z, z];
+    x[{ x = y; 0 }][2]              // OK we haven't checked any bounds before we index `x`.
+}
+
+fn modify_before_assert_slice_array(x: &[&[i32; 3]]) -> i32 {
+    let mut x = x;
+    let z: &[i32; 3] = &[1, 2, 3];
+    let y: &[&[i32; 3]] = &[z];
+    x[{ x = y; 0 }][2]              // OK we haven't checked any bounds before we index `x`.
+}
+
+fn modify_before_assert_array_array(x: &[&[i32; 3]; 3]) -> i32 {
+    let mut x = x;
+    let z: &[i32; 3] = &[1, 2, 3];
+    let y: &[&[i32; 3]; 3] = &[z, z, z];
+    x[{ x = y; 0 }][2]              // OK we haven't checked any bounds before we index `x`.
+}
+
+fn modify_after_assert_slice_slice(x: &[&[i32]]) -> i32 {
+    let mut x = x;
+    let z: &[i32] = &[1, 2, 3];
+    let y: &[&[i32]] = &[&z];
+    x[1][{ x = y; 2}]               //~ ERROR cannot assign `x` in indexing expression
+}
+
+fn modify_after_assert_array_slice(x: &[&[i32]; 1]) -> i32 {
+    let mut x = x;
+    let z: &[i32] = &[1, 2, 3];
+    let y: &[&[i32]; 1] = &[&z];
+    x[0][{ x = y; 2}]               // OK cannot invalidate a fixed-size array bounds check
+}
+
+fn modify_after_assert_slice_array(x: &[&[i32; 3]]) -> i32 {
+    let mut x = x;
+    let z: &[i32; 3] = &[1, 2, 3];
+    let y: &[&[i32; 3]] = &[&z];
+    x[1][{ x = y; 2}]               //~ ERROR cannot assign `x` in indexing expression
+}
+
+fn modify_after_assert_array_array(x: &[&[i32; 3]; 1]) -> i32 {
+    let mut x = x;
+    let z: &[i32; 3] = &[1, 2, 3];
+    let y: &[&[i32; 3]; 1] = &[&z];
+    x[0][{ x = y; 2}]               // OK cannot invalidate a fixed-size array bounds check
+}
+
+fn modify_after_assert_slice_slice_array(x: &[&[[i32; 1]]]) -> i32 {
+    let mut x = x;
+    let z: &[[i32; 1]] = &[[1], [2], [3]];
+    let y: &[&[[i32; 1]]] = &[&z];
+    x[1][{ x = y; 2}][0]            //~ ERROR cannot assign `x` in indexing expression
+}
+
+fn modify_after_assert_slice_slice_slice(x: &[&[&[i32]]]) -> i32 {
+    let mut x = x;
+    let z: &[&[i32]] = &[&[1], &[2], &[3]];
+    let y: &[&[&[i32]]] = &[z];
+    x[1][{ x = y; 2}][0]            //~ ERROR cannot assign `x` in indexing expression
+}
+
+
+fn main() {
+    println!("{}", modify_after_assert_slice_array(&[&[4, 5, 6], &[9, 10, 11]]));
+    println!("{}", modify_after_assert_slice_slice(&[&[4, 5, 6], &[9, 10, 11]]));
+    println!("{}", modify_after_assert_slice_slice_array(&[&[[4], [5], [6]], &[[9], [10], [11]]]));
+    println!("{}", modify_after_assert_slice_slice_slice(
+        &[&[&[4], &[5], &[6]], &[&[9], &[10], &[11]]]),
+    );
+}
diff --git a/src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr
new file mode 100644
index 00000000000..f9ed16f19cd
--- /dev/null
+++ b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr
@@ -0,0 +1,35 @@
+error[E0510]: cannot assign `x` in indexing expression
+  --> $DIR/slice-index-bounds-check-invalidation.rs:36:12
+   |
+LL |     x[1][{ x = y; 2}]
+   |     ----   ^^^^^ cannot assign
+   |     |
+   |     value is immutable in indexing expression
+
+error[E0510]: cannot assign `x` in indexing expression
+  --> $DIR/slice-index-bounds-check-invalidation.rs:50:12
+   |
+LL |     x[1][{ x = y; 2}]
+   |     ----   ^^^^^ cannot assign
+   |     |
+   |     value is immutable in indexing expression
+
+error[E0510]: cannot assign `x` in indexing expression
+  --> $DIR/slice-index-bounds-check-invalidation.rs:64:12
+   |
+LL |     x[1][{ x = y; 2}][0]
+   |     ----   ^^^^^ cannot assign
+   |     |
+   |     value is immutable in indexing expression
+
+error[E0510]: cannot assign `x` in indexing expression
+  --> $DIR/slice-index-bounds-check-invalidation.rs:71:12
+   |
+LL |     x[1][{ x = y; 2}][0]
+   |     ----   ^^^^^ cannot assign
+   |     |
+   |     value is immutable in indexing expression
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0510`.
diff --git a/src/test/ui/mir/mir_assign_eval_order.rs b/src/test/ui/mir/mir_assign_eval_order.rs
new file mode 100644
index 00000000000..1594421b0b1
--- /dev/null
+++ b/src/test/ui/mir/mir_assign_eval_order.rs
@@ -0,0 +1,67 @@
+// Test evaluation order of assignment expressions is right to left.
+
+// run-pass
+
+// We would previously not finish evaluating borrow and FRU expressions before
+// starting on the LHS
+
+struct S(i32);
+
+fn evaluate_reborrow_before_assign() {
+    let mut x = &1;
+    let y = &mut &2;
+    let z = &3;
+    // There's an implicit reborrow of `x` on the right-hand side of the
+    // assignement. Note that writing an explicit reborrow would not show this
+    // bug, as now there would be two reborrows on the right-hand side and at
+    // least one of them would happen before the left-hand side is evaluated.
+    *{ x = z; &mut *y } = x;
+    assert_eq!(*x, 3);
+    assert_eq!(**y, 1);             // y should be assigned the original value of `x`.
+}
+
+fn evaluate_mut_reborrow_before_assign() {
+    let mut x = &mut 1;
+    let y = &mut &mut 2;
+    let z = &mut 3;
+    *{ x = z; &mut *y } = x;
+    assert_eq!(*x, 3);
+    assert_eq!(**y, 1);            // y should be assigned the original value of `x`.
+}
+
+// We should evaluate `x[2]` and borrow the value out *before* evaluating the
+// LHS and changing its value.
+fn evaluate_ref_to_temp_before_assign_slice() {
+    let mut x = &[S(0), S(1), S(2)][..];
+    let y = &mut &S(7);
+    *{ x = &[S(3), S(4), S(5)]; &mut *y } = &x[2];
+    assert_eq!(2, y.0);
+    assert_eq!(5, x[2].0);
+}
+
+// We should evaluate `x[2]` and copy the value out *before* evaluating the LHS
+// and changing its value.
+fn evaluate_fru_to_temp_before_assign_slice() {
+    let mut x = &[S(0), S(1), S(2)][..];
+    let y = &mut S(7);
+    *{ x = &[S(3), S(4), S(5)]; &mut *y } = S { ..x[2] };
+    assert_eq!(2, y.0);
+    assert_eq!(5, x[2].0);
+}
+
+// We should evaluate `*x` and copy the value out *before* evaluating the LHS
+// and dropping `x`.
+fn evaluate_fru_to_temp_before_assign_box() {
+    let x = Box::new(S(0));
+    let y = &mut S(1);
+    *{ drop(x); &mut *y } = S { ..*x };
+    assert_eq!(0, y.0);
+}
+
+fn main() {
+    evaluate_reborrow_before_assign();
+    evaluate_mut_reborrow_before_assign();
+    evaluate_ref_to_temp_before_assign_slice();
+    evaluate_fru_to_temp_before_assign_slice();
+    evaluate_fru_to_temp_before_assign_box();
+}
diff --git a/src/test/ui/nll/issue-52534-2.stderr b/src/test/ui/nll/issue-52534-2.stderr
index dd8a87f7e29..cef4aba0240 100644
--- a/src/test/ui/nll/issue-52534-2.stderr
+++ b/src/test/ui/nll/issue-52534-2.stderr
@@ -1,8 +1,8 @@
 error[E0597]: `x` does not live long enough
-  --> $DIR/issue-52534-2.rs:6:9
+  --> $DIR/issue-52534-2.rs:6:13
    |
 LL |         y = &x
-   |         ^^^^^^ borrowed value does not live long enough
+   |             ^^ borrowed value does not live long enough
 LL |
 LL |     }
    |     - `x` dropped here while still borrowed
diff --git a/src/test/ui/span/issue-36537.stderr b/src/test/ui/span/issue-36537.stderr
index edb804e850e..0939584380a 100644
--- a/src/test/ui/span/issue-36537.stderr
+++ b/src/test/ui/span/issue-36537.stderr
@@ -1,8 +1,8 @@
 error[E0597]: `a` does not live long enough
-  --> $DIR/issue-36537.rs:5:9
+  --> $DIR/issue-36537.rs:5:13
    |
 LL |         p = &a;
-   |         ^^^^^^ borrowed value does not live long enough
+   |             ^^ borrowed value does not live long enough
 ...
 LL |     }
    |     - `a` dropped here while still borrowed