Suppress clone in iterator conversion

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);
     Convert(expr->getArg(0));
-    if (ctor->isCopyConstructor() && !IsRedundantCopyInConversion(ctx_, expr)) {
+    if (ctor->isCopyConstructor() && !suppress) {
       StrCat(".clone()");
     }
     return false;

cpp2rust/converter/converter.h

@@ -13,6 +13,7 @@
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 #include "converter/lex.h"
@@ -529,6 +530,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 +542,41 @@ 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->isCopyOrMoveConstructor() &&
+          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) {
+      return std::exchange(c.suppress_iterator_clone_, false);
+    }
+
+  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,16 +644,6 @@ std::string GetClassName(clang::QualType type) {
   return {};
 }
 
-bool IsRedundantCopyInConversion(clang::ASTContext &ctx,
-                                 const clang::CXXConstructExpr *expr) {
-  auto parents = ctx.getParentMapContext().getParents(*expr);
-  if (parents.empty()) {
-    return false;
-  }
-  auto *parent = parents[0].get<clang::CXXConstructExpr>();
-  return parent && parent->getConstructor()->isConvertingConstructor(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,9 @@ bool ConverterRefCount::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   }
 
   if (ctor->isCopyConstructor()) {
-    StrCat(ConvertRValue(expr->getArg(0)));
+    StrCat(PushSuppressIteratorClone::take(*this)
+               ? ConvertRValue(expr->getArg(0))
+               : ConvertFreshRValue(expr->getArg(0)));
     return false;
   }
 

rules/map/ir_refcount.json

@@ -462,14 +462,27 @@
   "f19": {
     "body": [
       {
-        "placeholder": {
-          "arg": 0,
-          "access": "read"
+        "method_call": {
+          "receiver": [
+            {
+              "placeholder": {
+                "arg": 0,
+                "access": "read"
+              }
+            }
+          ],
+          "body": [
+            {
+              "text": ".clone()"
+            }
+          ]
         }
       }
     ],
     "generics": {
-      "T1": [],
+      "T1": [
+        "Clone"
+      ],
       "T2": []
     },
     "params": {

rules/map/ir_unsafe.json

@@ -466,14 +466,27 @@
   "f19": {
     "body": [
       {
-        "placeholder": {
-          "arg": 0,
-          "access": "read"
+        "method_call": {
+          "receiver": [
+            {
+              "placeholder": {
+                "arg": 0,
+                "access": "read"
+              }
+            }
+          ],
+          "body": [
+            {
+              "text": ".clone()"
+            }
+          ]
         }
       }
     ],
     "generics": {
-      "T1": [],
+      "T1": [
+        "Clone"
+      ],
       "T2": []
     },
     "params": {

rules/map/tgt_refcount.rs

@@ -120,8 +120,8 @@ fn f17<T1: Ord + Clone + 'static, T2: 'static>(
     RefcountMapIter::find_key(a0, &a1)
 }
 
-fn f19<T1, T2>(a0: RefcountMapIter<T1, T2>) -> RefcountMapIter<T1, T2> {
-    a0
+fn f19<T1: Clone, T2>(a0: RefcountMapIter<T1, T2>) -> RefcountMapIter<T1, T2> {
+    a0.clone()
 }
 
 fn f20<T1: Ord + Clone + 'static, T2: 'static>(a0: RefcountMapIter<T1, T2>) -> Value<T1> {

rules/map/tgt_unsafe.rs

@@ -83,8 +83,8 @@ unsafe fn f17<T1: Ord + Clone, T2>(a0: BTreeMap<T1, Box<T2>>, a1: T1) -> UnsafeM
     UnsafeMapIterator::find_key(&a0 as *const BTreeMap<T1, Box<T2>>, &a1)
 }
 
-unsafe fn f19<T1, T2>(a0: UnsafeMapIterator<T1, T2>) -> UnsafeMapIterator<T1, T2> {
-    a0
+unsafe fn f19<T1: Clone, T2>(a0: UnsafeMapIterator<T1, T2>) -> UnsafeMapIterator<T1, T2> {
+    a0.clone()
 }
 
 unsafe fn f20<T1: Ord + Clone, T2>(a0: UnsafeMapIterator<T1, T2>) -> *const T1 {

tests/unit/initializer_list.cpp

@@ -0,0 +1,16 @@
+#include <cassert>
+#include <cstddef>
+#include <initializer_list>
+#include <vector>
+
+size_t f(std::initializer_list<int> bytes) {
+  std::vector<int> *buf = new std::vector<int>(bytes);
+  size_t n = bytes.size();
+  delete buf;
+  return n;
+}
+
+int main() {
+  assert(f({1, 2, 3}) == 3);
+  return 0;
+}

tests/unit/out/refcount/initializer_list.rs

@@ -0,0 +1,27 @@
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::cell::RefCell;
+use std::collections::BTreeMap;
+use std::io::prelude::*;
+use std::io::{Read, Seek, Write};
+use std::os::fd::AsFd;
+use std::rc::{Rc, Weak};
+pub fn f_0(bytes: Vec<i32>) -> u64 {
+    let bytes: Value<Vec<i32>> = Rc::new(RefCell::new(bytes));
+    let buf: Value<Ptr<Vec<i32>>> = Rc::new(RefCell::new(Ptr::alloc((*bytes.borrow()).clone())));
+    let n: Value<u64> = Rc::new(RefCell::new((*bytes.borrow()).len() as u64));
+    (*buf.borrow()).delete();
+    return (*n.borrow());
+}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    assert!(
+        (({
+            let _bytes: Vec<i32> = vec![1, 2, 3];
+            f_0(_bytes)
+        }) == 3_u64)
+    );
+    return 0;
+}

tests/unit/out/refcount/map.rs

@@ -192,19 +192,22 @@ fn main_0() -> i32 {
         &1_i16,
     )));
     let const_it: Value<RefcountMapIter<i16, u32>> = Rc::new(RefCell::new(
-        RefcountMapIter::find_key((m.as_pointer() as Ptr<BTreeMap<i16, Value<u32>>>), &10_i16),
+        RefcountMapIter::find_key((m.as_pointer() as Ptr<BTreeMap<i16, Value<u32>>>), &10_i16)
+            .clone(),
     ));
     let x1: Value<u32> = Rc::new(RefCell::new(if (*it.borrow()) == (*end.borrow()) {
         0_u32
     } else {
         (*(*it.borrow()).second().borrow())
     }));
     assert!(((*x1.borrow()) == 4_u32));
-    let x2: Value<u32> = Rc::new(RefCell::new(if (*const_it.borrow()) == (*end.borrow()) {
-        0_u32
-    } else {
-        (*(*const_it.borrow()).second().borrow())
-    }));
+    let x2: Value<u32> = Rc::new(RefCell::new(
+        if (*const_it.borrow()) == (*end.borrow()).clone() {
+            0_u32
+        } else {
+            (*(*const_it.borrow()).second().borrow())
+        },
+    ));
     assert!(((*x2.borrow()) == 0_u32));
     let x3: Value<u32> = Rc::new(RefCell::new(
         if (*it.borrow())
@@ -218,7 +221,7 @@ fn main_0() -> i32 {
     assert!(((*x3.borrow()) == 4_u32));
     let x4: Value<u32> = Rc::new(RefCell::new(
         if (*const_it.borrow())
-            == RefcountMapIter::end((m.as_pointer() as Ptr<BTreeMap<i16, Value<u32>>>))
+            == RefcountMapIter::end((m.as_pointer() as Ptr<BTreeMap<i16, Value<u32>>>)).clone()
         {
             0_u32
         } else {
@@ -274,7 +277,7 @@ fn main_0() -> i32 {
     );
     RefcountMapIter::erase(
         ((r).clone() as Ptr<BTreeMap<i16, Value<u32>>>),
-        &(*it4.borrow()),
+        &(*it4.borrow()).clone(),
     );
     assert!(((*r.upgrade().deref()).len() as u64 == 3_u64));
     assert!(

tests/unit/out/refcount/redundant_copy_in_conversion.rs

@@ -0,0 +1,55 @@
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::cell::RefCell;
+use std::collections::BTreeMap;
+use std::io::prelude::*;
+use std::io::{Read, Seek, Write};
+use std::os::fd::AsFd;
+use std::rc::{Rc, Weak};
+pub fn sink_0(it: RefcountMapIter<i32, i32>) -> i32 {
+    let it: Value<RefcountMapIter<i32, i32>> = Rc::new(RefCell::new(it));
+    let cit: Value<RefcountMapIter<i32, i32>> = Rc::new(RefCell::new((*it.borrow()).clone()));
+    return if (*cit.borrow()) == (*it.borrow()).clone() {
+        (*(*it.borrow()).second().borrow())
+    } else {
+        0
+    };
+}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    let m: Value<BTreeMap<i32, Value<i32>>> = Rc::new(RefCell::new(BTreeMap::new()));
+    (m.as_pointer() as Ptr<BTreeMap<i32, Value<i32>>>)
+        .with_mut(|__v: &mut BTreeMap<i32, Value<i32>>| {
+            __v.entry(0.clone())
+                .or_insert_with(|| Rc::new(RefCell::new(<i32>::default())))
+                .as_pointer()
+        })
+        .write(1);
+    let end: Value<RefcountMapIter<i32, i32>> = Rc::new(RefCell::new(RefcountMapIter::end(
+        (m.as_pointer() as Ptr<BTreeMap<i32, Value<i32>>>),
+    )));
+    let it0: Value<RefcountMapIter<i32, i32>> = Rc::new(RefCell::new(RefcountMapIter::find_key(
+        (m.as_pointer() as Ptr<BTreeMap<i32, Value<i32>>>),
+        &0,
+    )));
+    let const_it: Value<RefcountMapIter<i32, i32>> = Rc::new(RefCell::new((*it0.borrow()).clone()));
+    let r: Value<i32> = Rc::new(RefCell::new(
+        if (*const_it.borrow()) == (*end.borrow()).clone() {
+            0
+        } else {
+            1
+        },
+    ));
+    (*r.borrow_mut()) += ({
+        let _it: RefcountMapIter<i32, i32> = (*it0.borrow()).clone();
+        sink_0(_it)
+    });
+    (*r.borrow_mut()) += if (*end.borrow()) == (*end.borrow()) {
+        0
+    } else {
+        1
+    };
+    return (*r.borrow());
+}

tests/unit/out/unsafe/initializer_list.rs

@@ -0,0 +1,28 @@
+extern crate libc;
+use libc::*;
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::collections::BTreeMap;
+use std::io::{Read, Seek, Write};
+use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
+use std::rc::Rc;
+pub unsafe fn f_0(mut bytes: Vec<i32>) -> u64 {
+    let mut buf: *mut Vec<i32> = (Box::leak(Box::new(bytes.clone())) as *mut Vec<i32>);
+    let mut n: u64 = bytes.len() as u64;
+    ::std::mem::drop(Box::from_raw(buf));
+    return n;
+}
+pub fn main() {
+    unsafe {
+        std::process::exit(main_0() as i32);
+    }
+}
+unsafe fn main_0() -> i32 {
+    assert!(
+        ((unsafe {
+            let _bytes: Vec<i32> = vec![1, 2, 3];
+            f_0(_bytes)
+        }) == (3_u64))
+    );
+    return 0;
+}