Use fresh rvalues for comma operands and materialized temporaries (#461)

Fixes miscompilations, rvalue comma operands should be cloned, not
moved.

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -2776,7 +2776,11 @@ void Converter::ConvertBinaryOperator(clang::BinaryOperator *expr) {
       Convert(lhs);
     }
     StrCat(token::kSemiColon);
-    Convert(rhs);
+    if (isRValue()) {
+      StrCat(ConvertFreshRValue(rhs));
+    } else {
+      Convert(rhs);
+    }
   } else if (IsUnsignedArithOp(expr)) {
     if (expr->isCompoundAssignmentOp()) {
       Convert(lhs);

cpp2rust/converter/models/converter_refcount.cpp

@@ -496,7 +496,7 @@ ConverterRefCount::MaterializeTemp(const std::string &binding_name,
                                    clang::QualType param_type,
                                    clang::Expr *expr) {
   auto pointee = param_type.getNonReferenceType();
-  auto value = ConvertRValue(expr, pointee);
+  auto value = ConvertFreshRValue(expr, pointee);
   auto type_str = ToStringBase(pointee);
   const auto *decl = in_const_initializer_ ? keyword::kStatic : keyword::kLet;
 

tests/unit/comma_operator.cpp

@@ -1,4 +1,14 @@
 #include <cassert>
+#include <vector>
+
+struct Pair {
+  int first;
+  int second;
+};
+
+long twice(const long &v) { return v * 2; }
+
+int sum(const Pair &p) { return p.first + p.second; }
 
 int main() {
   int x = 1;
@@ -20,5 +30,22 @@ int main() {
     assert(b == 2);
   }
 
+  std::vector<int> v1 = {1, 2};
+  std::vector<int> v2 = (a = 5, v1);
+  v2.push_back(3);
+  assert(a == 5);
+  assert(v1.size() == 2);
+  assert(v2.size() == 3);
+
+  Pair p1{1, 2};
+  Pair p2 = (b = 6, p1);
+  p2.first = 10;
+  assert(p1.first == 1);
+  assert(p2.first == 10);
+
+  assert(twice(a) == 10);
+  assert(sum((a = 7, p1)) == 3);
+  assert(a == 7);
+
   return 0;
 }

tests/unit/out/refcount/comma_operator.rs

@@ -6,6 +6,20 @@ use std::io::prelude::*;
 use std::io::{Read, Seek, Write};
 use std::os::fd::AsFd;
 use std::rc::{Rc, Weak};
+#[derive(Clone, Record, ByteRepr, VaArg, FnPtrArg, Default)]
+#[byte_size(8)]
+pub struct Pair {
+    #[offset(0)]
+    pub first: i32,
+    #[offset(4)]
+    pub second: i32,
+}
+pub fn twice_0(v: Ptr<i64>) -> i64 {
+    return ((v.read()) * 2_i64);
+}
+pub fn sum_1(p: Ptr<Pair>) -> i32 {
+    return ({ p.with(|__s| __s.first) } + { p.with(|__s| __s.second) });
+}
 pub fn main() {
     __cpp2rust_init_globals();
     std::process::exit(main_0());
@@ -48,6 +62,45 @@ fn main_0() -> i32 {
         assert!((a == 1));
         assert!((b == 2));
     }
+    let v1: Value<Vec<i32>> = Rc::new(RefCell::new(vec![1, 2]));
+    let mut v2: Vec<i32> = {
+        a = 5;
+        (*v1.borrow()).clone()
+    }
+    .clone();
+    {
+        let __a1 = 3;
+        v2.push(__a1)
+    };
+    assert!((a == 5));
+    assert!(((*v1.borrow()).len() == 2_usize));
+    assert!((v2.len() == 3_usize));
+    let p1: Value<Pair> = Rc::new(RefCell::new(Pair {
+        first: 1,
+        second: 2,
+    }));
+    let mut p2: Pair = {
+        b = 6;
+        (*p1.borrow()).clone()
+    };
+    p2.first = 10;
+    assert!(({ (*p1.borrow()).first } == 1));
+    assert!((p2.first == 10));
+    assert!(
+        (({
+            let _v: Value<i64> = Rc::new(RefCell::new((a as i64)));
+            twice_0(_v.as_pointer())
+        }) == 10_i64)
+    );
+    assert!(
+        (({
+            sum_1({
+                a = 7;
+                p1.as_pointer()
+            })
+        }) == 3)
+    );
+    assert!((a == 7));
     return 0;
 }
 pub fn __cpp2rust_init_globals() {}

tests/unit/out/unsafe/comma_operator.rs

@@ -6,6 +6,18 @@ use std::collections::BTreeMap;
 use std::io::{Read, Seek, Write};
 use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
 use std::rc::Rc;
+#[repr(C)]
+#[derive(Copy, Clone, VaArg, FnPtrArg, Default)]
+pub struct Pair {
+    pub first: i32,
+    pub second: i32,
+}
+pub unsafe fn twice_0(v: *const i64) -> i64 {
+    return ((*v) * (2_i64));
+}
+pub unsafe fn sum_1(p: *const Pair) -> i32 {
+    return (((*p).first) + ((*p).second));
+}
 pub fn main() {
     unsafe {
         __cpp2rust_init_globals();
@@ -50,6 +62,45 @@ unsafe fn main_0() -> i32 {
         assert!(((a) == (1)));
         assert!(((b) == (2)));
     }
+    let mut v1: Vec<i32> = vec![1, 2];
+    let mut v2: Vec<i32> = {
+        a = 5;
+        (v1).clone()
+    }
+    .clone();
+    {
+        let __a1 = 3;
+        v2.push(__a1)
+    };
+    assert!(((a) == (5)));
+    assert!(((v1.len()) == (2_usize)));
+    assert!(((v2.len()) == (3_usize)));
+    let mut p1: Pair = Pair {
+        first: 1,
+        second: 2,
+    };
+    let mut p2: Pair = {
+        b = 6;
+        p1
+    };
+    p2.first = 10;
+    assert!(((p1.first) == (1)));
+    assert!(((p2.first) == (10)));
+    assert!(
+        ((unsafe {
+            let mut _v: i64 = (a as i64);
+            twice_0(&mut _v)
+        }) == (10_i64))
+    );
+    assert!(
+        ((unsafe {
+            sum_1(&{
+                a = 7;
+                p1
+            })
+        }) == (3))
+    );
+    assert!(((a) == (7)));
     return 0;
 }
 pub unsafe fn __cpp2rust_init_globals() {}