Translate move constructor and move assignment (#343)

Move constructor is translated as a normal constructor:

```rs
impl MoveOnly {
    pub unsafe fn MoveOnly_pmutMoveOnly(o: *mut MoveOnly) -> Self {
        let mut this = Self { v: (*o).v };
        (*o).v = 0;
        this
    }
}
```

Move assignment is translated using `operator_assign`:

```rs
impl MoveOnly {
    pub unsafe fn operator_assign_pmutMoveOnly(&mut self, o: *mut MoveOnly) -> *mut MoveOnly {
        if ((self as *mut MoveOnly) == (o)) {
            return &mut (*(self as *mut MoveOnly)) as *mut MoveOnly;
        }
        self.v = (*o).v;
        (*o).v = 0;
        return &mut (*(self as *mut MoveOnly)) as *mut MoveOnly;
    }
}
```

The combination defaulted move constructor and non-default copy
constructor is not supported yet. `std::move` is translated as a clone()
currently. It assumes that a clone is bitwise, i.e. the copy constructor
is implicit. If it's not, the `std::move` would call the user defined
copy constructor which is wrong. I will work on this in a future PR.

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -9,6 +9,7 @@
 #include <clang/Basic/SourceManager.h>
 #include <llvm/ADT/DenseMap.h>
 #include <llvm/Support/ConvertUTF.h>
+#include <llvm/Support/ErrorHandling.h>
 
 #include <algorithm>
 #include <format>
@@ -624,28 +625,6 @@ void Converter::EmitScopedDestructor(const clang::VarDecl *decl) {
                      kDestructorName));
 }
 
-static bool hasUserDefinedNonDefaultMoveCtor(clang::CXXRecordDecl *decl) {
-  for (const auto *ctor : decl->ctors()) {
-    if (ctor->isMoveConstructor()) {
-      auto source = ctor->getDefinition() ? ctor->getDefinition() : ctor;
-      if (source->isUserProvided() && !source->isDefaulted()) {
-        return true;
-      }
-    }
-  }
-
-  for (const auto *method : decl->methods()) {
-    if (method->isMoveAssignmentOperator()) {
-      auto source = method->getDefinition() ? method->getDefinition() : method;
-      if (source->isUserProvided() && !source->isDefaulted()) {
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
-
 bool IsPointerType(clang::QualType qual_type) {
   return qual_type->isPointerType() ||
          (qual_type->isArrayType() &&
@@ -954,10 +933,6 @@ bool Converter::VisitCXXRecordDecl(clang::CXXRecordDecl *decl) {
       return false;
     }
 
-    if (hasUserDefinedNonDefaultMoveCtor(decl)) {
-      assert(0 && "unsupported user-defined move ctor");
-    }
-
     sema_->ForceDeclarationOfImplicitMembers(decl);
     for (auto ctor : decl->ctors()) {
       if (ctor->isCopyConstructor() && ctor->isImplicit() &&
@@ -1057,7 +1032,7 @@ std::string Converter::GetSelfMaybeWithMut(const clang::CXXMethodDecl *decl) {
 }
 
 std::string Converter::GetCtorName(clang::CXXConstructorDecl *decl) {
-  if (decl->isCopyConstructor()) {
+  if (decl->isCopyOrMoveConstructor()) {
     return GetOverloadedFunctionName(decl);
   }
   return GetRecordName(decl->getParent()) +
@@ -1072,10 +1047,8 @@ bool Converter::VisitCXXConstructorDecl(clang::CXXConstructorDecl *decl) {
   }
   PushCurrFunction push_fn(*this, decl);
 
-  if (decl->isMoveConstructor()) {
-    assert(0 && "user-defined move constructor are not supported");
-  }
-  if (decl->isCopyConstructor() && !decl->doesThisDeclarationHaveABody()) {
+  if (decl->isCopyOrMoveConstructor() &&
+      !decl->doesThisDeclarationHaveABody()) {
     return false;
   }
 
@@ -1740,7 +1713,6 @@ bool Converter::VisitCallExpr(clang::CallExpr *expr) {
       return false;
     }
     StrCat(std::format("{}", ToString(expr->getArg(0))));
-    computed_expr_type_ = ComputedExprType::FreshValue;
     return false;
   }
 
@@ -3372,12 +3344,22 @@ bool Converter::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   }
 
   auto *ctor = expr->getConstructor();
+  // Default move is translated using a bitwise .clone() implementation.
+  // Bitwise clone is only satisfied by default copy constructor. If the copy
+  // constructor is user defined, then default move calls copy constructor,
+  // which is wrong.
+  if (IsDefaultedMoveConstructor(ctor) &&
+      !HasDefaultedCopyConstructor(ctor->getParent())) {
+    llvm::report_fatal_error("defaulted move constructor without a fieldwise "
+                             "copy constructor is not supported");
+  }
+
   if (IsPassThroughConstructor(ctor)) {
     // Take suppress before recursing into the child.
     bool suppress = PushSuppressIteratorClone::take(*this);
     Convert(expr->getArg(0));
-    if (ctor->isCopyConstructor() && !suppress &&
-        !TypeIsCopyable(expr->getType())) {
+    if ((ctor->isCopyConstructor() || IsDefaultedMoveConstructor(ctor)) &&
+        !suppress && !TypeIsCopyable(expr->getType())) {
       StrCat(".clone()");
     }
     return false;
@@ -3843,12 +3825,11 @@ Converter::GetStructAttributes(const clang::RecordDecl *decl) {
 
   std::vector<const char *> struct_attrs;
 
-  bool derive_clone =
-      IsCopyConstructible(decl) && !HasUserDefinedCopyConstructor(decl);
-  if (derive_clone && RecordHasCopyableFields(decl)) {
+  if (HasDefaultedCopyConstructor(decl) && RecordHasCopyableFields(decl)) {
     struct_attrs.emplace_back("Copy");
   }
-  if (derive_clone) {
+
+  if (HasDefaultedCopyConstructor(decl)) {
     struct_attrs.emplace_back("Clone");
   }
 
@@ -4040,7 +4021,8 @@ void Converter::ConvertAssignment(clang::Expr *lhs, clang::Expr *rhs,
 
   StrCat(lhs_as_string, assign_operator, rhs_as_string);
   if (!isVoid()) {
-    StrCat(token::kSemiColon, ConvertRValue(lhs));
+    StrCat(token::kSemiColon,
+           isAddrOf() ? ConvertRValue(lhs) : ConvertFreshRValue(lhs));
   }
 }
 

cpp2rust/converter/converter_lib.cpp

@@ -263,6 +263,21 @@ bool IsUserDefinedCopyConstructor(const clang::CXXConstructorDecl *ctor) {
          IsUserDefinedDecl(ctor);
 }
 
+bool IsUserDefinedMoveConstructor(const clang::CXXConstructorDecl *ctor) {
+  return ctor->isMoveConstructor() && ctor->isUserProvided() &&
+         IsUserDefinedDecl(ctor);
+}
+
+bool IsUserDefinedCopyOrMoveConstructor(const clang::CXXConstructorDecl *ctor) {
+  return IsUserDefinedCopyConstructor(ctor) ||
+         IsUserDefinedMoveConstructor(ctor);
+}
+
+bool IsDefaultedMoveConstructor(const clang::CXXConstructorDecl *ctor) {
+  return ctor->isMoveConstructor() && !ctor->isUserProvided() &&
+         IsUserDefinedDecl(ctor->getParent());
+}
+
 clang::CXXConstructorDecl *
 GetUserDefinedCopyConstructor(const clang::RecordDecl *decl) {
   auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
@@ -277,11 +292,20 @@ GetUserDefinedCopyConstructor(const clang::RecordDecl *decl) {
   return nullptr;
 }
 
-bool HasUserDefinedCopyConstructor(const clang::RecordDecl *decl) {
-  return GetUserDefinedCopyConstructor(decl) != nullptr;
+bool HasDefaultedCopyConstructor(const clang::RecordDecl *decl) {
+  auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
+  if (!cxx) {
+    return true;
+  }
+  for (const auto *ctor : cxx->ctors()) {
+    if (ctor->isCopyConstructor()) {
+      return !ctor->isUserProvided() && !ctor->isDeleted();
+    }
+  }
+  return !cxx->defaultedCopyConstructorIsDeleted();
 }
 
-bool IsCopyConstructible(const clang::RecordDecl *decl) {
+bool HasCallableCopyConstructor(const clang::RecordDecl *decl) {
   auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
   if (!cxx) {
     return true;
@@ -299,12 +323,13 @@ bool IsCopyConstructible(const clang::RecordDecl *decl) {
 }
 
 bool IsRValueConvertingConstructor(const clang::CXXConstructorDecl *ctor) {
-  return ctor->isConvertingConstructor(false) && ctor->getNumParams() == 1 &&
+  return !ctor->isCopyOrMoveConstructor() &&
+         ctor->isConvertingConstructor(false) && ctor->getNumParams() == 1 &&
          ctor->getParamDecl(0)->getType()->isRValueReferenceType();
 }
 
 bool IsPassThroughConstructor(const clang::CXXConstructorDecl *ctor) {
-  return !IsUserDefinedCopyConstructor(ctor) &&
+  return !IsUserDefinedCopyOrMoveConstructor(ctor) &&
          (ctor->isCopyOrMoveConstructor() ||
           IsRValueConvertingConstructor(ctor));
 }
@@ -313,9 +338,13 @@ bool IsConvertibleCXXRecordDecl(const clang::CXXRecordDecl *decl) {
   return decl->isThisDeclarationADefinition() &&
          std::all_of(
              decl->method_begin(), decl->method_end(), [](auto *method) {
+               auto *ctor = clang::dyn_cast<clang::CXXConstructorDecl>(method);
                return method->getDefinition() || method->isPureVirtual() ||
                       method->getTemplateInstantiationPattern() ||
-                      method->getDescribedFunctionTemplate();
+                      method->getDescribedFunctionTemplate() ||
+                      (ctor ? ctor->isCopyOrMoveConstructor()
+                            : method->isCopyAssignmentOperator() ||
+                                  method->isMoveAssignmentOperator());
              });
 }
 

cpp2rust/converter/converter_lib.h

@@ -65,12 +65,18 @@ bool IsOverloadedMethod(const clang::CXXMethodDecl *decl);
 
 bool IsUserDefinedCopyConstructor(const clang::CXXConstructorDecl *ctor);
 
+bool IsUserDefinedMoveConstructor(const clang::CXXConstructorDecl *ctor);
+
+bool IsUserDefinedCopyOrMoveConstructor(const clang::CXXConstructorDecl *ctor);
+
+bool IsDefaultedMoveConstructor(const clang::CXXConstructorDecl *ctor);
+
 clang::CXXConstructorDecl *
 GetUserDefinedCopyConstructor(const clang::RecordDecl *decl);
 
-bool HasUserDefinedCopyConstructor(const clang::RecordDecl *decl);
+bool HasCallableCopyConstructor(const clang::RecordDecl *decl);
 
-bool IsCopyConstructible(const clang::RecordDecl *decl);
+bool HasDefaultedCopyConstructor(const clang::RecordDecl *decl);
 
 bool IsRValueConvertingConstructor(const clang::CXXConstructorDecl *ctor);
 

cpp2rust/converter/models/converter_refcount.cpp

@@ -5,6 +5,7 @@
 
 #include <clang/AST/RecordLayout.h>
 #include <clang/Basic/OperatorKinds.h>
+#include <llvm/Support/ErrorHandling.h>
 
 #include <algorithm>
 #include <format>
@@ -491,7 +492,7 @@ void ConverterRefCount::AddCloneTrait(const clang::RecordDecl *decl) {
     return;
   }
 
-  if (!IsCopyConstructible(cxx)) {
+  if (!HasCallableCopyConstructor(cxx)) {
     return;
   }
 
@@ -1880,12 +1881,23 @@ bool ConverterRefCount::VisitCXXConstructExpr(clang::CXXConstructExpr *expr) {
   }
 
   auto *ctor = expr->getConstructor();
-  if (ctor->isMoveConstructor() || IsRValueConvertingConstructor(ctor)) {
+  if (IsRValueConvertingConstructor(ctor) ||
+      (ctor->isMoveConstructor() && !IsUserDefinedDecl(ctor->getParent()))) {
     StrCat(ConvertLValue(expr->getArg(0)));
     return false;
   }
 
-  if (ctor->isCopyConstructor() && !IsUserDefinedCopyConstructor(ctor)) {
+  // Default move is translated using a bitwise .clone() implementation.
+  // Bitwise clone is only satisfied by default copy constructor. If the copy
+  // constructor is user defined, then default move calls copy constructor,
+  // which is wrong.
+  if (IsDefaultedMoveConstructor(ctor) &&
+      !HasDefaultedCopyConstructor(ctor->getParent())) {
+    llvm::report_fatal_error("defaulted move constructor without a fieldwise "
+                             "copy constructor is not supported");
+  }
+  if (ctor->isCopyOrMoveConstructor() &&
+      !IsUserDefinedCopyOrMoveConstructor(ctor)) {
     StrCat(PushSuppressIteratorClone::take(*this)
                ? ConvertRValue(expr->getArg(0))
                : ConvertFreshRValue(expr->getArg(0)));
@@ -2632,7 +2644,8 @@ void ConverterRefCount::SetUFCSReceiver(clang::Expr *base, bool is_arrow,
     }
     return;
   }
-  if (!base->isLValue() && base->getType()->isRecordType()) {
+  if (!base->isLValue() && base->getType()->isRecordType() &&
+      !IsReferenceType(base->IgnoreImplicit())) {
     PushConversionKind push(*this, ConversionKind::FullRefCount);
     ufcs_receiver_ =
         token::kRef + BoxValue(ConvertRValue(base)) + ".as_pointer()";

tests/lit/lit/formats/Cpp2RustTest.py

@@ -131,6 +131,9 @@ def translate(self):
         )
         out, err, returncode = lit.util.executeCommand(cmd)
 
+        if returncode != 0 and exp.should_not_translate:
+            return (lit.Test.XFAIL, "")
+
         if not self.rs_file.exists():
             return (
                 exp.fail_code,

tests/unit/copy_move_defaulted.cpp

@@ -0,0 +1,105 @@
+// translation-fail
+#include <cassert>
+#include <utility>
+#include <vector>
+
+struct Inner {
+  int x;
+};
+
+struct Explicit {
+  int v;
+  Inner inner;
+  int arr[2];
+  Explicit(int v) : v(v), inner{v * 10}, arr{v, v + 1} {}
+  Explicit(const Explicit &) = default;
+  Explicit(Explicit &&) = default;
+  Explicit &operator=(const Explicit &) = default;
+  Explicit &operator=(Explicit &&) = default;
+  ~Explicit() {}
+};
+
+struct Implicit {
+  int v;
+  Inner inner;
+  int arr[2];
+};
+
+struct DefaultCopyUserMove {
+  int v;
+  DefaultCopyUserMove(int v) : v(v) {}
+  DefaultCopyUserMove(const DefaultCopyUserMove &) = default;
+  DefaultCopyUserMove(DefaultCopyUserMove &&o) : v(o.v) { o.v = 0; }
+  DefaultCopyUserMove &operator=(const DefaultCopyUserMove &) = default;
+  DefaultCopyUserMove &operator=(DefaultCopyUserMove &&o) {
+    v = o.v;
+    o.v = 0;
+    return *this;
+  }
+};
+
+struct UserCopyDefaultMove {
+  int v;
+  UserCopyDefaultMove(int v) : v(v) {}
+  UserCopyDefaultMove(const UserCopyDefaultMove &o) : v(o.v + 100) {}
+  UserCopyDefaultMove(UserCopyDefaultMove &&) = default;
+  UserCopyDefaultMove &operator=(const UserCopyDefaultMove &o) {
+    v = o.v + 100;
+    return *this;
+  }
+  UserCopyDefaultMove &operator=(UserCopyDefaultMove &&) = default;
+};
+
+static bool same(const Explicit &a, const Explicit &b) {
+  return a.v == b.v && a.inner.x == b.inner.x && a.arr[0] == b.arr[0] &&
+         a.arr[1] == b.arr[1];
+}
+
+int main() {
+  Explicit a(1);
+  Explicit b = a;
+  Explicit c(a);
+  Explicit d(std::move(a));
+  assert(same(b, a) && same(c, a) && same(d, a));
+
+  Explicit e(2), f(3);
+  e = b;
+  f = std::move(c);
+  assert(same(e, b) && same(f, c));
+  Explicit g(4);
+  g = e = f;
+  assert(same(g, f) && same(e, f));
+
+  Implicit i{5, {50}, {5, 6}};
+  Implicit j = i;
+  Implicit k = std::move(i);
+  assert(j.v == 5 && j.inner.x == 50 && j.arr[1] == 6);
+  assert(i.v == 5 && k.v == 5);
+  Implicit l{0, {0}, {0, 0}};
+  l = j;
+  assert(l.v == 5 && l.inner.x == 50 && l.arr[0] == 5);
+
+  std::vector<Explicit> vec;
+  vec.push_back(b);
+  vec.push_back(Explicit(9));
+  assert(vec[0].v == 1 && vec[1].v == 9);
+
+  DefaultCopyUserMove m(7);
+  DefaultCopyUserMove m1 = m;
+  DefaultCopyUserMove m2 = std::move(m);
+  assert(m1.v == 7 && m2.v == 7 && m.v == 0);
+  DefaultCopyUserMove m3(1), m4(1);
+  m3 = m1;
+  m4 = std::move(m1);
+  assert(m3.v == 7 && m4.v == 7 && m1.v == 0);
+
+  UserCopyDefaultMove u(8);
+  UserCopyDefaultMove u1 = u;
+  UserCopyDefaultMove u2 = std::move(u);
+  assert(u1.v == 108 && u2.v == 8 && u.v == 8);
+  UserCopyDefaultMove u3(1), u4(1);
+  u3 = u2;
+  u4 = std::move(u2);
+  assert(u3.v == 108 && u4.v == 8);
+  return 0;
+}