Remove unneeded Value<T> boxing from constructors

Author: nunoplopes

cpp2rust/converter/models/converter_refcount.cpp

@@ -581,16 +581,26 @@ bool ConverterRefCount::RecordImplementsClone(const clang::RecordDecl *decl) {
          HasCallableCopyConstructor(decl);
 }
 
+// Bases are translated to traits, so they hold no state of the struct; they
+// only need to be copied without side effects.
+static bool HasDefaultedBaseCopies(const clang::CXXRecordDecl *decl) {
+  return std::ranges::all_of(decl->bases(), [](auto &base) {
+    auto *record = base.getType()->getAsCXXRecordDecl();
+    return record && HasDefaultedCopyConstructor(record) &&
+           HasDefaultedBaseCopies(record);
+  });
+}
+
 bool ConverterRefCount::RecordDerivesClone(const clang::RecordDecl *decl) {
   if (HasDefaultedCopyConstructor(decl) && RecordHasOnlyReferenceFields(decl)) {
     return true;
   }
   // The copy of a struct copies each field, which a derived Clone does too,
   // except for Values, which it shares.
   auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
-  if (decl->isUnion() ||
-      (cxx && (!HasCallableCopyConstructor(cxx) ||
-               GetUserDefinedCopyConstructor(cxx) || cxx->getNumBases() > 0))) {
+  if (decl->isUnion() || (cxx && (!HasCallableCopyConstructor(cxx) ||
+                                  GetUserDefinedCopyConstructor(cxx) ||
+                                  !HasDefaultedBaseCopies(cxx)))) {
     return false;
   }
   PushConversionKind push(*this, ConversionKind::Pointee);
@@ -611,7 +621,7 @@ bool ConverterRefCount::RecordDerivesDeepClone(const clang::RecordDecl *decl) {
   // own clone(), and Values are copied deeply.
   auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
   return !cxx ||
-         (!GetUserDefinedCopyConstructor(cxx) && cxx->getNumBases() == 0);
+         (!GetUserDefinedCopyConstructor(cxx) && HasDefaultedBaseCopies(cxx));
 }
 
 void ConverterRefCount::AddCloneTrait(const clang::RecordDecl *decl) {
@@ -3146,6 +3156,7 @@ void ConverterRefCount::SetUFCSReceiver(clang::Expr *base, bool is_arrow,
     bool in_ctor =
         curr_function_ && clang::isa<clang::CXXConstructorDecl>(curr_function_);
     if (in_ctor) {
+      ctor_uses_this_ = true;
       ufcs_receiver_ = "&this";
     } else if (ThisIsRustPtr()) {
       ufcs_receiver_ = keyword::kSelfValue;
@@ -3307,21 +3318,43 @@ void ConverterRefCount::ConvertCXXConstructorBody(
     clang::CXXConstructorDecl *decl) {
   EmitFunctionPreamble(decl);
   auto record_name = GetRecordName(decl->getParent());
+  auto saved_uses_this = std::exchange(ctor_uses_this_, false);
+  std::string init;
+  {
+    Buffer buf(*this);
+    if (decl->isDelegatingConstructor()) {
+      Convert((*decl->init_begin())->getInit());
+    } else {
+      StrCat("Self");
+      PushBrace this_init(*this);
+      EmitConstructorFieldInits(decl);
+    }
+    init = std::move(buf).str();
+  }
+  std::string body;
+  {
+    Buffer buf(*this);
+    ConvertBodyStmts(decl->getBody());
+    body = std::move(buf).str();
+  }
+  bool uses_this = std::exchange(ctor_uses_this_, saved_uses_this);
+
+  if (!uses_this) {
+    if (Trim(body).empty()) {
+      StrCat(init);
+    } else {
+      StrCat(keyword::kLet, "__this", token::kColon, record_name,
+             token::kAssign, init, token::kSemiColon, body, "__this");
+    }
+    return;
+  }
   StrCat(keyword::kLet, "__this", token::kColon,
          std::format("Value<{}>", record_name), token::kAssign,
-         "Rc::new(RefCell::new(");
-  if (decl->isDelegatingConstructor()) {
-    Convert((*decl->init_begin())->getInit());
-  } else {
-    StrCat("Self");
-    PushBrace this_init(*this);
-    EmitConstructorFieldInits(decl);
-  }
-  StrCat("))", token::kSemiColon);
+         std::format("Rc::new(RefCell::new({}))", init), token::kSemiColon);
   StrCat(keyword::kLet, "this", token::kColon,
          std::format("Ptr<{}>", record_name), token::kAssign,
          "__this.as_pointer()", token::kSemiColon);
-  ConvertBodyStmts(decl->getBody());
+  StrCat(body);
   StrCat("Rc::try_unwrap(__this).ok().unwrap().into_inner()");
 }
 
@@ -3338,6 +3371,7 @@ bool ConverterRefCount::VisitCXXThisExpr(
   bool in_ctor =
       curr_function_ && clang::isa<clang::CXXConstructorDecl>(curr_function_);
   if (in_ctor) {
+    ctor_uses_this_ = true;
     StrCat("this");
   } else {
     StrCat("(*", keyword::kSelfValue, ')');

cpp2rust/converter/models/converter_refcount.h

@@ -412,6 +412,10 @@ class ConverterRefCount final : public Converter {
   bool IsUnboxedVar(const clang::ValueDecl *decl) const;
   std::shared_ptr<const BoxedVars> boxed_vars_;
 
+  // Set when a constructor's translation refers to `this`, which then needs
+  // the object being built to be in a Value.
+  bool ctor_uses_this_ = false;
+
   /// The kind of conversion that should be performed.
   enum class ConversionKind : uint8_t {
     Unboxed,

tests/multi-file/defaulted_move_cross_tu/out/refcount/defaulted_move_cross_tu.rs

@@ -19,27 +19,23 @@ pub struct S {
 impl S {
     pub fn new(x: i32) -> Self {
         let x: Value<i32> = Rc::new(RefCell::new(x));
-        let __this: Value<S> = Rc::new(RefCell::new(Self {
+        Self {
             v: Rc::new(RefCell::new(vec![
                 (*x.borrow());
                 ((*x.borrow()) as usize) as usize
             ])),
             n: Rc::new(RefCell::new(Box::new([(*x.borrow()), ((*x.borrow()) + 1)]))),
-        }));
-        let this: Ptr<S> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        }
     }
     pub fn move_from(_a0: Ptr<S>) -> Self {
-        let __this: Value<S> = Rc::new(RefCell::new(Self {
+        Self {
             v: Rc::new(RefCell::new(std::mem::take(
                 &mut (*{ (*_a0.upgrade().deref()).v.clone() }.borrow_mut()),
             ))),
             n: Rc::new(RefCell::new(Box::new(std::array::from_fn::<_, 2, _>(
                 |__i: usize| (*{ (*_a0.upgrade().deref()).n.clone() }.borrow())[(__i) as usize],
             )))),
-        }));
-        let this: Ptr<S> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        }
     }
 }
 impl Default for S {

tests/multi-file/out_of_line_methods_cross_tu/out/refcount/out_of_line_methods_cross_tu.rs

@@ -16,22 +16,13 @@ impl S {}
 pub trait Base {
     fn apply(&mut self, x: i32) -> i32;
 }
-#[derive(Record, ByteRepr, VaArg, FnPtrArg, Default)]
+#[derive(Clone, Record, ByteRepr, VaArg, FnPtrArg, Default)]
 #[byte_size(16)]
 pub struct Derived {
     #[offset(8)]
     pub factor: i32,
 }
 impl Derived {}
-impl Clone for Derived {
-    fn clone(&self) -> Self {
-        let __this: Value<Derived> = Rc::new(RefCell::new(Self {
-            factor: { self.factor },
-        }));
-        let this: Ptr<Derived> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
-    }
-}
 pub fn main() {
     __cpp2rust_init_globals();
     std::process::exit(main_0());
@@ -49,16 +40,12 @@ fn main_0() -> i32 {
 }
 impl S {
     pub fn new(mut x: i32) -> Self {
-        let __this: Value<S> = Rc::new(RefCell::new(Self { v: x }));
-        let this: Ptr<S> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { v: x }
     }
 }
 impl Derived {
     pub fn new(mut factor: i32) -> Self {
-        let __this: Value<Derived> = Rc::new(RefCell::new(Self { factor: factor }));
-        let this: Ptr<Derived> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { factor: factor }
     }
 }
 impl S {}

tests/ub/out/refcount/ctor_ref_member.rs

@@ -15,9 +15,7 @@ pub struct S {
 }
 impl S {
     pub fn new(x: Ptr<i32>) -> Self {
-        let __this: Value<S> = Rc::new(RefCell::new(Self { r: (x).clone() }));
-        let this: Ptr<S> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { r: (x).clone() }
     }
 }
 pub fn main() {

tests/unit/out/refcount/10_struct.rs

@@ -33,9 +33,7 @@ pub struct Partial {
 }
 impl Partial {
     pub fn new_1(mut q: Ptr<i32>) -> Self {
-        let __this: Value<Partial> = Rc::new(RefCell::new(Self { p: (q).clone() }));
-        let this: Ptr<Partial> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { p: (q).clone() }
     }
 }
 impl Default for Partial {

tests/unit/out/refcount/constructor.rs

@@ -34,9 +34,7 @@ pub struct Point {
 }
 impl Point {
     pub fn new_1(mut x: i32, mut y: i32) -> Self {
-        let __this: Value<Point> = Rc::new(RefCell::new(Self { x: x, y: y }));
-        let this: Ptr<Point> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { x: x, y: y }
     }
     pub fn new_2(mut v: i32) -> Self {
         let __this: Value<Point> = Rc::new(RefCell::new(Point::new_1({ v }, { (v + 1) })));

tests/unit/out/refcount/copy_assign.rs

@@ -19,17 +19,13 @@ pub struct Partial {
 }
 impl Partial {
     pub fn new(mut v: i32, mut keep: i32) -> Self {
-        let __this: Value<Partial> = Rc::new(RefCell::new(Self { v: v, keep: keep }));
-        let this: Ptr<Partial> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { v: v, keep: keep }
     }
     pub fn copy_from(o: Ptr<Partial>) -> Self {
-        let __this: Value<Partial> = Rc::new(RefCell::new(Self {
+        Self {
             v: o.with(|__s| __s.v),
             keep: o.with(|__s| __s.keep),
-        }));
-        let this: Ptr<Partial> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        }
     }
 }
 impl Clone for Partial {
@@ -49,9 +45,7 @@ pub struct NonConstAssign {
 }
 impl NonConstAssign {
     pub fn new() -> Self {
-        let __this: Value<NonConstAssign> = Rc::new(RefCell::new(Self { mark: 0 }));
-        let this: Ptr<NonConstAssign> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { mark: 0 }
     }
 }
 impl Default for NonConstAssign {
@@ -67,9 +61,7 @@ pub struct RefQualified {
 }
 impl RefQualified {
     pub fn new() -> Self {
-        let __this: Value<RefQualified> = Rc::new(RefCell::new(Self { mark: 0 }));
-        let this: Ptr<RefQualified> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { mark: 0 }
     }
 }
 impl Default for RefQualified {

tests/unit/out/refcount/copy_ctor.rs

@@ -17,17 +17,14 @@ pub struct Counted {
 }
 impl Counted {
     pub fn new(mut v: i32) -> Self {
-        let __this: Value<Counted> = Rc::new(RefCell::new(Self { v: v }));
-        let this: Ptr<Counted> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { v: v }
     }
     pub fn copy_from(o: Ptr<Counted>) -> Self {
-        let __this: Value<Counted> = Rc::new(RefCell::new(Self {
+        let __this: Counted = Self {
             v: o.with(|__s| __s.v),
-        }));
-        let this: Ptr<Counted> = __this.as_pointer();
+        };
         (*copies_0.with(Value::clone).borrow_mut()).prefix_inc();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        __this
     }
 }
 impl Clone for Counted {
@@ -44,23 +41,17 @@ pub struct NonConst {
 }
 impl NonConst {
     pub fn new() -> Self {
-        let __this: Value<NonConst> = Rc::new(RefCell::new(Self { mark: 0 }));
-        let this: Ptr<NonConst> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { mark: 0 }
     }
     pub fn new_1(o: Ptr<NonConst>) -> Self {
-        let __this: Value<NonConst> = Rc::new(RefCell::new(Self {
+        Self {
             mark: (o.with(|__s| __s.mark) + 1),
-        }));
-        let this: Ptr<NonConst> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        }
     }
     pub fn new_2(o: Ptr<NonConst>) -> Self {
-        let __this: Value<NonConst> = Rc::new(RefCell::new(Self {
+        Self {
             mark: (o.with(|__s| __s.mark) + 10),
-        }));
-        let this: Ptr<NonConst> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        }
     }
 }
 impl Clone for NonConst {
@@ -84,15 +75,12 @@ pub struct Ignored {
 }
 impl Ignored {
     pub fn new(mut v: i32) -> Self {
-        let __this: Value<Ignored> = Rc::new(RefCell::new(Self { v: v }));
-        let this: Ptr<Ignored> = __this.as_pointer();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        Self { v: v }
     }
     pub fn copy_from(_a0: Ptr<Ignored>) -> Self {
-        let __this: Value<Ignored> = Rc::new(RefCell::new(Self { v: -1_i32 }));
-        let this: Ptr<Ignored> = __this.as_pointer();
+        let __this: Ignored = Self { v: -1_i32 };
         (*copies_0.with(Value::clone).borrow_mut()).prefix_inc();
-        Rc::try_unwrap(__this).ok().unwrap().into_inner()
+        __this
     }
 }
 impl Clone for Ignored {