Avoid hoisting for function call arguments that don't alias (#203)

In terms of converter, the PR is small. I moved GetAllVars and
MayCauseBorrowMutError from converter to converter_lib because they are
converter-agnostic. Then I added the aliasing logic in CollectCallInfo 2
functions in converter_lib: ReferencesThis and ArgsMayAlias.

The core idea is: if an argument of a function call does not alias with
any other argument, then it can be inlined instead of hoisted. This
includes aliases over this, for example `this.method(this.member)`

Author: lucic71

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());
 }