transpile: In is_simple_lvalue, also check index expr for side effects

Author: Rua

c2rust-transpile/src/translator/named_references.rs

@@ -3,6 +3,10 @@
 //! corresponding to a single Rust expression.
 
 use super::*;
+use syn::{
+    Arm, ExprArray, ExprConst, ExprGroup, ExprIf, ExprMatch, ExprRange, ExprRawAddr, ExprReference,
+    ExprRepeat, ExprStruct, ExprTuple, ExprUnsafe, FieldValue,
+};
 
 /// Check if something is a valid Rust lvalue. Inspired by `librustc::ty::expr_is_lval`.
 fn is_lvalue(e: &Expr) -> bool {
@@ -28,12 +32,95 @@ fn is_simple_lvalue(e: &Expr) -> bool {
             ref expr,
             ..
         })
-        | Field(ExprField { base: ref expr, .. })
-        | Index(ExprIndex { ref expr, .. }) => is_simple_lvalue(expr),
+        | Field(ExprField { base: ref expr, .. }) => is_simple_lvalue(expr),
+        Index(ExprIndex {
+            ref expr,
+            ref index,
+            ..
+        }) => is_simple_lvalue(expr) && !expr_has_side_effects(index),
         _ => false,
     }
 }
 
+/// Returns whether the expression has (or could have) any side effects.
+/// This is pessimistic, and does not cover all expression types currently.
+fn expr_has_side_effects(expr: &Expr) -> bool {
+    use Expr::*;
+
+    match unparen(expr) {
+        Array(ExprArray { elems, .. }) => elems.iter().any(|elem| expr_has_side_effects(elem)),
+        Cast(ExprCast { expr, .. }) => expr_has_side_effects(expr),
+        Binary(ExprBinary { left, right, .. }) => {
+            expr_has_side_effects(left) || expr_has_side_effects(right)
+        }
+        Block(ExprBlock { block, .. }) => block_has_side_effects(block),
+        Const(ExprConst { block, .. }) => block_has_side_effects(block),
+        Field(ExprField { base, .. }) => expr_has_side_effects(base),
+        Group(ExprGroup { expr, .. }) => expr_has_side_effects(expr),
+        Index(ExprIndex { expr, index, .. }) => {
+            expr_has_side_effects(expr) || expr_has_side_effects(index)
+        }
+        Infer(_) => false,
+        If(ExprIf {
+            cond,
+            then_branch,
+            else_branch,
+            ..
+        }) => {
+            expr_has_side_effects(cond)
+                || block_has_side_effects(then_branch)
+                || else_branch
+                    .as_ref()
+                    .map_or(false, |(_, expr)| expr_has_side_effects(expr))
+        }
+        Lit(_) => false,
+        Match(ExprMatch { expr, arms, .. }) => {
+            expr_has_side_effects(expr)
+                || arms.iter().any(|Arm { guard, body, .. }| {
+                    guard
+                        .as_ref()
+                        .map_or(false, |(_, expr)| expr_has_side_effects(expr))
+                        || expr_has_side_effects(body)
+                })
+        }
+        Paren(ExprParen { expr, .. }) => expr_has_side_effects(expr),
+        Path(_) => false,
+        Range(ExprRange { start, end, .. }) => {
+            start
+                .as_ref()
+                .map_or(false, |expr| expr_has_side_effects(expr))
+                || end
+                    .as_ref()
+                    .map_or(false, |expr| expr_has_side_effects(expr))
+        }
+        RawAddr(ExprRawAddr { expr, .. }) => expr_has_side_effects(expr),
+        Reference(ExprReference { expr, .. }) => expr_has_side_effects(expr),
+        Repeat(ExprRepeat { expr, len, .. }) => {
+            expr_has_side_effects(expr) || expr_has_side_effects(len)
+        }
+        Struct(ExprStruct { fields, rest, .. }) => {
+            fields
+                .iter()
+                .any(|FieldValue { expr, .. }| expr_has_side_effects(expr))
+                || rest
+                    .as_ref()
+                    .map_or(false, |rest| expr_has_side_effects(rest))
+        }
+        Tuple(ExprTuple { elems, .. }) => elems.iter().any(|elem| expr_has_side_effects(elem)),
+        Unary(ExprUnary { expr, .. }) => expr_has_side_effects(expr),
+        Unsafe(ExprUnsafe { block, .. }) => block_has_side_effects(block),
+        _ => true,
+    }
+}
+
+fn block_has_side_effects(block: &Block) -> bool {
+    use Stmt::*;
+    block.stmts.iter().any(|stmt| match stmt {
+        Expr(expr, _) => expr_has_side_effects(expr),
+        _ => true,
+    })
+}
+
 pub struct NamedReference<R> {
     pub lvalue: Box<Expr>,
     pub rvalue: R,

c2rust-transpile/tests/snapshots/snapshots__transpile@exprs.c.2021.snap

@@ -49,10 +49,14 @@ pub unsafe extern "C" fn unary_with_side_effect() {
     (b"\0".as_ptr() as *const ::core::ffi::c_char).offset(side_effect() as isize)
         as *const ::core::ffi::c_char;
     *arr[side_effect() as usize];
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(1);
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(-1);
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(1);
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(-1);
+    let ref mut c2rust_fresh0 = arr[side_effect() as usize];
+    *c2rust_fresh0 = (*c2rust_fresh0).offset(1);
+    let ref mut c2rust_fresh1 = arr[side_effect() as usize];
+    *c2rust_fresh1 = (*c2rust_fresh1).offset(-1);
+    let ref mut c2rust_fresh2 = arr[side_effect() as usize];
+    *c2rust_fresh2 = (*c2rust_fresh2).offset(1);
+    let ref mut c2rust_fresh3 = arr[side_effect() as usize];
+    *c2rust_fresh3 = (*c2rust_fresh3).offset(-1);
 }
 #[no_mangle]
 pub unsafe extern "C" fn unsigned_compound_desugaring() {

c2rust-transpile/tests/snapshots/snapshots__transpile@exprs.c.2024.snap

@@ -49,10 +49,14 @@ pub unsafe extern "C" fn unary_with_side_effect() {
     (b"\0".as_ptr() as *const ::core::ffi::c_char).offset(side_effect() as isize)
         as *const ::core::ffi::c_char;
     *arr[side_effect() as usize];
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(1);
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(-1);
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(1);
-    arr[side_effect() as usize] = arr[side_effect() as usize].offset(-1);
+    let ref mut c2rust_fresh0 = arr[side_effect() as usize];
+    *c2rust_fresh0 = (*c2rust_fresh0).offset(1);
+    let ref mut c2rust_fresh1 = arr[side_effect() as usize];
+    *c2rust_fresh1 = (*c2rust_fresh1).offset(-1);
+    let ref mut c2rust_fresh2 = arr[side_effect() as usize];
+    *c2rust_fresh2 = (*c2rust_fresh2).offset(1);
+    let ref mut c2rust_fresh3 = arr[side_effect() as usize];
+    *c2rust_fresh3 = (*c2rust_fresh3).offset(-1);
 }
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn unsigned_compound_desugaring() {