Remove more hand-written temporaries from rules

Author: nunoplopes

cpp2rust/converter/converter.cpp

@@ -5018,7 +5018,9 @@ std::string Converter::ConvertPlaceholder(clang::Expr *expr, clang::Expr *arg,
         [this, arg](clang::QualType type) {
           return MaterializeTemp(NewTempName(), type, arg);
         });
+    // The temporary is initialized in a statement, which ends its borrows.
     if (!materialized.empty()) {
+      borrows_ = false;
       return materialized;
     }
   }
@@ -5246,7 +5248,17 @@ std::string Converter::ConvertIRFragment(
               RuleRegistry::ParamIsPointer(ctx_, expr, arg_idx),
           .is_index_base = ph->is_index_base,
       };
-      result += ConvertPlaceholder(expr, arg, ph_ctx);
+      // A value is read in a block, which ends its borrows before the rule
+      // accesses what they borrow, unless it may point to a temporary, which
+      // would end too.
+      auto outer_borrows = std::exchange(borrows_, false);
+      auto value = ConvertPlaceholder(expr, arg, ph_ctx);
+      if (borrows_ && isFresh() && !CreatesTemporary(arg)) {
+        value = std::format("({{ {} }})", value);
+        borrows_ = false;
+      }
+      borrows_ |= outer_borrows;
+      result += value;
     } else if (std::get_if<TranslationRule::VaArgsFragment>(&frag)) {
       assert(expr && "variadic arguments outside an expression rule");
       result += ConvertVariadicTail(expr, all_args);

cpp2rust/converter/converter.h

@@ -874,6 +874,10 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
   // closure parameter of its with_mut.
   std::vector<std::pair<std::string, std::string>> mut_borrows_;
 
+  // Whether the expression being converted borrows a value in a temporary,
+  // e.g., a guard of a RefCell, which lives until the end of the statement.
+  bool borrows_ = false;
+
   // The variables that arguments of rules are bound to by value, whose
   // address is not taken.
   std::unordered_set<const clang::VarDecl *> bound_values_;

cpp2rust/converter/converter_lib.cpp

@@ -2010,6 +2010,12 @@ bool ReadsMemory(const clang::Stmt *stmt) {
   });
 }
 
+bool CreatesTemporary(const clang::Stmt *stmt) {
+  return AnyStmt(stmt, [](const clang::Stmt *s) {
+    return clang::isa<clang::MaterializeTemporaryExpr>(s);
+  });
+}
+
 bool ReferencesThis(const clang::Stmt *stmt) {
   return AnyStmt(stmt, [](const clang::Stmt *s) {
     return clang::isa<clang::CXXThisExpr>(s);

cpp2rust/converter/converter_lib.h

@@ -325,6 +325,9 @@ bool ReferencesThis(const clang::Stmt *stmt);
 // Whether evaluating `stmt` reads a value from memory, e.g., a pointer.
 bool ReadsMemory(const clang::Stmt *stmt);
 
+// Whether stmt materializes a temporary, which a value may point to.
+bool CreatesTemporary(const clang::Stmt *stmt);
+
 bool MayCauseBorrowMutError(const clang::Expr *lhs, const clang::Expr *rhs);
 
 bool ArgsMayAlias(const clang::Expr *a, const clang::Expr *b);

cpp2rust/converter/models/boxed_vars.cpp

@@ -228,9 +228,10 @@ class AddressTakenVisitor
 
 private:
   // The arguments of a call translated by a rule are passed by value or
-  // borrowed, unless the rule takes a pointer to them, or they are bound to
-  // references. A pointer parameter receives the value of an argument that is
-  // itself translated to a pointer.
+  // borrowed, unless the rule takes a pointer to them, other than to a
+  // temporary copy of them, or they are bound to references. A pointer
+  // parameter receives the value of an argument that is itself translated to a
+  // pointer.
   void AddRuleArgs(clang::Expr *expr, clang::Expr **args, unsigned num_args) {
     auto *rule = RuleRegistry::GetExprRule(ctx_, expr);
     if (!rule) {
@@ -248,7 +249,8 @@ class AddressTakenVisitor
         TraverseStmt(init);
         continue;
       }
-      if (!rule->takesAddress(ctx_, i, arg)) {
+      if (!rule->takesAddress(ctx_, i, arg) ||
+          rule->takesTemporaryAddress(ctx_, expr, i, arg)) {
         AddValueUse(arg);
       }
     }

cpp2rust/converter/models/converter_refcount.cpp

@@ -1018,17 +1018,20 @@ void ConverterRefCount::ConvertDeclRefValue(clang::Expr *expr,
     if (is_global_value && TypeIsCopyable(decl_t)) {
       StrCat(str, ".with(|rc| *rc.borrow())");
     } else if (is_global_value) {
+      borrows_ = true;
       // Borrow the global in place, like a local. Cloning it would be
       // expensive for arrays, and writes through its fields would be lost.
       StrCat(std::format("(*{}.with(Value::clone).borrow())", std::move(str)));
     } else {
       StrCat(std::format("(*{}.borrow())", std::move(str)));
+      borrows_ = true;
     }
   } else {
     if (is_global_value) {
       str += ".with(Value::clone)";
     }
     StrCat(std::format("(*{}.borrow_mut())", std::move(str)));
+    borrows_ = true;
   }
 
   // A copied global is read into a new value.
@@ -1827,6 +1830,7 @@ bool ConverterRefCount::VisitMemberExpr(clang::MemberExpr *expr) {
   } else {
     StrCat(std::format("(*{}.borrow_mut())", std::move(str)));
   }
+  borrows_ = true;
   SetValueFreshness(expr->getType());
   return false;
 }
@@ -2895,7 +2899,11 @@ bool ConverterRefCount::DerefReadsValue(clang::QualType pointee_type) {
 
 const char *
 ConverterRefCount::GetPointerDerefSuffix(clang::QualType pointee_type) {
-  return DerefReadsValue(pointee_type) ? ".read()" : ".upgrade().deref()";
+  if (DerefReadsValue(pointee_type)) {
+    return ".read()";
+  }
+  borrows_ = true;
+  return ".upgrade().deref()";
 }
 
 void ConverterRefCount::EmitPointee(std::string ptr,

cpp2rust/converter/translation_rule.cpp

@@ -277,8 +277,6 @@ ExprRule::argsToBind(clang::ASTContext &ctx, const clang::Expr *expr,
   // The right operand of an assignment is evaluated before the left one.
   auto *op = clang::dyn_cast<clang::CXXOperatorCallExpr>(expr);
   bool right_to_left = op && op->isAssignmentOp();
-  auto *call = clang::dyn_cast<clang::CallExpr>(expr);
-  unsigned first_arg = clang::isa<clang::CXXMemberCallExpr>(expr);
   std::vector<bool> bind(args.size());
   bool bind_rest = false;
   unsigned next_first = position;
@@ -293,10 +291,7 @@ ExprRule::argsToBind(clang::ASTContext &ctx, const clang::Expr *expr,
     if (arg->HasSideEffects(ctx)) {
       // A temporary that a reference parameter is bound to would be
       // materialized before the call, out of order.
-      bool materialized =
-          call && i >= first_arg && takesAddress(ctx, i, arg) &&
-          GetRefBindingTempType(ctx, call, i - first_arg).has_value();
-      bind_rest |= uses[i] > 1 || materialized ||
+      bind_rest |= uses[i] > 1 || takesTemporaryAddress(ctx, expr, i, arg) ||
                    (uses[i] == 1 && first[i] > next_first);
       bind[i] = bind_rest;
       if (uses[i] == 1) {
@@ -318,6 +313,15 @@ bool ExprRule::takesAddress(clang::ASTContext &ctx, unsigned i,
          !RuleRegistry::MapsToPointer(ctx, arg->getType());
 }
 
+bool ExprRule::takesTemporaryAddress(clang::ASTContext &ctx,
+                                     const clang::Expr *expr, unsigned i,
+                                     const clang::Expr *arg) const {
+  auto *call = clang::dyn_cast<clang::CallExpr>(expr);
+  unsigned first_arg = clang::isa<clang::CXXMemberCallExpr>(expr);
+  return call && i >= first_arg && takesAddress(ctx, i, arg) &&
+         GetRefBindingTempType(ctx, call, i - first_arg).has_value();
+}
+
 void RefExprFragment::dump() const {
   log() << "  ref_expr: " << prefix << '\n';
   for (auto &frag : inner) {

cpp2rust/converter/translation_rule.h

@@ -130,6 +130,11 @@ struct ExprRule {
   // pointer.
   bool takesAddress(clang::ASTContext &ctx, unsigned i,
                     const clang::Expr *arg) const;
+
+  // Whether the body takes a pointer to a temporary copy of arg, argument i
+  // of expr, that a reference parameter is bound to.
+  bool takesTemporaryAddress(clang::ASTContext &ctx, const clang::Expr *expr,
+                             unsigned i, const clang::Expr *arg) const;
 };
 
 struct TypeRule {

rules/map/tgt_refcount.rs

@@ -24,9 +24,7 @@ fn f3<T1: CppLess + 'static, T2: 'static>(
     a0: Ptr<Map<T1, (Value<T1>, Value<T2>)>>,
     a1: BidirectionalIterator<(Value<T1>, Value<T2>)>,
 ) -> BidirectionalIterator<(Value<T1>, Value<T2>)> {
-    // The block ends the borrow of the iterator, e.g., in it = m.erase(it).
-    let __it = a1;
-    Map::erase(a0, &__it)
+    Map::erase(a0, &a1)
 }
 
 fn f5<T1, T2>() -> Map<T1, (Value<T1>, Value<T2>)> {
@@ -89,9 +87,7 @@ fn f27<T1: CppLess + 'static, T2: 'static>(
     a0: Ptr<Map<T1, (Value<T1>, Value<T2>)>>,
     a1: BidirectionalIterator<(Value<T1>, Value<T2>)>,
 ) -> BidirectionalIterator<(Value<T1>, Value<T2>)> {
-    // The block ends the borrow of the iterator, e.g., in it = m.erase(it).
-    let __it = a1;
-    Map::erase(a0, &__it)
+    Map::erase(a0, &a1)
 }
 
 fn f28<T1: CppLess + 'static, T2: 'static>(a0: Ptr<Map<T1, (Value<T1>, Value<T2>)>>, a1: Ptr<T1>) -> usize {

rules/pwd/tgt_refcount.rs

@@ -8,11 +8,6 @@ fn t1() -> libcc2rs::Passwd {
 }
 
 fn f2(a0: u32, a1: Ptr<Passwd>, a2: Ptr<i8>, a3: usize, a4: Ptr<Ptr<Passwd>>) -> i32 {
-    // Computing the arguments first ends their borrows of what the call writes.
-    let __pwbuf = a1;
-    let __buf = a2;
-    let __buflen = a3;
-    let __out = a4;
     match nix::unistd::User::from_uid(nix::unistd::Uid::from_raw(a0)) {
         Ok(Some(__u)) => {
             let __strs: [Vec<u8>; 5] = [
@@ -23,30 +18,30 @@ fn f2(a0: u32, a1: Ptr<Passwd>, a2: Ptr<i8>, a3: usize, a4: Ptr<Ptr<Passwd>>) ->
                 __u.shell.as_os_str().as_encoded_bytes().to_vec(),
             ];
             let __needed: usize = __strs.iter().map(|__s| __s.len() + 1).sum();
-            if __needed > __buflen {
-                __out.write(Ptr::null());
+            if __needed > a3 {
+                a4.write(Ptr::null());
                 ::libc::ERANGE
             } else {
                 let mut __ptrs: Vec<Ptr<i8>> = Vec::new();
                 let mut __off: usize = 0;
                 for __s in &__strs {
-                    __ptrs.push(__buf.offset(__off));
+                    __ptrs.push(a2.offset(__off));
                     let __end = __s.len();
-                    __buf.offset(__off).write_c_bytes(__s);
-                    elem!(__buf, __off + __end).write(0);
+                    a2.offset(__off).write_c_bytes(__s);
+                    elem!(a2, __off + __end).write(0);
                     __off += __end + 1;
                 }
-                __pwbuf.with_mut(|__pw| *__pw = Passwd::from_user_in(&__u, &__ptrs));
-                __out.write(__pwbuf.clone());
+                a1.with_mut(|__pw| *__pw = Passwd::from_user_in(&__u, &__ptrs));
+                a4.write(a1.clone());
                 0
             }
         }
         Ok(None) => {
-            __out.write(Ptr::null());
+            a4.write(Ptr::null());
             0
         }
         Err(__e) => {
-            __out.write(Ptr::null());
+            a4.write(Ptr::null());
             __e as i32
         }
     }