Remove bottom-up IsRedundantCopyInConversion with top-down PushSuppressIteratorClone

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -2757,6 +2757,8 @@ void Converter::ConvertCXXConstructExprArgs(clang::CXXConstructExpr *expr) {
 }
 
 bool Converter::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
+  PushSuppressIteratorClone push(*this, expr);
+
   if (auto str = GetMappedAsString(expr, expr->getArgs(), expr->getNumArgs());
       !str.empty()) {
     StrCat(str);
@@ -2767,8 +2769,10 @@ bool Converter::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   if (ctor->isCopyOrMoveConstructor() ||
       (ctor->isConvertingConstructor(false) && ctor->getNumParams() == 1 &&
        ctor->getParamDecl(0)->getType()->isRValueReferenceType())) {
+    // Take supress before recursing into the child.
+    bool suppress = PushSuppressIteratorClone::take(*this, expr);
     Convert(expr->getArg(0));
-    if (ctor->isCopyConstructor() && !IsRedundantCopyInConversion(ctx_, expr)) {
+    if (ctor->isCopyConstructor() && !suppress) {
       StrCat(".clone()");
     }
     return false;

cpp2rust/converter/converter.h

@@ -529,6 +529,7 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
   bool in_function_formals_ = false;
   bool in_const_initializer_ = false;
   std::optional<bool> autoref_mut_;
+  bool suppress_iterator_clone_ = false;
 
   struct PushExplicitAutoref {
     Converter &c;
@@ -540,6 +541,43 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
     ~PushExplicitAutoref() { c.autoref_mut_ = prev; }
   };
 
+  struct PushSuppressIteratorClone {
+    Converter &c;
+    bool prev;
+    PushSuppressIteratorClone(Converter &c, clang::CXXConstructExpr *expr)
+        : c(c), prev(c.suppress_iterator_clone_) {
+      auto *ctor = expr->getConstructor();
+      if (ctor->isConvertingConstructor(/*AllowExplicit=*/false) &&
+          ctor->getNumParams() == 1 && IsIteratorType(expr->getType())) {
+        c.suppress_iterator_clone_ = true;
+      }
+    }
+    ~PushSuppressIteratorClone() { c.suppress_iterator_clone_ = prev; }
+    PushSuppressIteratorClone(const PushSuppressIteratorClone &) = delete;
+    PushSuppressIteratorClone &
+    operator=(const PushSuppressIteratorClone &) = delete;
+
+    static bool take(Converter &c, clang::CXXConstructExpr *inner) {
+      bool suppress =
+          c.suppress_iterator_clone_ && IsIteratorType(inner->getType());
+      c.suppress_iterator_clone_ = false;
+      return suppress;
+    }
+
+  private:
+    static bool IsIteratorType(clang::QualType qt) {
+      if (auto *record = qt->getAsCXXRecordDecl()) {
+        for (auto *d : record->decls()) {
+          if (auto *tnd = llvm::dyn_cast<clang::TypedefNameDecl>(d)) {
+            if (tnd->getName() == "iterator_category")
+              return true;
+          }
+        }
+      }
+      return false;
+    }
+  };
+
   struct PushConstInitializer {
     Converter &c;
     bool prev;

cpp2rust/converter/converter_lib.cpp

@@ -644,34 +644,6 @@ std::string GetClassName(clang::QualType type) {
   return {};
 }
 
-static bool IsIteratorType(clang::QualType qt) {
-  if (auto *record = qt->getAsCXXRecordDecl()) {
-    for (auto *d : record->decls()) {
-      if (auto *tnd = llvm::dyn_cast<clang::TypedefNameDecl>(d)) {
-        if (tnd->getName() == "iterator_category")
-          return true;
-      }
-    }
-  }
-  return false;
-}
-
-bool IsRedundantCopyInConversion(clang::ASTContext &ctx,
-                                 const clang::CXXConstructExpr *expr) {
-  return false;
-  if (auto parents = ctx.getParentMapContext().getParents(*expr);
-      !parents.empty()) {
-    if (auto *parent = parents[0].get<clang::CXXConstructExpr>()) {
-      if (parent->getConstructor()->isConvertingConstructor(
-              /* AllowExplicit= */ false)) {
-        return IsIteratorType(expr->getType()) &&
-               IsIteratorType(parent->getType());
-      }
-    }
-  }
-  return false;
-}
-
 bool IsVaListType(clang::QualType type) {
   for (auto t = type; !t.isNull();) {
     if (auto *adjusted = t->getAs<clang::AdjustedType>()) {

cpp2rust/converter/converter_lib.h

@@ -145,9 +145,6 @@ GetForRangeIteratorType(const clang::CXXForRangeStmt *for_range);
 
 std::string GetClassName(clang::QualType type);
 
-bool IsRedundantCopyInConversion(clang::ASTContext &ctx,
-                                 const clang::CXXConstructExpr *expr);
-
 bool IsVaListType(clang::QualType type);
 
 bool IsBuiltinVaStart(const clang::CallExpr *expr);

cpp2rust/converter/models/converter_refcount.cpp

@@ -1541,6 +1541,7 @@ std::string ConverterRefCount::ConvertStream(clang::Expr *expr) {
 
 bool ConverterRefCount::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   PushConversionKind push(*this, ConversionKind::Unboxed);
+  PushSuppressIteratorClone push_suppress(*this, expr);
 
   if (auto str = GetMappedAsString(expr, expr->getArgs(), expr->getNumArgs());
       !str.empty()) {
@@ -1564,7 +1565,7 @@ bool ConverterRefCount::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   }
 
   if (ctor->isCopyConstructor()) {
-    StrCat(IsRedundantCopyInConversion(ctx_, expr)
+    StrCat(PushSuppressIteratorClone::take(*this, expr)
                ? ConvertRValue(expr->getArg(0))
                : ConvertFreshRValue(expr->getArg(0)));
     return false;