Avoid hoisting for function call arguments that don't alias

cpp2rust/converter/converter.cpp

@@ -1681,6 +1681,24 @@ Converter::CallInfo Converter::CollectCallInfo(clang::CallExpr *expr) {
     }
   }
 
+  // Inline arguments that don't alias
+  clang::Expr *receiver = GetCallObject(expr);
+  for (auto &ca : info.args) {
+    if (ca.kind != Kind::Hoisted) {
+      continue;
+    }
+    bool aliases = receiver && ArgsMayAlias(ca.expr, receiver);
+    for (const auto &other : info.args) {
+      if (&other != &ca && ArgsMayAlias(ca.expr, other.expr)) {
+        aliases = true;
+        break;
+      }
+    }
+    if (!aliases) {
+      ca.kind = Kind::Inline;
+    }
+  }
+
   return info;
 }
 

cpp2rust/converter/converter_lib.cpp

@@ -13,6 +13,7 @@
 #include <cctype>
 #include <filesystem>
 #include <format>
+#include <ranges>
 #include <unordered_set>
 
 #include "converter/lex.h"
@@ -643,6 +644,78 @@ clang::Expr *GetCallObject(clang::CallExpr *expr) {
   return nullptr;
 }
 
+static void GetAllVarsImpl(const clang::Stmt *stmt,
+                           std::unordered_set<const clang::ValueDecl *> &vars) {
+  if (!stmt) {
+    return;
+  }
+
+  if (auto *decl_ref = clang::dyn_cast<clang::DeclRefExpr>(stmt)) {
+    vars.insert(decl_ref->getDecl());
+  } else if (auto *member = clang::dyn_cast<clang::MemberExpr>(stmt)) {
+    vars.insert(member->getMemberDecl());
+    GetAllVarsImpl(member->getBase(), vars);
+  }
+
+  for (auto *child : stmt->children()) {
+    GetAllVarsImpl(child, vars);
+  }
+}
+
+std::unordered_set<const clang::ValueDecl *>
+GetAllVars(const clang::Stmt *stmt) {
+  std::unordered_set<const clang::ValueDecl *> vars;
+  GetAllVarsImpl(stmt, vars);
+  return vars;
+}
+
+bool ReferencesThis(const clang::Stmt *stmt) {
+  if (!stmt) {
+    return false;
+  }
+  if (clang::isa<clang::CXXThisExpr>(stmt)) {
+    return true;
+  }
+  for (auto *child : stmt->children()) {
+    if (ReferencesThis(child)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+bool MayCauseBorrowMutError(const clang::Expr *lhs, const clang::Expr *rhs) {
+  auto lhs_vars = GetAllVars(lhs);
+  auto rhs_vars = GetAllVars(rhs);
+
+  auto predicate = [lhs](auto *var) {
+    auto qual_type = var->getType();
+    return (qual_type->isPointerType() || qual_type->isReferenceType()) &&
+           qual_type->getPointeeType()
+                   .getCanonicalType()
+                   .getUnqualifiedType() ==
+               lhs->getType().getCanonicalType().getUnqualifiedType();
+  };
+
+  if (std::ranges::any_of(rhs_vars, predicate) ||
+      (std::ranges::any_of(lhs_vars, predicate) && !rhs_vars.empty())) {
+    return true;
+  }
+
+  for (auto *lhs_var : lhs_vars) {
+    if (rhs_vars.count(lhs_var))
+      return true;
+  }
+  return false;
+}
+
+bool ArgsMayAlias(const clang::Expr *a, const clang::Expr *b) {
+  if (ReferencesThis(a) && ReferencesThis(b)) {
+    return true;
+  }
+  return MayCauseBorrowMutError(a, b) || MayCauseBorrowMutError(b, a);
+}
+
 std::vector<clang::Expr *>
 BuildUnifiedArgs(clang::Expr *expr, clang::Expr **args, unsigned num_args) {
   std::vector<clang::Expr *> all_args;

cpp2rust/converter/converter_lib.h

@@ -12,6 +12,7 @@
 #include <optional>
 #include <string>
 #include <string_view>
+#include <unordered_set>
 #include <vector>
 
 #include "logging.h"
@@ -139,6 +140,15 @@ void ForEachTemplateArgument(
 
 clang::Expr *GetCallObject(clang::CallExpr *expr);
 
+std::unordered_set<const clang::ValueDecl *>
+GetAllVars(const clang::Stmt *stmt);
+
+bool ReferencesThis(const clang::Stmt *stmt);
+
+bool MayCauseBorrowMutError(const clang::Expr *lhs, const clang::Expr *rhs);
+
+bool ArgsMayAlias(const clang::Expr *a, const clang::Expr *b);
+
 clang::Expr *GetCalleeOrExpr(clang::Expr *expr);
 
 bool HasReceiver(clang::Expr *expr);

cpp2rust/converter/models/converter_refcount.cpp

@@ -1826,51 +1826,6 @@ void ConverterRefCount::ConvertVarInit(clang::QualType qual_type,
   }
 }
 
-static std::unordered_set<const clang::ValueDecl *>
-GetAllVars(const clang::Stmt *stmt) {
-  std::unordered_set<const clang::ValueDecl *> vars;
-
-  if (auto *decl_ref = clang::dyn_cast<clang::DeclRefExpr>(stmt)) {
-    vars.insert(decl_ref->getDecl());
-  } else if (auto *member = clang::dyn_cast<clang::MemberExpr>(stmt)) {
-    vars.insert(member->getMemberDecl());
-    auto child_vars = GetAllVars(member->getBase());
-    vars.insert(child_vars.begin(), child_vars.end());
-  }
-
-  for (auto *child : stmt->children()) {
-    auto child_vars = GetAllVars(child);
-    vars.insert(child_vars.begin(), child_vars.end());
-  }
-  return vars;
-}
-
-bool ConverterRefCount::MayCauseBorrowMutError(const clang::Expr *lhs,
-                                               const clang::Expr *rhs) {
-  auto lhs_vars = GetAllVars(lhs);
-  auto rhs_vars = GetAllVars(rhs);
-
-  auto predicate = [lhs](auto *var) {
-    auto qual_type = var->getType();
-    return (qual_type->isPointerType() || qual_type->isReferenceType()) &&
-           qual_type->getPointeeType()
-                   .getCanonicalType()
-                   .getUnqualifiedType() ==
-               lhs->getType().getCanonicalType().getUnqualifiedType();
-  };
-
-  if (std::ranges::any_of(rhs_vars, predicate) ||
-      (std::ranges::any_of(lhs_vars, predicate) && !rhs_vars.empty())) {
-    return true;
-  }
-
-  for (auto *lhs_var : lhs_vars) {
-    if (rhs_vars.count(lhs_var))
-      return true;
-  }
-  return false;
-}
-
 void ConverterRefCount::EmitSetOrAssign(clang::Expr *lhs,
                                         std::string_view rhs) {
   auto lhs_str = ConvertLValue(lhs);

cpp2rust/converter/models/converter_refcount.h

@@ -142,8 +142,6 @@ class ConverterRefCount final : public Converter {
   std::vector<const char *>
   GetStructAttributes(const clang::RecordDecl *decl) override;
 
-  bool MayCauseBorrowMutError(const clang::Expr *lhs, const clang::Expr *rhs);
-
   bool Convert(clang::QualType qual_type) override;
   bool
   Convert(clang::Expr *expr,

tests/multi-file/opaque_forward_decl/out/refcount/opaque_forward_decl.rs

@@ -20,10 +20,7 @@ fn main_0() -> i32 {
         p: Rc::new(RefCell::new(Ptr::<opaque>::null())),
         x: Rc::new(RefCell::new(42)),
     }));
-    ({
-        let _c: Ptr<container> = (c.as_pointer());
-        touch_0(_c)
-    });
+    ({ touch_0((c.as_pointer())) });
     assert!(((((*(*c.borrow()).x.borrow()) == 42) as i32) != 0));
     assert!(((((*(*c.borrow()).p.borrow()).is_null()) as i32) != 0));
     return 0;

tests/multi-file/opaque_forward_decl/out/unsafe/opaque_forward_decl.rs

@@ -22,10 +22,7 @@ unsafe fn main_0() -> i32 {
         p: std::ptr::null_mut(),
         x: 42,
     };
-    (unsafe {
-        let _c: *mut container = (&mut c as *mut container);
-        touch_0(_c)
-    });
+    (unsafe { touch_0((&mut c as *mut container)) });
     assert!(((((c.x) == (42)) as i32) != 0));
     assert!(((((c.p).is_null()) as i32) != 0));
     return 0;

tests/ub/out/refcount/dangling-prvalue-as-lvalue.rs

@@ -14,10 +14,7 @@ pub fn main() {
 }
 fn main_0() -> i32 {
     let v: Value<Vec<i32>> = Rc::new(RefCell::new(vec![1, 2]));
-    let b: Ptr<i32> = ({
-        let _a: Ptr<i32> = (v.as_pointer() as Ptr<i32>);
-        foo_0(_a)
-    });
+    let b: Ptr<i32> = ({ foo_0((v.as_pointer() as Ptr<i32>)) });
     (*v.borrow_mut()).clear();
     return (b.read());
 }

tests/ub/out/refcount/ub12.rs

@@ -15,10 +15,7 @@ pub fn main() {
 }
 fn main_0() -> i32 {
     let alloc: Value<Ptr<i32>> = Rc::new(RefCell::new(Ptr::alloc(1)));
-    ({
-        let _ptr: Ptr<i32> = (*alloc.borrow()).clone();
-        escape_0(_ptr)
-    });
+    ({ escape_0((*alloc.borrow()).clone()) });
     (*alloc.borrow()).delete();
     return 0;
 }

tests/ub/out/refcount/ub13.rs

@@ -15,9 +15,6 @@ pub fn main() {
 }
 fn main_0() -> i32 {
     let p1: Value<Ptr<i32>> = Rc::new(RefCell::new(Ptr::alloc(1)));
-    ({
-        let _p: Ptr<i32> = (*p1.borrow()).clone();
-        escape_0(_p)
-    });
+    ({ escape_0((*p1.borrow()).clone()) });
     return ((*p1.borrow()).read());
 }

tests/ub/out/refcount/ub16.rs

@@ -20,12 +20,9 @@ fn main_0() -> i32 {
             .collect::<Box<[i32]>>(),
     )));
     let out: Value<i32> = Rc::new(RefCell::new(
-        (({
-            let _a: Ptr<i32> = ((*p1.borrow()).offset((1) as isize));
-            foo_0(_a)
-        })
-        .offset((4) as isize)
-        .read()),
+        (({ foo_0(((*p1.borrow()).offset((1) as isize))) })
+            .offset((4) as isize)
+            .read()),
     ));
     (*p1.borrow()).delete_array();
     return 0;

tests/ub/out/refcount/ub19.rs

@@ -15,9 +15,6 @@ pub fn main() {
 }
 fn main_0() -> i32 {
     let x: Value<Ptr<i32>> = Rc::new(RefCell::new(Ptr::alloc(1)));
-    ({
-        let _array: Ptr<i32> = (*x.borrow()).clone();
-        foo_0(_array)
-    });
+    ({ foo_0((*x.borrow()).clone()) });
     return 0;
 }

tests/ub/out/refcount/ub20.rs

@@ -19,9 +19,6 @@ fn main_0() -> i32 {
             .map(|_| <i32>::default())
             .collect::<Box<[i32]>>(),
     )));
-    ({
-        let _single: Ptr<i32> = (*x.borrow()).clone();
-        foo_0(_single)
-    });
+    ({ foo_0((*x.borrow()).clone()) });
     return 0;
 }

tests/ub/out/refcount/ub21.rs

@@ -23,8 +23,5 @@ fn main_0() -> i32 {
         ('t' as u8),
         ('r' as u8),
     ])));
-    return (({
-        let _s: Ptr<u8> = (s.as_pointer() as Ptr<u8>);
-        strlen_0(_s)
-    }) as i32);
+    return (({ strlen_0((s.as_pointer() as Ptr<u8>)) }) as i32);
 }

tests/ub/out/refcount/ub4.rs

@@ -24,11 +24,7 @@ fn main_0() -> i32 {
     let x1: Value<i32> = Rc::new(RefCell::new(1));
     if ((*x1.borrow()) != 0) {
         let x2: Value<i32> = Rc::new(RefCell::new(-1_i32));
-        (*out.borrow_mut()) = ({
-            let _x1: Ptr<i32> = x1.as_pointer();
-            let _x2: Ptr<i32> = x2.as_pointer();
-            smaller_0(_x1, _x2)
-        });
+        (*out.borrow_mut()) = ({ smaller_0(x1.as_pointer(), x2.as_pointer()) });
     }
     return ((*out.borrow()).read());
 }

tests/ub/out/refcount/ub5.rs

@@ -16,10 +16,7 @@ pub fn main() {
 fn main_0() -> i32 {
     let x: Value<i32> = Rc::new(RefCell::new(1));
     let p: Value<Ptr<i32>> = Rc::new(RefCell::new((x.as_pointer())));
-    ({
-        let _p: Ptr<Ptr<i32>> = (p.as_pointer());
-        null_0(_p)
-    });
+    ({ null_0((p.as_pointer())) });
     let r: Ptr<i32> = (*p.borrow()).clone();
     return (r.read());
 }

tests/ub/out/refcount/ub6.rs

@@ -69,14 +69,6 @@ fn main_0() -> i32 {
                 .map(|_| <Ptr<i32>>::default())
                 .collect::<Box<[_]>>(),
         )))));
-    ({
-        let _arr: Ptr<Option<Value<Box<[Ptr<i32>]>>>> = arr.as_pointer();
-        let _n1: Ptr<i32> = n.as_pointer();
-        fill_1(_arr, _n1)
-    });
-    return (({
-        let _arr: Ptr<Option<Value<Box<[Ptr<i32>]>>>> = arr.as_pointer();
-        let _n1: Ptr<i32> = n.as_pointer();
-        any_2(_arr, _n1)
-    }) as i32);
+    ({ fill_1(arr.as_pointer(), n.as_pointer()) });
+    return (({ any_2(arr.as_pointer(), n.as_pointer()) }) as i32);
 }

tests/ub/out/refcount/ub7.rs

@@ -26,8 +26,5 @@ fn main_0() -> i32 {
         ('n' as u8),
         ('g' as u8),
     ])));
-    return (({
-        let _s: Ptr<u8> = ((s.as_pointer() as Ptr<u8>).offset(0 as isize));
-        strlen_0(_s)
-    }) as i32);
+    return (({ strlen_0(((s.as_pointer() as Ptr<u8>).offset(0 as isize))) }) as i32);
 }

tests/ub/out/unsafe/dangling-prvalue-as-lvalue.rs

@@ -16,10 +16,7 @@ pub fn main() {
 }
 unsafe fn main_0() -> i32 {
     let mut v: Vec<i32> = vec![1, 2];
-    let b: *const i32 = (unsafe {
-        let _a: *const i32 = &(*v.as_mut_ptr()) as *const i32;
-        foo_0(_a)
-    });
+    let b: *const i32 = (unsafe { foo_0(&(*v.as_mut_ptr()) as *const i32) });
     v.clear();
     return (*b);
 }

tests/ub/out/unsafe/ub12.rs

@@ -16,10 +16,7 @@ pub fn main() {
 }
 unsafe fn main_0() -> i32 {
     let mut alloc: *mut i32 = (Box::leak(Box::new(1)) as *mut i32);
-    (unsafe {
-        let _ptr: *mut i32 = alloc;
-        escape_0(_ptr)
-    });
+    (unsafe { escape_0(alloc) });
     ::std::mem::drop(Box::from_raw(alloc));
     return 0;
 }

tests/ub/out/unsafe/ub13.rs

@@ -16,9 +16,6 @@ pub fn main() {
 }
 unsafe fn main_0() -> i32 {
     let mut p1: *mut i32 = (Box::leak(Box::new(1)) as *mut i32);
-    (unsafe {
-        let _p: *mut i32 = p1;
-        escape_0(_p)
-    });
+    (unsafe { escape_0(p1) });
     return (*p1);
 }