Evaluate try_emplace arguments before checking the key

cpp2rust/converter/converter.cpp

@@ -2697,6 +2697,10 @@ bool Converter::VisitCXXRewrittenBinaryOperator(
 }
 
 bool Converter::VisitBinaryOperator(clang::BinaryOperator *expr) {
+  if (expr->isAssignmentOp() && isAddrOf()) {
+    ConvertAssignmentRef(expr);
+    return false;
+  }
   if (expr->getOpcode() == clang::BO_Cmp) {
     EmitFresh(std::format("std::cmp::Ord::cmp(&({}), &({}))",
                           ConvertRValue(expr->getLHS()),
@@ -2717,6 +2721,52 @@ bool Converter::VisitBinaryOperator(clang::BinaryOperator *expr) {
   return false;
 }
 
+void Converter::ConvertAssignmentRef(clang::BinaryOperator *expr) {
+  auto *lhs = expr->getLHS();
+  auto *rhs = expr->getRHS();
+  auto value = ConvertFreshRValue(rhs);
+  auto ptr = ConvertFreshObject(lhs);
+  auto loc = expr->getExprLoc();
+  clang::DeclContext *context = curr_function_;
+  if (!context) {
+    context =
+        clang::BlockDecl::Create(ctx_, ctx_.getTranslationUnitDecl(), loc);
+  }
+  auto bind = [&](clang::QualType type) {
+    return clang::VarDecl::Create(
+        ctx_, context, loc, loc, &ctx_.Idents.get(NewTempName()), type,
+        ctx_.getTrivialTypeSourceInfo(type, loc), clang::SC_None);
+  };
+  auto *left = bind(ctx_.getLValueReferenceType(lhs->getType()));
+  auto *right = bind(rhs->getType());
+  bound_values_.insert(right);
+  auto ref = [&](clang::VarDecl *var) {
+    return clang::DeclRefExpr::Create(ctx_, {}, {}, var, false, loc,
+                                      var->getType().getNonReferenceType(),
+                                      clang::VK_LValue);
+  };
+  expr->setLHS(ref(left));
+  expr->setRHS(clang::ImplicitCastExpr::Create(
+      ctx_, rhs->getType(), clang::CK_LValueToRValue, ref(right), nullptr,
+      clang::VK_PRValue, clang::FPOptionsOverride()));
+  std::string assignment;
+  {
+    Buffer buf(*this);
+    PushExprKind push(*this, ExprKind::Void);
+    ConvertBinaryOperator(expr);
+    assignment = std::move(buf).str();
+  }
+  expr->setLHS(lhs);
+  expr->setRHS(rhs);
+  // C++ evaluates the right operand before finding the place to write. Reuse
+  // the ordinary assignment conversion on the bound operands, then return the
+  // same place, without evaluating either operand again.
+  StrCat(std::format(
+      "match ({{ {0} }}, {1}) {{ ({2}, {3}) => {{ {4}; {3} }} }}", value, ptr,
+      GetNamedDeclAsString(right), GetNamedDeclAsString(left), assignment));
+  MarkFresh();
+}
+
 void Converter::ConvertBinaryOperator(clang::BinaryOperator *expr) {
   auto type = expr->getType();
   auto *lhs = expr->getLHS();
@@ -2895,6 +2945,14 @@ bool Converter::ConvertIncAndDec(clang::UnaryOperator *expr) {
   if (!method) {
     return false;
   }
+  if (expr->isPrefix() && isAddrOf()) {
+    auto ptr = ConvertFreshObject(expr->getSubExpr());
+    auto name = NewTempName();
+    StrCat(std::format("{{ let {0} = {1}; (*{0}).{2}(); {0} }}", name, ptr,
+                       method));
+    MarkFresh();
+    return true;
+  }
   {
     PushExprKind push(*this, ExprKind::LValue);
     Convert(expr->getSubExpr());
@@ -5137,8 +5195,13 @@ std::string Converter::GetMappedAsString(clang::Expr *expr, clang::Expr **args,
   auto all_args = BuildUnifiedArgs(expr, args, num_args);
   auto bind = tgt_ir->argsToBind(ctx_, expr, all_args);
   std::vector<std::string> values, names;
-  // The variables are local to the function.
-  for (size_t i = 0; curr_function_ && i < all_args.size(); ++i) {
+  // Bindings in a global initializer are local to the expansion too.
+  clang::DeclContext *binding_context = curr_function_;
+  if (!binding_context && tgt_ir->usesInit()) {
+    binding_context = clang::BlockDecl::Create(
+        ctx_, ctx_.getTranslationUnitDecl(), expr->getExprLoc());
+  }
+  for (size_t i = 0; binding_context && i < all_args.size(); ++i) {
     if (!bind[i]) {
       continue;
     }
@@ -5158,7 +5221,7 @@ std::string Converter::GetMappedAsString(clang::Expr *expr, clang::Expr **args,
     auto var_type = by_value ? type : ctx_.getLValueReferenceType(type);
     auto loc = arg->getExprLoc();
     auto *var = clang::VarDecl::Create(
-        ctx_, curr_function_, loc, loc, &ctx_.Idents.get(NewTempName()),
+        ctx_, binding_context, loc, loc, &ctx_.Idents.get(NewTempName()),
         var_type, ctx_.getTrivialTypeSourceInfo(var_type, loc), clang::SC_None);
     // The temporaries of a value, e.g., borrows, end with its block.
     if (by_value) {
@@ -5170,8 +5233,15 @@ std::string Converter::GetMappedAsString(clang::Expr *expr, clang::Expr **args,
           arg, ToString(ctx_.getPointerType(type)), convert_to));
     }
     names.push_back((by_value ? "mut " : "") + GetNamedDeclAsString(var));
+    // Preserve the value category used to forward an init argument to its
+    // constructor, in particular an rvalue that must be moved, not copied.
     all_args[i] = clang::DeclRefExpr::Create(ctx_, {}, {}, var, false, loc,
                                              type, clang::VK_LValue);
+    if (i >= tgt_ir->params.size() && arg->isXValue()) {
+      all_args[i] = clang::ImplicitCastExpr::Create(
+          ctx_, type, clang::CK_NoOp, all_args[i], nullptr, clang::VK_XValue,
+          clang::FPOptionsOverride());
+    }
     if (by_value && !type->isRecordType()) {
       all_args[i] = clang::ImplicitCastExpr::Create(
           ctx_, type, clang::CK_LValueToRValue, all_args[i], nullptr,

cpp2rust/converter/converter.h

@@ -506,6 +506,7 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
   virtual bool VisitBinaryOperator(clang::BinaryOperator *expr);
   bool VisitCXXRewrittenBinaryOperator(clang::CXXRewrittenBinaryOperator *expr);
 
+  void ConvertAssignmentRef(clang::BinaryOperator *expr);
   virtual void ConvertBinaryOperator(clang::BinaryOperator *expr);
 
   // The value that the compound assignment expr assigns to its left operand,

cpp2rust/converter/models/boxed_vars.cpp

@@ -237,11 +237,17 @@ class AddressTakenVisitor
     if (!rule) {
       return;
     }
-    // Variadic arguments are not considered.
     auto all_args = BuildUnifiedArgs(expr, args, num_args);
     auto bound = rule->argsToBind(ctx_, expr, all_args);
-    for (unsigned i = 0; i < all_args.size() && i < rule->params.size(); ++i) {
+    for (unsigned i = 0; i < all_args.size(); ++i) {
       if (bound[i]) {
+        if (all_args[i]->isGLValue()) {
+          AddAddressUse(all_args[i]);
+        }
+        continue;
+      }
+      // Unbound variadic arguments are not considered.
+      if (i >= rule->params.size()) {
         continue;
       }
       auto *arg = IgnoreNoOpCasts(all_args[i]);
@@ -279,6 +285,29 @@ class AddressTakenVisitor
     }
   }
 
+  // A bound lvalue must stay addressable even when its evaluation also writes
+  // it, as in ++counter or counter = value.
+  void AddAddressUse(const clang::Expr *expr) {
+    expr = expr->IgnoreParenImpCasts();
+    if (auto *ref = clang::dyn_cast<clang::DeclRefExpr>(expr)) {
+      AddressTaken(ref->getDecl());
+    } else if (auto *object = GetEnclosingObject(expr)) {
+      AddAddressUse(object);
+    } else if (auto *unary = clang::dyn_cast<clang::UnaryOperator>(expr);
+               unary && unary->isIncrementDecrementOp()) {
+      AddAddressUse(unary->getSubExpr());
+    } else if (auto *binary = clang::dyn_cast<clang::BinaryOperator>(expr)) {
+      if (binary->isAssignmentOp()) {
+        AddAddressUse(binary->getLHS());
+      } else if (binary->getOpcode() == clang::BO_Comma) {
+        AddAddressUse(binary->getRHS());
+      }
+    } else if (auto *cond = clang::dyn_cast<clang::ConditionalOperator>(expr)) {
+      AddAddressUse(cond->getTrueExpr());
+      AddAddressUse(cond->getFalseExpr());
+    }
+  }
+
   void AddressTaken(const clang::ValueDecl *decl) {
     if (!IsUserDefinedDecl(decl)) {
       return;

cpp2rust/converter/models/converter_refcount.cpp

@@ -909,6 +909,16 @@ bool ConverterRefCount::ConvertIncAndDec(clang::UnaryOperator *expr) {
     return false;
   }
 
+  if (expr->isPrefix() && isAddrOf()) {
+    auto ptr = ConvertFreshObject(sub_expr);
+    auto name = NewTempName();
+    StrCat(
+        std::format("{{ let {0} = {1}; {0}.with_mut(|__v| __v.{2}()); {0} }}",
+                    name, ptr, method));
+    MarkFresh();
+    return true;
+  }
+
   auto str = ConvertLValue(sub_expr);
   if (!pending_deref_.empty()) {
     StrCat(pending_deref_.take(), ".with_mut(|__v| __v.", method, "())");

cpp2rust/converter/translation_rule.cpp

@@ -126,7 +126,8 @@ std::vector<BodyFragment> ParseBodyFragmentsJSON(const llvm::json::Array &arr) {
     } else if (frag_obj->get("va_args")) {
       result.push_back(VaArgsFragment{});
     } else if (frag_obj->get("init")) {
-      result.push_back(InitFragment{});
+      result.push_back(
+          InitFragment{frag_obj->getBoolean("conditional").value_or(false)});
     }
   }
   return result;
@@ -271,6 +272,18 @@ ExprRule::argsToBind(clang::ASTContext &ctx, const clang::Expr *expr,
         ++uses[ph->n];
       }
       ++position;
+    } else if (auto *init = std::get_if<InitFragment>(&frag)) {
+      // init constructs from the trailing arguments at this position, but
+      // their evaluation belongs to the call even when construction is skipped.
+      for (size_t i = params.size(); i < args.size(); ++i) {
+        if (uses[i]++ == 0) {
+          first[i] = position;
+        }
+        if (init->conditional) {
+          ++uses[i];
+        }
+        ++position;
+      }
     }
     return false;
   });
@@ -280,7 +293,9 @@ ExprRule::argsToBind(clang::ASTContext &ctx, const clang::Expr *expr,
   std::vector<bool> bind(args.size());
   bool bind_rest = false;
   unsigned next_first = position;
-  auto num_args = std::min(args.size(), params.size());
+  bool init_args = usesInit();
+  auto num_args =
+      init_args ? args.size() : std::min(args.size(), params.size());
   for (auto k = num_args; k-- > 0;) {
     auto i = right_to_left ? num_args - 1 - k : k;
     auto *arg = args[i];
@@ -291,8 +306,10 @@ 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.
-      bind_rest |= uses[i] > 1 || takesTemporaryAddress(ctx, expr, i, arg) ||
-                   (uses[i] == 1 && first[i] > next_first);
+      bind_rest |=
+          uses[i] > 1 ||
+          (i < params.size() && takesTemporaryAddress(ctx, expr, i, arg)) ||
+          (uses[i] == 1 && first[i] > next_first);
       bind[i] = bind_rest;
       if (uses[i] == 1) {
         next_first = std::min(next_first, first[i]);
@@ -309,6 +326,10 @@ ExprRule::argsToBind(clang::ASTContext &ctx, const clang::Expr *expr,
 
 bool ExprRule::takesAddress(clang::ASTContext &ctx, unsigned i,
                             const clang::Expr *arg) const {
+  // An init pack forwards references to its arguments, not their values.
+  if (i >= params.size()) {
+    return usesInit();
+  }
   return params[i].is_pointer() && !arg->getType()->isPointerType() &&
          !RuleRegistry::MapsToPointer(ctx, arg->getType());
 }

cpp2rust/converter/translation_rule.h

@@ -49,6 +49,8 @@ struct VaArgsFragment {
 };
 
 struct InitFragment {
+  bool conditional = false;
+
   void dump() const;
 };
 
@@ -122,7 +124,8 @@ struct ExprRule {
   // evaluated once and in order: those with side effects that the body may
   // evaluate more than once, or after a later one, and those with side effects
   // evaluated before them. Values computed from others, which the body may
-  // evaluate more than once, are bound too. args are those of expr.
+  // evaluate more than once, are bound too. Arguments a conditional init would
+  // skip are bound without constructing init. args are those of expr.
   std::vector<bool> argsToBind(clang::ASTContext &ctx, const clang::Expr *expr,
                                llvm::ArrayRef<clang::Expr *> args) const;
 

docs/src/rules/ir.md

@@ -83,7 +83,10 @@ The fragment kinds are:
   result it emits the lvalue alone, so `p.first = 1.5` is `p.0 = 1.5` rather
   than `(*&mut p.0) = 1.5`.
 - `va_args`: the expansion point for a variadic tail.
-- `init`: the value built from the call's trailing arguments.
+- `init`: the value built from the call's trailing arguments. Its optional
+  `conditional` flag marks construction in a branch, loop, or closure. The
+  converter binds side-effectful arguments before entering such a rule body,
+  without making the construction unconditional.
 
 Every type in the Rules IR (in `params`, `return_type`, and type rules) is a
 `TypeInfo` object, the type text plus a set of flags:

rule-preprocessor/src/ir.rs

@@ -182,6 +182,8 @@ pub enum BodyFragment {
     },
     Init {
         init: std::marker::PhantomData<()>,
+        #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+        conditional: bool,
     },
     GenericCall {
         generic_call: GenericCallInner,

rule-preprocessor/src/syntactic.rs

@@ -248,6 +248,7 @@ impl<'a> FragmentCtx<'a> {
                     self.flush_text();
                     self.fragments.push(BodyFragment::Init {
                         init: std::marker::PhantomData,
+                        conditional: Self::is_conditional(token),
                     });
                     return;
                 }