transpile: In name_reference, also check expression for side effects

c2rust-transpile/src/translator/mod.rs

@@ -18,11 +18,11 @@ use proc_macro2::{Punct, Spacing::*, Span, TokenStream, TokenTree};
 use syn::spanned::Spanned as _;
 use syn::{
     AttrStyle, BareVariadic, BinOp, Block, Expr, ExprBinary, ExprBlock, ExprBreak, ExprCast,
-    ExprField, ExprIndex, ExprParen, ExprReturn, ExprUnary, FnArg, ForeignItem, ForeignItemFn,
-    ForeignItemMacro, ForeignItemStatic, ForeignItemType, Ident, Item, ItemConst, ItemEnum,
-    ItemExternCrate, ItemFn, ItemForeignMod, ItemImpl, ItemMacro, ItemMod, ItemStatic, ItemStruct,
-    ItemTrait, ItemTraitAlias, ItemType, ItemUnion, ItemUse, Lit, MacroDelimiter, PathSegment,
-    ReturnType, Stmt, Type, TypeTuple, UnOp, UseTree, Visibility,
+    ExprParen, ExprReturn, ExprUnary, FnArg, ForeignItem, ForeignItemFn, ForeignItemMacro,
+    ForeignItemStatic, ForeignItemType, Ident, Item, ItemConst, ItemEnum, ItemExternCrate, ItemFn,
+    ItemForeignMod, ItemImpl, ItemMacro, ItemMod, ItemStatic, ItemStruct, ItemTrait,
+    ItemTraitAlias, ItemType, ItemUnion, ItemUse, Lit, MacroDelimiter, PathSegment, ReturnType,
+    Stmt, Type, TypeTuple, UnOp, UseTree, Visibility,
 };
 
 use crate::diagnostics::TranslationResult;
@@ -3341,10 +3341,12 @@ impl<'c> Translation<'c> {
             }
 
             BinaryConditional(ty, lhs, rhs) => {
+                let rhs = self.convert_expr(ctx, rhs, None)?;
+
                 if ctx.is_unused() {
-                    let mut lhs = self.convert_condition(ctx, false, lhs)?;
-                    let rhs = self.convert_expr(ctx, rhs, None)?;
-                    lhs = lhs.merge_unsafe(rhs.is_unsafe());
+                    let lhs = self
+                        .convert_condition(ctx, false, lhs)?
+                        .merge_unsafe(rhs.is_unsafe());
 
                     Ok(lhs.and_then(|val| {
                         WithStmts::new(
@@ -3359,19 +3361,28 @@ impl<'c> Translation<'c> {
                         )
                     }))
                 } else {
-                    self.name_reference_write_read(ctx, lhs)?.try_map(
-                        |NamedReference {
-                             rvalue: lhs_val, ..
-                         }| {
-                            let cond = self.match_bool(ctx, true, ty.ctype, lhs_val.clone())?;
-                            let ite = mk().ifte_expr(
-                                cond,
-                                mk().block(vec![mk().expr_stmt(lhs_val)]),
-                                Some(self.convert_expr(ctx, rhs, None)?.to_expr()),
-                            );
-                            Ok(ite)
-                        },
-                    )
+                    let lhs = self
+                        .convert_expr(ctx.used(), lhs, None)?
+                        .merge_unsafe(rhs.is_unsafe());
+                    let fresh_name = self.renamer.borrow_mut().fresh();
+
+                    lhs.and_then_try(|lhs| {
+                        let fresh_stmt = mk().local_stmt(Box::new(mk().local(
+                            mk().ident_pat(&fresh_name),
+                            None,
+                            Some(lhs),
+                        )));
+
+                        let cond =
+                            self.match_bool(ctx, true, ty.ctype, mk().ident_expr(&fresh_name))?;
+                        let ite = mk().ifte_expr(
+                            cond,
+                            mk().block(vec![mk().expr_stmt(mk().ident_expr(&fresh_name))]),
+                            Some(rhs.to_expr()),
+                        );
+
+                        Ok(WithStmts::new(vec![fresh_stmt], ite))
+                    })
                 }
             }
 

c2rust-transpile/src/translator/named_references.rs

@@ -18,22 +18,6 @@ fn is_lvalue(e: &Expr) -> bool {
     )
 }
 
-/// Check if something is a side-effect free Rust lvalue.
-fn is_simple_lvalue(e: &Expr) -> bool {
-    use Expr::*;
-    match *unparen(e) {
-        Path(..) => true,
-        Unary(ExprUnary {
-            op: syn::UnOp::Deref(_),
-            ref expr,
-            ..
-        })
-        | Field(ExprField { base: ref expr, .. })
-        | Index(ExprIndex { ref expr, .. }) => is_simple_lvalue(expr),
-        _ => false,
-    }
-}
-
 pub struct NamedReference<R> {
     pub lvalue: Box<Expr>,
     pub rvalue: R,
@@ -106,29 +90,29 @@ impl<'c> Translation<'c> {
             .kind
             .get_qual_type()
             .ok_or_else(|| format_err!("bad reference type"))?;
+
+        let is_pure = self.ast_context.is_expr_pure(reference);
         let read = |write| self.read(reference_ty, write);
         let reference = self.convert_expr(ctx.used(), reference, Some(reference_ty))?;
         reference.and_then_try(|reference| {
-            if !uses_read && is_lvalue(&reference) {
+            if is_lvalue(&reference) && (is_pure || !uses_read) {
+                let rvalue = uses_read.then(|| read(reference.clone())).transpose()?;
+
                 Ok(WithStmts::new_val(NamedReference {
                     lvalue: reference,
-                    rvalue: None,
-                }))
-            } else if is_simple_lvalue(&reference) {
-                Ok(WithStmts::new_val(NamedReference {
-                    lvalue: reference.clone(),
-                    rvalue: Some(read(reference)?),
+                    rvalue,
                 }))
             } else {
                 // This is the case where we explicitly need to factor out possible side-effects.
 
-                let ptr_name = self.renamer.borrow_mut().fresh();
+                let ptr_name = self.renamer.borrow_mut().pick_name("c2rust_lvalue");
 
-                // let ref mut p = lhs;
+                // let p = &raw mut lhs;
+                self.use_feature("raw_ref_op");
                 let compute_ref = mk().local_stmt(Box::new(mk().local(
-                    mk().mutbl().ident_ref_pat(&ptr_name),
+                    mk().ident_pat(&ptr_name),
                     None,
-                    Some(reference),
+                    Some(mk().mutbl().raw_borrow_expr(reference)),
                 )));
 
                 let write =

c2rust-transpile/src/translator/operators.rs

@@ -212,7 +212,7 @@ impl<'c> Translation<'c> {
             )))
         } else {
             let lhs = self.convert_cast(
-                ctx,
+                ctx.used(),
                 initial_lhs_type_id,
                 compute_lhs_type_id,
                 WithStmts::new_val(read.clone()),
@@ -234,8 +234,15 @@ impl<'c> Translation<'c> {
                 )
             })?;
 
-            let val =
-                self.convert_cast(ctx, compute_res_type_id, lhs_type_id, val, None, None, None)?;
+            let val = self.convert_cast(
+                ctx.used(),
+                compute_res_type_id,
+                lhs_type_id,
+                val,
+                None,
+                None,
+                None,
+            )?;
 
             Ok(val.map(|val| mk().assign_expr(write.clone(), val)))
         }
@@ -424,7 +431,7 @@ impl<'c> Translation<'c> {
                         .expect("Cannot convert non-assignment operator");
 
                     let lhs = self.convert_cast(
-                        ctx,
+                        ctx.used(),
                         initial_lhs_type_id,
                         expr_or_comp_type_id,
                         WithStmts::new_val(read.clone()),
@@ -447,7 +454,7 @@ impl<'c> Translation<'c> {
                     )?;
 
                     let val = self.convert_cast(
-                        ctx,
+                        ctx.used(),
                         result_type_id,
                         expr_type_id,
                         val,
@@ -650,7 +657,7 @@ impl<'c> Translation<'c> {
             };
 
         self.convert_assignment_operator_with_rhs(
-            ctx.used(),
+            ctx,
             op,
             ty,
             arg,
@@ -789,7 +796,7 @@ impl<'c> Translation<'c> {
                 .map(|a| mk().unary_expr(UnOp::Not(Default::default()), a))),
 
             CUnOp::Not => {
-                let val = self.convert_condition(ctx, false, arg)?;
+                let val = self.convert_condition(ctx.used(), false, arg)?;
                 Ok(val.map(|x| mk().cast_expr(x, mk().abs_path_ty(vec!["core", "ffi", "c_int"]))))
             }
             CUnOp::Extension => {

c2rust-transpile/tests/snapshots.rs

@@ -354,6 +354,11 @@ fn test_compound_literals() {
     transpile("compound_literals.c").run();
 }
 
+#[test]
+fn test_conditions() {
+    transpile("conditions.c").run();
+}
+
 #[test]
 fn test_empty_init() {
     transpile("empty_init.c").run();

c2rust-transpile/tests/snapshots/if_else.c → c2rust-transpile/tests/snapshots/conditions.c

@@ -83,3 +83,22 @@ int nested_early_returns(int x) {
     }
     return x;
 }
+
+void conditional_operator(const unsigned sz, int buf[const]) {
+    int x = 0, y = 1;
+    *(0 ? &y : &x) = 10;
+
+    buf[2] = 1 ? 2 : 3;
+    buf[3] = 0 ? 2 : 3;
+}
+
+static int id(int i) { return i;}
+static int add(int *p, int i, int r) { *p += i; return r;}
+
+void binary_conditional_operator(const unsigned sz, int buf[const]) {
+    buf[0] = id(0) ?: id(1);
+    buf[1] = id(2) ?: id(3);
+
+    (void) (add(buf+2, 2, 0) ?: add(buf+3, 3, 0));
+    (void) (add(buf+4, 4, 1) ?: add(buf+5, 5, 0));
+}

c2rust-transpile/tests/snapshots/exprs.c

@@ -2,7 +2,13 @@ int puts(const char *str);
 
 static int side_effect(){
     puts("the return of side effect");
-    return 10;
+    return 0;
+}
+
+static int* lvalue_side_effect(){
+    puts("the return of side effect");
+    static int VAL = 42;
+    return &VAL;
 }
 
 void unary_without_side_effect(){
@@ -20,18 +26,43 @@ void unary_without_side_effect(){
 }
 
 void unary_with_side_effect(){
-    char* arr[1] = {0};
+    char *arr[1] = {0};
 
     -side_effect();
     +side_effect();
     ~side_effect();
     !side_effect();
     &""[side_effect()];
-    *arr[side_effect()];
+}
+
+void inc_decl_with_rvalue_side_effect() {
+    int arr[1] = {0};
+
+    // Increment/decrement, expression value not used
     ++arr[side_effect()];
     --arr[side_effect()];
     arr[side_effect()]++;
     arr[side_effect()]--;
+
+    // Increment/decrement, expression value is used
+    int pre_inc = ++arr[side_effect()];
+    int pre_dec = --arr[side_effect()];
+    int post_inc = arr[side_effect()]++;
+    int post_dec = arr[side_effect()]--;
+}
+
+void inc_decl_with_lvalue_side_effect() {
+    // Increment/decrement, expression value not used
+    ++*lvalue_side_effect();
+    --*lvalue_side_effect();
+    (*lvalue_side_effect())++;
+    (*lvalue_side_effect())--;
+
+    // Increment/decrement, expression value is used
+    int pre_inc = ++*lvalue_side_effect();
+    int pre_dec = --*lvalue_side_effect();
+    int post_inc = (*lvalue_side_effect())++;
+    int post_dec = (*lvalue_side_effect())--;
 }
 
 void unsigned_compound_desugaring(void) {