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;
@@ -3351,9 +3351,10 @@ 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.merge_unsafe(rhs.is_unsafe());
 
                     Ok(lhs.and_then(|val| {
@@ -3369,19 +3370,27 @@ 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 mut lhs = self.convert_expr(ctx.used(), lhs, None)?;
+                    lhs.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 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

@@ -214,7 +214,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()),
@@ -236,8 +236,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)))
         }
@@ -438,7 +445,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()),
@@ -461,7 +468,7 @@ impl<'c> Translation<'c> {
                     )?;
 
                     let val = self.convert_cast(
-                        ctx,
+                        ctx.used(),
                         result_type_id,
                         expr_type_id,
                         val,
@@ -674,7 +681,7 @@ impl<'c> Translation<'c> {
             };
 
         self.convert_assignment_operator_with_rhs(
-            ctx.used(),
+            ctx,
             op,
             ty,
             arg,
@@ -814,7 +821,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

@@ -334,6 +334,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) {

c2rust-transpile/tests/snapshots/snapshots__transpile@if_else.c.snap → c2rust-transpile/tests/snapshots/snapshots__transpile@conditions.c.2021.snap

@@ -1,7 +1,6 @@
 ---
 source: c2rust-transpile/tests/snapshots.rs
-expression: cat tests/snapshots/if_else.rs
-input_file: c2rust-transpile/tests/snapshots/if_else.c
+expression: cat tests/snapshots/conditions.2021.rs
 ---
 #![allow(
     clippy::missing_safety_doc,
@@ -12,6 +11,7 @@ input_file: c2rust-transpile/tests/snapshots/if_else.c
     unused_assignments,
     unused_mut
 )]
+#![feature(raw_ref_op)]
 #[no_mangle]
 pub unsafe extern "C" fn simple_if_else(mut x: ::core::ffi::c_int) -> ::core::ffi::c_int {
     if x != 0 {
@@ -93,3 +93,77 @@ pub unsafe extern "C" fn nested_early_returns(mut x: ::core::ffi::c_int) -> ::co
     }
     return x;
 }
+#[no_mangle]
+pub unsafe extern "C" fn conditional_operator(
+    sz: ::core::ffi::c_uint,
+    buf: *mut ::core::ffi::c_int,
+) {
+    let mut x: ::core::ffi::c_int = 0 as ::core::ffi::c_int;
+    let mut y: ::core::ffi::c_int = 1 as ::core::ffi::c_int;
+    *if false { &raw mut y } else { &raw mut x } = 10 as ::core::ffi::c_int;
+    *(buf as *mut ::core::ffi::c_int).offset(2 as ::core::ffi::c_int as isize) = if true {
+        2 as ::core::ffi::c_int
+    } else {
+        3 as ::core::ffi::c_int
+    };
+    *(buf as *mut ::core::ffi::c_int).offset(3 as ::core::ffi::c_int as isize) = if false {
+        2 as ::core::ffi::c_int
+    } else {
+        3 as ::core::ffi::c_int
+    };
+}
+unsafe extern "C" fn id(mut i: ::core::ffi::c_int) -> ::core::ffi::c_int {
+    return i;
+}
+unsafe extern "C" fn add(
+    mut p: *mut ::core::ffi::c_int,
+    mut i: ::core::ffi::c_int,
+    mut r: ::core::ffi::c_int,
+) -> ::core::ffi::c_int {
+    *p += i;
+    return r;
+}
+#[no_mangle]
+pub unsafe extern "C" fn binary_conditional_operator(
+    sz: ::core::ffi::c_uint,
+    buf: *mut ::core::ffi::c_int,
+) {
+    let c2rust_fresh0 = id(0 as ::core::ffi::c_int);
+    *(buf as *mut ::core::ffi::c_int).offset(0 as ::core::ffi::c_int as isize) =
+        if c2rust_fresh0 != 0 {
+            c2rust_fresh0
+        } else {
+            id(1 as ::core::ffi::c_int)
+        };
+    let c2rust_fresh1 = id(2 as ::core::ffi::c_int);
+    *(buf as *mut ::core::ffi::c_int).offset(1 as ::core::ffi::c_int as isize) =
+        if c2rust_fresh1 != 0 {
+            c2rust_fresh1
+        } else {
+            id(3 as ::core::ffi::c_int)
+        };
+    if add(
+        (buf as *mut ::core::ffi::c_int).offset(2 as ::core::ffi::c_int as isize),
+        2 as ::core::ffi::c_int,
+        0 as ::core::ffi::c_int,
+    ) == 0
+    {
+        add(
+            (buf as *mut ::core::ffi::c_int).offset(3 as ::core::ffi::c_int as isize),
+            3 as ::core::ffi::c_int,
+            0 as ::core::ffi::c_int,
+        );
+    }
+    if add(
+        (buf as *mut ::core::ffi::c_int).offset(4 as ::core::ffi::c_int as isize),
+        4 as ::core::ffi::c_int,
+        1 as ::core::ffi::c_int,
+    ) == 0
+    {
+        add(
+            (buf as *mut ::core::ffi::c_int).offset(5 as ::core::ffi::c_int as isize),
+            5 as ::core::ffi::c_int,
+            0 as ::core::ffi::c_int,
+        );
+    }
+}