Fix #549: std::swap and std::sort copy elements rather than moving them in place

Also, handle copy and move assignment operators

Author: nunoplopes

cpp2rust/converter/converter.cpp

@@ -793,8 +793,9 @@ bool Converter::RecordHasCopyableFields(const clang::RecordDecl *decl) {
       }
     }
 
-    // Look recursively into fields that are RecordDecl
-    if (auto field_record = f->getType()->getAsRecordDecl()) {
+    // Look recursively into fields that are RecordDecl, or arrays of them
+    if (auto field_record =
+            ctx_.getBaseElementType(f->getType())->getAsRecordDecl()) {
       if (!RecordDerivesCopy(field_record)) {
         return false;
       }
@@ -3411,7 +3412,8 @@ bool Converter::VisitMemberExpr(clang::MemberExpr *expr) {
     if (is_reference_type || RuleRegistry::ReturnsPointer(ctx_, expr)) {
       computed_expr_type_ = ComputedExprType::Pointer;
     } else {
-      StrCat(token::kRef);
+      StrCat(token::kRef,
+             expr->getType().isConstQualified() ? "" : keyword_mut_);
       computed_expr_type_ = ComputedExprType::FreshPointer;
     }
     StrCat(str);
@@ -4365,6 +4367,14 @@ Converter::GetStructAttributes(const clang::RecordDecl *decl) {
     struct_attrs.emplace_back("MoveCtorUnsafe");
   }
 
+  if (HasCopyAssignOperator(decl)) {
+    struct_attrs.emplace_back("CopyAssignUnsafe");
+  }
+
+  if (HasMoveAssignOperator(decl)) {
+    struct_attrs.emplace_back("MoveAssignUnsafe");
+  }
+
   if (auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
       cxx && RecordNeedsDestruction(cxx)) {
     struct_attrs.emplace_back("DestructorUnsafe");

cpp2rust/converter/converter_lib.cpp

@@ -533,11 +533,61 @@ bool IsConvertibleMoveAssignment(const clang::CXXMethodDecl *method) {
          HasFieldWithMoveAssign(parent);
 }
 
+static bool IsTranslatedCopyAssignment(const clang::CXXMethodDecl *method) {
+  if (!method->isCopyAssignmentOperator() || method->isDeleted() ||
+      method->isVirtual()) {
+    return false;
+  }
+  return (method->isUserProvided() && IsUserDefinedDecl(method) &&
+          method->hasBody()) ||
+         IsConvertibleCopyAssignment(method);
+}
+
+bool HasCopyAssignOperator(const clang::RecordDecl *decl) {
+  auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
+  return cxx && llvm::any_of(cxx->methods(), [](auto *method) {
+           return IsTranslatedCopyAssignment(method) &&
+                  CanUseCopyOrMoveName(method, "copy_assign");
+         });
+}
+
+bool HasMoveAssignOperator(const clang::RecordDecl *decl) {
+  auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
+  return cxx && llvm::any_of(cxx->methods(), [](auto *method) {
+           return IsConvertibleMoveAssignment(method) && !method->isVirtual() &&
+                  CanUseCopyOrMoveName(method, "move_assign");
+         });
+}
+
+// Whether a defaulted copy assignment of decl isn't an assignment of a clone
+// of the source, which runs the copy constructors of the record and of its
+// fields: when the record has a copy constructor, or a field (or element of
+// an array field) has a copy assignment operator.
+static bool NeedsDefaultedCopyAssignment(const clang::CXXRecordDecl *decl) {
+  return GetUserDefinedCopyConstructor(decl) ||
+         llvm::any_of(decl->fields(), [](const clang::FieldDecl *field) {
+           auto *record =
+               field->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl();
+           return record && HasCopyAssignOperator(record);
+         });
+}
+
+bool IsConvertibleCopyAssignment(const clang::CXXMethodDecl *method) {
+  auto *parent = method->getParent();
+  if (!method->isCopyAssignmentOperator() || method->isUserProvided() ||
+      method->isDeleted() || !method->hasBody() ||
+      !IsConvertibleImplicitMemberParent(parent)) {
+    return false;
+  }
+  return NeedsDefaultedCopyAssignment(parent);
+}
+
 bool IsConvertibleImplicitMember(const clang::CXXMethodDecl *method) {
   if (auto *ctor = clang::dyn_cast<clang::CXXConstructorDecl>(method)) {
     return IsConvertibleMoveConstructor(ctor);
   }
-  return IsConvertibleMoveAssignment(method);
+  return IsConvertibleMoveAssignment(method) ||
+         IsConvertibleCopyAssignment(method);
 }
 
 bool IsDefaultedMoveConstructor(const clang::CXXConstructorDecl *ctor) {
@@ -1263,7 +1313,7 @@ bool IsImplicitAssignmentCall(const clang::CallExpr *expr) {
   if (method->isUserProvided() && IsUserDefinedDecl(method)) {
     return false;
   }
-  return !IsConvertibleMoveAssignment(method);
+  return !IsConvertibleImplicitMember(method);
 }
 
 const clang::CXXRecordDecl *AsLambdaClass(clang::QualType type) {
@@ -1421,7 +1471,7 @@ bool IsUserOperatorCall(const clang::CXXOperatorCallExpr *expr) {
     return IsUserDefinedDecl(owner);
   }
   if (const auto *method = clang::dyn_cast<clang::CXXMethodDecl>(callee);
-      method && IsConvertibleMoveAssignment(method)) {
+      method && IsConvertibleImplicitMember(method)) {
     return true;
   }
   if (!callee->isUserProvided() || !IsUserDefinedDecl(callee)) {
@@ -1554,6 +1604,11 @@ void DefineImplicitMembers(clang::Sema &sema, clang::CXXRecordDecl *decl) {
         !HasDefaultedCopyAssignment(decl)) {
       sema.DefineImplicitMoveAssignment(decl->getLocation(), method);
     }
+    if (method->isCopyAssignmentOperator() && !method->isUserProvided() &&
+        !method->doesThisDeclarationHaveABody() && !method->isDeleted() &&
+        NeedsDefaultedCopyAssignment(decl)) {
+      sema.DefineImplicitCopyAssignment(decl->getLocation(), method);
+    }
   }
   auto define_defaulted_comparison = [&](clang::FunctionDecl *fn) {
     if (!fn || !IsComparisonOperator(fn) || !fn->isDefaulted() ||

cpp2rust/converter/converter_lib.h

@@ -100,6 +100,16 @@ bool HasMoveFromConstructor(const clang::RecordDecl *decl);
 
 bool IsConvertibleMoveAssignment(const clang::CXXMethodDecl *method);
 
+bool IsConvertibleCopyAssignment(const clang::CXXMethodDecl *method);
+
+// Whether decl has a copy assignment operator converted to a copy_assign
+// method.
+bool HasCopyAssignOperator(const clang::RecordDecl *decl);
+
+// Whether decl has a move assignment operator converted to a move_assign
+// method.
+bool HasMoveAssignOperator(const clang::RecordDecl *decl);
+
 bool IsConvertibleImplicitMember(const clang::CXXMethodDecl *method);
 
 clang::CXXConstructorDecl *

cpp2rust/converter/models/converter_refcount.cpp

@@ -2520,6 +2520,14 @@ ConverterRefCount::GetStructAttributes(const clang::RecordDecl *decl) {
     attrs.emplace_back("MoveCtor");
   }
 
+  if (HasCopyAssignOperator(decl)) {
+    attrs.emplace_back("CopyAssign");
+  }
+
+  if (HasMoveAssignOperator(decl)) {
+    attrs.emplace_back("MoveAssign");
+  }
+
   if (auto *cxx = clang::dyn_cast<clang::CXXRecordDecl>(decl);
       cxx && RecordNeedsDestruction(cxx)) {
     attrs.emplace_back("Destructor");
@@ -2837,7 +2845,10 @@ bool ConverterRefCount::ConvertCXXOperatorCallExpr(
                            ConvertSubscriptIndex(expr->getArg(1))));
       }
 
-      if (is_inner_boxed) {
+      // The pointer to the element is already a pointer to the whole inner
+      // container, which as_pointer can't tell from a pointer to its first
+      // element without a type.
+      if (is_inner_boxed && (!isObject() || WantsElementPtr())) {
         StrCat(".as_pointer()");
         if (!isObject()) {
           StrCat(std::format("as Ptr<{}>)", ToString(expr->getType())));

libcc2rs-macros/src/assign.rs

@@ -0,0 +1,49 @@
+// Copyright (c) 2022-present INESC-ID.
+// Distributed under the MIT license that can be found in the LICENSE file.
+
+use proc_macro::TokenStream;
+use quote::{format_ident, quote};
+use syn::{DeriveInput, parse_macro_input};
+
+// Implements libcc2rs::CopyAssign or MoveAssign (or their unsafe versions)
+// with the method of the same name, for `op` "copy" or "move".
+pub fn expand(input: TokenStream, op: &str, is_unsafe: bool) -> TokenStream {
+    let input = parse_macro_input!(input as DeriveInput);
+    let name = &input.ident;
+    let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
+    let method = format_ident!("{}_assign", op);
+    let is_move = op == "move";
+    if is_unsafe {
+        let tr = if is_move {
+            quote!(::libcc2rs::MoveAssignUnsafe)
+        } else {
+            quote!(::libcc2rs::CopyAssignUnsafe)
+        };
+        let src_ty = if is_move {
+            quote!(*mut Self)
+        } else {
+            quote!(*const Self)
+        };
+        quote! {
+            impl #impl_generics #tr for #name #ty_generics #where_clause {
+                unsafe fn #method(dst: *mut Self, src: #src_ty) {
+                    unsafe { (*dst).#method(src) };
+                }
+            }
+        }
+    } else {
+        let tr = if is_move {
+            quote!(::libcc2rs::MoveAssign)
+        } else {
+            quote!(::libcc2rs::CopyAssign)
+        };
+        quote! {
+            impl #impl_generics #tr for #name #ty_generics #where_clause {
+                fn #method(dst: &::libcc2rs::Ptr<Self>, src: ::libcc2rs::Ptr<Self>) {
+                    dst.#method(src);
+                }
+            }
+        }
+    }
+    .into()
+}

libcc2rs-macros/src/lib.rs

@@ -3,6 +3,7 @@
 
 use proc_macro::TokenStream;
 
+mod assign;
 mod byte_repr;
 mod destructor;
 mod fn_ptr_arg;
@@ -165,6 +166,38 @@ pub fn derive_move_ctor_unsafe(input: TokenStream) -> TokenStream {
     move_ctor::expand(input, true)
 }
 
+//     #[derive(CopyAssign, MoveAssign)]
+//     pub struct S { ... }
+//     impl SImpl for Ptr<S> {
+//         fn copy_assign(&self, o: Ptr<S>) -> Ptr<S> { ... }
+//         fn move_assign(&self, o: Ptr<S>) -> Ptr<S> { ... }
+//     }
+//
+// Implements libcc2rs::CopyAssign and MoveAssign for S with its copy and move
+// assignment operators, such that std::swap assigns the values of type S
+// with them, as C++ does. CopyAssignUnsafe and MoveAssignUnsafe are the same
+// for the unsafe model, whose copy_assign and move_assign are methods of S.
+
+#[proc_macro_derive(CopyAssign)]
+pub fn derive_copy_assign(input: TokenStream) -> TokenStream {
+    assign::expand(input, "copy", false)
+}
+
+#[proc_macro_derive(CopyAssignUnsafe)]
+pub fn derive_copy_assign_unsafe(input: TokenStream) -> TokenStream {
+    assign::expand(input, "copy", true)
+}
+
+#[proc_macro_derive(MoveAssign)]
+pub fn derive_move_assign(input: TokenStream) -> TokenStream {
+    assign::expand(input, "move", false)
+}
+
+#[proc_macro_derive(MoveAssignUnsafe)]
+pub fn derive_move_assign_unsafe(input: TokenStream) -> TokenStream {
+    assign::expand(input, "move", true)
+}
+
 //     #[derive(Destructor)]
 //     pub struct S { ... }
 //     impl SImpl for Ptr<S> {

libcc2rs/src/capture.rs

@@ -26,6 +26,7 @@ use std::rc::Rc;
 use crate::fn_ptr::{FnPtr, FnSig};
 use crate::rc::{AsPointer, Ptr, Value};
 use crate::reinterpret::ByteRepr;
+use crate::{Map, UnsafeMap};
 
 // The C++ move constructor of a type, in the refcount model, for the types
 // whose move isn't a copy. Records implement it with #[derive(MoveCtor)].
@@ -41,6 +42,36 @@ pub trait MoveCtorUnsafe: Sized {
     unsafe fn move_ctor(src: *mut Self) -> Self;
 }
 
+// The C++ copy assignment operator of a type, in the refcount model, for the
+// types whose copy assignment isn't an assignment of a clone. Records
+// implement it with #[derive(CopyAssign)].
+pub trait CopyAssign: Sized {
+    fn copy_assign(dst: &Ptr<Self>, src: Ptr<Self>);
+}
+
+// The C++ copy assignment operator of a type, in the unsafe model. Records
+// implement it with #[derive(CopyAssignUnsafe)].
+pub trait CopyAssignUnsafe: Sized {
+    /// # Safety
+    /// `dst` and `src` must point to live values.
+    unsafe fn copy_assign(dst: *mut Self, src: *const Self);
+}
+
+// The C++ move assignment operator of a type, in the refcount model, for the
+// types whose move assignment isn't an assignment of a moved value. Records
+// implement it with #[derive(MoveAssign)].
+pub trait MoveAssign: Sized {
+    fn move_assign(dst: &Ptr<Self>, src: Ptr<Self>);
+}
+
+// The C++ move assignment operator of a type, in the unsafe model. Records
+// implement it with #[derive(MoveAssignUnsafe)].
+pub trait MoveAssignUnsafe: Sized {
+    /// # Safety
+    /// `dst` and `src` must point to live values.
+    unsafe fn move_assign(dst: *mut Self, src: *mut Self);
+}
+
 impl<T: FnSig> MoveCtor for FnPtr<T> {
     fn move_ctor(src: Ptr<Self>) -> Self {
         src.with(FnPtr::move_from)
@@ -76,6 +107,8 @@ macro_rules! impl_move_ctor_by_take {
 impl_move_ctor_by_take!(
     <T> Vec<T>,
     <K, V> BTreeMap<K, V>,
+    <K, T> Map<K, T>,
+    <K, T> UnsafeMap<K, T>,
 );
 
 // unique_ptr.

libcc2rs/src/lib.rs

@@ -40,7 +40,17 @@ mod fn_ptr;
 pub use fn_ptr::{FnPtr, Generic};
 
 mod capture;
-pub use capture::{Destructor, DestructorUnsafe, MoveCtor, MoveCtorUnsafe};
+pub use capture::{
+    CopyAssign, CopyAssignUnsafe, Destructor, DestructorUnsafe, MoveAssign, MoveAssignUnsafe,
+    MoveCtor, MoveCtorUnsafe,
+};
+
+mod swap;
+// Used by cpp_swap! and cpp_swap_unsafe!.
+#[doc(hidden)]
+pub mod __swap {
+    pub use crate::swap::*;
+}
 // Used by lambda! and lambda_unsafe!.
 #[doc(hidden)]
 pub mod __capture {
@@ -86,6 +96,7 @@ mod format;
 pub use format::*;
 
 pub use libcc2rs_macros::{
-    ByteRepr, Destructor, DestructorUnsafe, FnPtrArg, MoveCtor, MoveCtorUnsafe, Record, VaArg,
-    goto, goto_block, lambda, lambda_unsafe, switch,
+    ByteRepr, CopyAssign, CopyAssignUnsafe, Destructor, DestructorUnsafe, FnPtrArg, MoveAssign,
+    MoveAssignUnsafe, MoveCtor, MoveCtorUnsafe, Record, VaArg, goto, goto_block, lambda,
+    lambda_unsafe, switch,
 };