Remove unneeded clones in converter and rules (#371)

This PR removes unneeded clones **in converter** by calling
`SetValueFreshness`/`computed_expr_type_`/`SetFreshType` on every
fressh-value-producing path. It asserts that `computed_expr_type_` is
not Unknown after a conversion to make sure that we did not miss any
path.

PendingDeref carries freshness separately. Producers of PendingDeref set
freshness and consumers act based on that. I also added
`ComputedExprType::Pending` because freshness is unknown until the
consumer of PendingDeref sets it.

This PR also removes unneeded clones **in rules** using the `Access` IR
attribute. Rules no longer clone their arguments. The preprocessor
infers how each argument is used: `borrow, borrow_mut, move, take`. A
`move` (`let s = a0`) consumes the argument, and it's cloned if it's not
already fresh. A `borrow` usage (`a0.len()`) is never cloned.

Besides removing unneeded clones, on the converter side, this fixes
#363. On the rules side,
`rule-preprocessor` infers the access type of each argument
automatically instead on relying on the rule author to encode it
properly. This caught a bug in `std::vector::push_back(T&&)`: the rule
argument took the argument by value and cloned it, instead of moving it.

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -681,6 +681,13 @@ bool Converter::RecordDerivesDefault(const clang::RecordDecl *decl) {
   return true;
 }
 
+bool Converter::IsPassThroughRule(clang::Expr *expr) const {
+  const auto *rule = Mapper::GetExprRule(GetCalleeOrExpr(expr));
+  return rule && rule->body.size() == 1 &&
+         std::holds_alternative<TranslationRule::PlaceholderFragment>(
+             rule->body[0]);
+}
+
 bool Converter::RecordDerivesCopy(const clang::RecordDecl *decl) const {
   auto *derives = Mapper::MappedDerives(ctx_.getCanonicalTagType(decl));
   return derives &&
@@ -1491,7 +1498,12 @@ bool Converter::Convert(clang::Expr *expr,
       NeedsImplicitScalarCast(expr->IgnoreImplicit()->getType(),
                               *implicit_convert_to);
   PushParen paren(*this, needs_conversion);
+  computed_expr_type_ = ComputedExprType::Unknown;
   bool result = TraverseStmt(expr);
+  if (expr && computed_expr_type_ == ComputedExprType::Unknown) {
+    expr->dump();
+    assert(false && "computed_expr_type_ not set");
+  }
   if (needs_conversion) {
     ConvertCast(*implicit_convert_to);
     computed_expr_type_ = ComputedExprType::FreshValue;
@@ -1721,17 +1733,20 @@ void Converter::ConvertVAArgCall(clang::CallExpr *expr) {
 bool Converter::VisitCallExpr(clang::CallExpr *expr) {
   if (IsBuiltinVaStart(expr) || IsBuiltinVaEnd(expr) || IsBuiltinVaCopy(expr)) {
     ConvertVAArgCall(expr);
+    SetFreshType(expr->getType());
     return false;
   }
 
   // p->~T() on a scalar is a no-op
   if (clang::isa<clang::CXXPseudoDestructorExpr>(
           expr->getCallee()->IgnoreParenImpCasts())) {
+    SetFreshType(expr->getType());
     return false;
   }
 
   if (auto plugin_str = TryPluginConvert(expr)) {
     StrCat(*plugin_str);
+    SetFreshType(expr->getType());
     return false;
   }
 
@@ -1750,9 +1765,10 @@ bool Converter::VisitCallExpr(clang::CallExpr *expr) {
       str = GetMappedAsString(expr, args, num_args, &ctx);
     };
 
-    if ((IsReferenceType(expr) ||
-         GetReturnTypeOfFunction(expr)->isReferenceType()) &&
-        !isAddrOf() && !isVoid()) {
+    bool deref_ref = (IsReferenceType(expr) ||
+                      GetReturnTypeOfFunction(expr)->isReferenceType()) &&
+                     !isAddrOf() && !isVoid();
+    if (deref_ref) {
       str = "( * " + std::move(str) + " )";
     }
 
@@ -1761,6 +1777,11 @@ bool Converter::VisitCallExpr(clang::CallExpr *expr) {
     }
 
     StrCat(str);
+    if (deref_ref) {
+      SetValueFreshness(expr->getType());
+    } else if (!IsPassThroughRule(expr)) {
+      SetFreshType(expr->getType());
+    }
     return false;
   }
 
@@ -1795,6 +1816,7 @@ bool Converter::VisitCallExpr(clang::CallExpr *expr) {
   }
 
   StrCat(str);
+  SetFreshType(expr->getType());
   return false;
 }
 
@@ -1809,6 +1831,7 @@ void Converter::EmitFnPtrCall(clang::Expr *callee) {
 void Converter::ConvertFunctionToFunctionPointer(
     const clang::FunctionDecl *fn_decl) {
   StrCat(std::format("Some({})", Mapper::MapFunctionName(fn_decl)));
+  computed_expr_type_ = ComputedExprType::FreshPointer;
 }
 
 Converter::CallInfo Converter::CollectCallInfo(clang::CallExpr *expr) {
@@ -2256,6 +2279,7 @@ void Converter::ConvertIntegerToEnumeralCast(clang::Expr *to,
       auto dst_enum = to->getType()->getAs<clang::EnumType>();
       if (src_enum && dst_enum && dst_enum->getDecl() == src_enum) {
         StrCat(EnumeratorName(ec));
+        computed_expr_type_ = ComputedExprType::FreshValue;
         return;
       }
     }
@@ -2266,6 +2290,7 @@ void Converter::ConvertIntegerToEnumeralCast(clang::Expr *to,
     Convert(from);
   }
   StrCat(keyword::kAs, GetUnsafeTypeAsString(to->getType()));
+  computed_expr_type_ = ComputedExprType::FreshValue;
 }
 
 void Converter::ConvertIntegralToBooleanCast(clang::ImplicitCastExpr *expr) {
@@ -2285,6 +2310,7 @@ void Converter::ConvertIntegralToBooleanCast(clang::ImplicitCastExpr *expr) {
   Convert(sub_expr);
   StrCat(token::kDiff);
   StrCat(token::kZero);
+  computed_expr_type_ = ComputedExprType::FreshValue;
 }
 
 bool Converter::IsCastRedundantInRust(clang::Expr *expr,
@@ -2323,6 +2349,7 @@ bool Converter::VisitImplicitCastExpr(clang::ImplicitCastExpr *expr) {
     } else {
       StrCat(dest_pointee_const ? ".as_ptr()" : ".as_mut_ptr()");
     }
+    computed_expr_type_ = ComputedExprType::FreshPointer;
     break;
   }
   case clang::CastKind::CK_BitCast: {
@@ -2334,6 +2361,7 @@ bool Converter::VisitImplicitCastExpr(clang::ImplicitCastExpr *expr) {
       StrCat(ConvertPointeeType(sub_expr->getType()));
     }
     ConvertCast(type);
+    SetFreshType(type);
     break;
   }
   case clang::CastKind::CK_NoOp: {
@@ -2358,13 +2386,15 @@ bool Converter::VisitImplicitCastExpr(clang::ImplicitCastExpr *expr) {
       PushParen paren(*this);
       Convert(sub_expr);
       ConvertCast(type);
+      SetFreshType(type);
     } else {
       {
         PushParen paren(*this, suffix);
         Convert(sub_expr);
       }
       if (suffix) {
         StrCat(suffix);
+        SetFreshType(type);
       }
     }
     break;
@@ -2424,6 +2454,7 @@ bool Converter::VisitImplicitCastExpr(clang::ImplicitCastExpr *expr) {
         ConvertCast(type);
       }
     }
+    SetFreshType(type);
   }
   return false;
 }
@@ -2714,6 +2745,7 @@ bool Converter::ConvertIncAndDec(clang::UnaryOperator *expr) {
 bool Converter::VisitUnaryOperator(clang::UnaryOperator *expr) {
   if (auto str = GetMappedAsString(expr); !str.empty()) {
     StrCat(str);
+    SetFreshType(expr->getType());
     return false;
   }
 
@@ -2770,6 +2802,7 @@ bool Converter::VisitUnaryOperator(clang::UnaryOperator *expr) {
   default:
     StrCat(expr->getOpcodeStr(opcode));
     Convert(sub_expr);
+    SetFreshType(expr->getType());
   }
   return false;
 }
@@ -2882,6 +2915,9 @@ bool Converter::VisitDeclRefExpr(clang::DeclRefExpr *expr) {
       ConvertFunctionToFunctionPointer(fn_decl);
       return false;
     }
+    StrCat(str);
+    SetFreshType(expr->getType());
+    return false;
   }
 
   if (auto var_decl = clang::dyn_cast<clang::VarDecl>(decl)) {
@@ -2891,6 +2927,7 @@ bool Converter::VisitDeclRefExpr(clang::DeclRefExpr *expr) {
                 init->IgnoreUnlessSpelledInSource())) {
           PushParen paren(*this);
           VisitLambdaExpr(lambda);
+          computed_expr_type_ = ComputedExprType::FreshValue;
           return false;
         }
       }
@@ -2900,10 +2937,16 @@ bool Converter::VisitDeclRefExpr(clang::DeclRefExpr *expr) {
   if (!decl->getType()->getAs<clang::ReferenceType>() && isAddrOf()) {
     StrCat(token::kRef, decl->getType().isConstQualified() ? "" : keyword_mut_,
            str);
+    computed_expr_type_ = ComputedExprType::FreshPointer;
     return false;
   }
 
   StrCat(str);
+  if (clang::isa<clang::EnumConstantDecl>(decl)) {
+    computed_expr_type_ = ComputedExprType::FreshValue;
+    return false;
+  }
+  SetValueFreshness(expr->getType());
   return false;
 }
 
@@ -2996,6 +3039,7 @@ bool Converter::VisitMemberExpr(clang::MemberExpr *expr) {
     SetUFCSReceiver(expr->getBase(), expr->isArrow(), method);
     StrCat(GetRecordName(method->getParent()), token::kDoubleColon,
            GetMethodName(method));
+    SetFreshType(expr->getType());
     return false;
   }
   std::string str;
@@ -3029,10 +3073,16 @@ bool Converter::VisitMemberExpr(clang::MemberExpr *expr) {
   if (!isAddrOf() && member->getType()->isFunctionPointerType()) {
     PushParen paren(*this);
     StrCat(str);
+    SetValueFreshness(expr->getType());
     return false;
   }
 
   StrCat(str);
+  if (clang::isa<clang::CXXMethodDecl>(member)) {
+    SetFreshType(expr->getType());
+  } else {
+    SetValueFreshness(expr->getType());
+  }
   return false;
 }
 
@@ -3144,6 +3194,7 @@ bool Converter::VisitCXXThisExpr(clang::CXXThisExpr *expr) {
     PushParen paren(*this);
     StrCat(keyword::kSelfValue, keyword::kAs, ToString(expr->getType()));
   }
+  computed_expr_type_ = ComputedExprType::FreshPointer;
   return false;
 }
 
@@ -3169,6 +3220,7 @@ bool Converter::VisitInitListExpr(clang::InitListExpr *expr) {
       } else {
         StrCat(GetArrayDefaultAsString(qual_type));
       }
+      SetFreshType(qual_type);
       return false;
     }
 
@@ -3204,6 +3256,7 @@ bool Converter::VisitInitListExpr(clang::InitListExpr *expr) {
       }
     }
   }
+  SetFreshType(qual_type);
   return false;
 }
 
@@ -3407,6 +3460,9 @@ bool Converter::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   if (auto str = GetMappedAsString(expr, expr->getArgs(), expr->getNumArgs());
       !str.empty()) {
     StrCat(str);
+    if (!IsPassThroughRule(expr)) {
+      SetFreshType(expr->getType());
+    }
     return false;
   }
 
@@ -3428,13 +3484,15 @@ bool Converter::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
     if ((ctor->isCopyConstructor() || IsDefaultedMoveConstructor(ctor)) &&
         !suppress && !TypeIsCopyable(expr->getType())) {
       StrCat(".clone()");
+      SetFreshType(expr->getType());
     }
     return false;
   }
 
   if (ctor->isDefaultConstructor() && !ctor->isUserProvided()) {
     auto ty = expr->getType();
     StrCat(GetDefaultAsString(ty));
+    SetFreshType(expr->getType());
     return false;
   }
 
@@ -3444,6 +3502,7 @@ bool Converter::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   } else {
     ConvertCXXConstructExprArgs(expr);
   }
+  SetFreshType(expr->getType());
   return false;
 }
 
@@ -3541,6 +3600,12 @@ bool Converter::VisitCXXDefaultArgExpr(clang::CXXDefaultArgExpr *expr) {
   return false;
 }
 
+bool Converter::VisitConstantExpr(clang::ConstantExpr *expr) {
+  Convert(expr->getSubExpr());
+  SetFreshType(expr->getType());
+  return false;
+}
+
 bool Converter::VisitLambdaExpr(clang::LambdaExpr *expr) {
   if (isAddrOf() && expr->capture_size() == 0) {
     StrCat("Some");
@@ -4477,13 +4542,15 @@ void Converter::ConvertAddrOf(clang::Expr *expr, clang::QualType pointer_type) {
                        : keyword_mut_);
     Convert(expr);
     ConvertCast(pointer_type);
+    computed_expr_type_ = ComputedExprType::FreshPointer;
   } else {
     StrCat(token::kRef);
     if (!pointer_type->getPointeeType().isConstQualified()) {
       StrCat(keyword_mut_);
     }
     Convert(expr);
     ConvertCast(pointer_type);
+    computed_expr_type_ = ComputedExprType::FreshPointer;
   }
 }
 
@@ -4495,6 +4562,7 @@ void Converter::EmitDeref(std::string inner, clang::QualType pointee_type) {
   }
   PushParen paren(*this);
   StrCat(GetPointerDerefPrefix(pointee_type), std::move(inner));
+  SetValueFreshness(pointee_type);
 }
 
 void Converter::ConvertDeref(clang::Expr *expr) {
@@ -4555,8 +4623,8 @@ void Converter::PlaceholderCtx::dump() const {
                << ", is_cpp_ptr: " << is_cpp_ptr
                << ", maps_to_rust_ptr: " << maps_to_rust_ptr
                << ", declared_in_rule_as_rust_ptr: "
-               << declared_in_rule_as_rust_ptr << ", access: "
-               << (access == TranslationRule::Access::kRead ? "read" : "write")
+               << declared_in_rule_as_rust_ptr
+               << ", access: " << static_cast<int>(access)
                << ", param_type: " << param_type
                << ", materialize_idx: " << materialize_idx << '\n';
 }
@@ -4594,10 +4662,10 @@ std::string Converter::ConvertPlaceholder(clang::Expr *expr, clang::Expr *arg,
   if (ph_ctx.needs_object_receiver()) {
     Buffer buf(*this);
     PushExplicitAutoref autoref(
-        *this,
-        ph_ctx.is_index_base
-            ? std::optional(ph_ctx.access == TranslationRule::Access::kWrite)
-            : std::nullopt);
+        *this, ph_ctx.is_index_base
+                   ? std::optional(ph_ctx.access ==
+                                   TranslationRule::Access::kBorrowMut)
+                   : std::nullopt);
     PushExprKind push(*this, ExprKind::RValue);
     ConvertDeref(arg);
     return std::move(buf).str();
@@ -4611,13 +4679,17 @@ std::string Converter::ConvertPlaceholder(clang::Expr *expr, clang::Expr *arg,
     return ConvertLValue(arg);
   }
 
-  if (ph_ctx.access == TranslationRule::Access::kMove) {
+  if (ph_ctx.access == TranslationRule::Access::kTake) {
     if (clang::isa<clang::MaterializeTemporaryExpr>(arg)) {
       return ConvertRValue(arg);
     }
     return std::format("std::mem::take(&mut {})", ConvertLValue(arg));
   }
 
+  if (ph_ctx.access == TranslationRule::Access::kMove) {
+    return ConvertFreshRValue(arg, ph_ctx.implicit_convert_to);
+  }
+
   return ConvertRValue(arg, ph_ctx.implicit_convert_to);
 }
 
@@ -4774,6 +4846,12 @@ void Converter::SetFresh() {
   case ComputedExprType::FreshValue:
   case ComputedExprType::FreshPointer:
     break;
+  case ComputedExprType::Unknown:
+    assert(0 && "Unreachable ComputedExprType::Unknown");
+    break;
+  case ComputedExprType::Pending:
+    assert(0 && "Unreachable ComputedExprType::Pending");
+    break;
   }
 }