[cxx-interop] Revert changes to copy/move constructors definition

include/swift/ClangImporter/ClangImporter.h

@@ -733,12 +733,6 @@ ValueDecl *getImportedMemberOperator(const DeclBaseName &name,
 /// as permissive as the input C++ access.
 AccessLevel convertClangAccess(clang::AccessSpecifier access);
 
-/// Lookup and return the copy constructor of \a decl
-///
-/// Returns nullptr if \a decl doesn't have a valid copy constructor
-const clang::CXXConstructorDecl *
-findCopyConstructor(const clang::CXXRecordDecl *decl);
-
 /// Read file IDs from 'private_fileid' Swift attributes on a Clang decl.
 ///
 /// May return >1 fileID when a decl is annotated more than once, which should

lib/ClangImporter/ClangImporter.cpp

@@ -8322,39 +8322,16 @@ bool importer::isViewType(const clang::CXXRecordDecl *decl) {
   return !hasOwnedValueAttr(decl) && hasPointerInSubobjects(decl);
 }
 
-const clang::CXXConstructorDecl *
-importer::findCopyConstructor(const clang::CXXRecordDecl *decl) {
-  for (auto ctor : decl->ctors()) {
-    if (ctor->isCopyConstructor() &&
-        // FIXME: Support default arguments (rdar://142414553)
-        ctor->getNumParams() == 1 && ctor->getAccess() == clang::AS_public &&
-        !ctor->isDeleted() && !ctor->isIneligibleOrNotSelected())
-      return ctor;
-  }
-  return nullptr;
-}
-
-static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl,
-                                  ClangImporter::Implementation *importerImpl) {
-  if (!decl->hasSimpleCopyConstructor()) {
-    auto *copyCtor = findCopyConstructor(decl);
-    if (!copyCtor)
-      return false;
-
-    if (!copyCtor->isDefaulted())
-      return true;
-  }
+static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
+  if (decl->hasSimpleCopyConstructor())
+    return true;
 
-  // If the copy constructor is defaulted or implicitly declared, we should only
-  // import the type as copyable if all its fields are also copyable
-  // FIXME: we should also look at the bases
-  return llvm::none_of(decl->fields(), [&](clang::FieldDecl *field) {
-    if (const auto *rd = field->getType()->getAsRecordDecl()) {
-      return (!field->getType()->isReferenceType() &&
-              !field->getType()->isPointerType() &&
-              recordHasMoveOnlySemantics(rd, importerImpl));
-    }
-    return false;
+  return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
+    return ctor->isCopyConstructor() && !ctor->isDeleted() &&
+           !ctor->isIneligibleOrNotSelected() &&
+           // FIXME: Support default arguments (rdar://142414553)
+           ctor->getNumParams() == 1 &&
+           ctor->getAccess() == clang::AccessSpecifier::AS_public;
   });
 }
 
@@ -8553,8 +8530,8 @@ CxxValueSemantics::evaluate(Evaluator &evaluator,
     return CxxValueSemanticsKind::Copyable;
   }
 
-  bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) &&
-                    hasCopyTypeOperations(cxxRecordDecl, importerImpl);
+  bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) && 
+                    hasCopyTypeOperations(cxxRecordDecl);
   bool isMovable = hasMoveTypeOperations(cxxRecordDecl);
 
   if (!hasDestroyTypeOperations(cxxRecordDecl) || (!isCopyable && !isMovable)) {

lib/ClangImporter/ImportDecl.cpp

@@ -186,15 +186,6 @@ bool importer::recordHasReferenceSemantics(
   return semanticsKind == CxxRecordSemanticsKind::Reference;
 }
 
-bool importer::recordHasMoveOnlySemantics(
-    const clang::RecordDecl *decl,
-    ClangImporter::Implementation *importerImpl) {
-  auto semanticsKind = evaluateOrDefault(
-      importerImpl->SwiftContext.evaluator,
-      CxxValueSemantics({decl->getTypeForDecl(), importerImpl}), {});
-  return semanticsKind == CxxValueSemanticsKind::MoveOnly;
-}
-
 bool importer::hasImmortalAttrs(const clang::RecordDecl *decl) {
   return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
            if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
@@ -2104,7 +2095,10 @@ namespace {
     }
 
     bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl) {
-      return importer::recordHasMoveOnlySemantics(decl, &Impl);
+      auto semanticsKind = evaluateOrDefault(
+          Impl.SwiftContext.evaluator,
+          CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {});
+      return semanticsKind == CxxValueSemanticsKind::MoveOnly;
     }
 
     void markReturnsUnsafeNonescapable(AbstractFunctionDecl *fd) {
@@ -2283,9 +2277,13 @@ namespace {
             loc, ArrayRef<InheritedEntry>(), nullptr, dc);
       Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;
 
-      if (decl->isInStdNamespace() && decl->getName() == "promise" && recordHasMoveOnlySemantics(decl)) {
-        // Do not import std::promise.
-        return nullptr;
+      if (recordHasMoveOnlySemantics(decl)) {
+        if (decl->isInStdNamespace() && decl->getName() == "promise") {
+          // Do not import std::promise.
+          return nullptr;
+        }
+        result->addAttribute(new (Impl.SwiftContext)
+                                 MoveOnlyAttr(/*Implicit=*/true));
       }
 
       // FIXME: Figure out what to do with superclasses in C++. One possible
@@ -2532,11 +2530,6 @@ namespace {
                                  cxxRecordDecl->ctors().empty());
       }
 
-      if (recordHasMoveOnlySemantics(decl)) {
-        result->getAttrs().add(new (Impl.SwiftContext)
-                                   MoveOnlyAttr(/*Implicit=*/true));
-      }
-
       // TODO: builtin "zeroInitializer" does not work with non-escapable
       // types yet. Don't generate an initializer.
       if (hasZeroInitializableStorage && needsEmptyInitializer &&
@@ -3132,13 +3125,15 @@ namespace {
         // instantiate its copy constructor.
         bool isExplicitlyNonCopyable = hasNonCopyableAttr(decl);
 
+        clang::CXXConstructorDecl *copyCtor = nullptr;
+        clang::CXXConstructorDecl *moveCtor = nullptr;
         clang::CXXConstructorDecl *defaultCtor = nullptr;
         if (decl->needsImplicitCopyConstructor() && !isExplicitlyNonCopyable) {
-          clangSema.DeclareImplicitCopyConstructor(
+          copyCtor = clangSema.DeclareImplicitCopyConstructor(
               const_cast<clang::CXXRecordDecl *>(decl));
         }
         if (decl->needsImplicitMoveConstructor()) {
-          clangSema.DeclareImplicitMoveConstructor(
+          moveCtor = clangSema.DeclareImplicitMoveConstructor(
               const_cast<clang::CXXRecordDecl *>(decl));
         }
         if (decl->needsImplicitDefaultConstructor()) {
@@ -3155,13 +3150,28 @@ namespace {
                 // Note: we use "doesThisDeclarationHaveABody" here because
                 // that's what "DefineImplicitCopyConstructor" checks.
                 !declCtor->doesThisDeclarationHaveABody()) {
-              if (declCtor->isDefaultConstructor()) {
+              if (declCtor->isCopyConstructor()) {
+                if (!copyCtor && !isExplicitlyNonCopyable)
+                  copyCtor = declCtor;
+              } else if (declCtor->isMoveConstructor()) {
+                if (!moveCtor)
+                  moveCtor = declCtor;
+              } else if (declCtor->isDefaultConstructor()) {
                 if (!defaultCtor)
                   defaultCtor = declCtor;
               }
             }
           }
         }
+        if (copyCtor && !isExplicitlyNonCopyable &&
+            !decl->isAnonymousStructOrUnion()) {
+          clangSema.DefineImplicitCopyConstructor(clang::SourceLocation(),
+                                                  copyCtor);
+        }
+        if (moveCtor && !decl->isAnonymousStructOrUnion()) {
+          clangSema.DefineImplicitMoveConstructor(clang::SourceLocation(),
+                                                  moveCtor);
+        }
         if (defaultCtor) {
           clangSema.DefineImplicitDefaultConstructor(clang::SourceLocation(),
                                                      defaultCtor);
@@ -3170,8 +3180,7 @@ namespace {
         if (decl->needsImplicitDestructor()) {
           auto dtor = clangSema.DeclareImplicitDestructor(
               const_cast<clang::CXXRecordDecl *>(decl));
-          if (!dtor->isDeleted() && !dtor->isIneligibleOrNotSelected())
-            clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor);
+          clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor);
         }
       }
 

lib/ClangImporter/ImporterImpl.h

@@ -1974,11 +1974,6 @@ namespace importer {
 bool recordHasReferenceSemantics(const clang::RecordDecl *decl,
                                  ClangImporter::Implementation *importerImpl);
 
-/// Returns true if the given C/C++ record should be imported as non-copyable into
-/// Swift.
-bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl,
-                                ClangImporter::Implementation *importerImpl);
-
 /// Whether this is a forward declaration of a type. We ignore forward
 /// declarations in certain cases, and instead process the real declarations.
 bool isForwardDeclOfType(const clang::Decl *decl);

lib/IRGen/GenStruct.cpp

@@ -553,7 +553,15 @@ namespace {
       const auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(ClangDecl);
       if (!cxxRecordDecl)
         return nullptr;
-      return importer::findCopyConstructor(cxxRecordDecl);
+      for (auto ctor : cxxRecordDecl->ctors()) {
+        if (ctor->isCopyConstructor() &&
+            // FIXME: Support default arguments (rdar://142414553)
+            ctor->getNumParams() == 1 &&
+            ctor->getAccess() == clang::AS_public && !ctor->isDeleted() &&
+            !ctor->isIneligibleOrNotSelected())
+          return ctor;
+      }
+      return nullptr;
     }
 
     const clang::CXXConstructorDecl *findMoveConstructor() const {
@@ -611,7 +619,7 @@ namespace {
           /*invocation subs*/ SubstitutionMap(), IGF.IGM.Context);
     }
 
-    void emitCopyWithCopyOrMoveConstructor(
+    void emitCopyWithCopyConstructor(
         IRGenFunction &IGF, SILType T,
         const clang::CXXConstructorDecl *copyConstructor, llvm::Value *src,
         llvm::Value *dest) const {
@@ -621,27 +629,7 @@ namespace {
 
       auto &ctx = IGF.IGM.Context;
       auto *importer = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
-
-      if (copyConstructor->isDefaulted() &&
-          copyConstructor->getAccess() == clang::AS_public &&
-          !copyConstructor->isDeleted() &&
-          !copyConstructor->isIneligibleOrNotSelected() &&
-          // Note: we use "doesThisDeclarationHaveABody" here because
-          // that's what "DefineImplicitCopyConstructor" checks.
-          !copyConstructor->doesThisDeclarationHaveABody()) {
-        assert(!copyConstructor->getParent()->isAnonymousStructOrUnion() &&
-               "Cannot do codegen of special member functions of anonymous "
-               "structs/unions");
-        if (copyConstructor->isCopyConstructor())
-          importer->getClangSema().DefineImplicitCopyConstructor(
-              clang::SourceLocation(),
-              const_cast<clang::CXXConstructorDecl *>(copyConstructor));
-        else
-          importer->getClangSema().DefineImplicitMoveConstructor(
-              clang::SourceLocation(),
-              const_cast<clang::CXXConstructorDecl *>(copyConstructor));
-      }
-
+
       auto &diagEngine = importer->getClangSema().getDiagnostics();
       clang::DiagnosticErrorTrap trap(diagEngine);
       auto clangFnAddr =
@@ -821,9 +809,9 @@ namespace {
                             Address srcAddr, SILType T,
                             bool isOutlined) const override {
       if (auto copyConstructor = findCopyConstructor()) {
-        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
-                                          srcAddr.getAddress(),
-                                          destAddr.getAddress());
+        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
+                                    srcAddr.getAddress(),
+                                    destAddr.getAddress());
         return;
       }
       StructTypeInfoBase<AddressOnlyCXXClangRecordTypeInfo, FixedTypeInfo,
@@ -836,9 +824,9 @@ namespace {
                         SILType T, bool isOutlined) const override {
       if (auto copyConstructor = findCopyConstructor()) {
         destroy(IGF, destAddr, T, isOutlined);
-        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
-                                          srcAddr.getAddress(),
-                                          destAddr.getAddress());
+        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
+                                    srcAddr.getAddress(),
+                                    destAddr.getAddress());
         return;
       }
       StructTypeInfoBase<AddressOnlyCXXClangRecordTypeInfo, FixedTypeInfo,
@@ -850,15 +838,17 @@ namespace {
                             SILType T, bool isOutlined,
                             bool zeroizeIfSensitive) const override {
       if (auto moveConstructor = findMoveConstructor()) {
-        emitCopyWithCopyOrMoveConstructor(IGF, T, moveConstructor,
-                                          src.getAddress(), dest.getAddress());
+        emitCopyWithCopyConstructor(IGF, T, moveConstructor,
+                                    src.getAddress(),
+                                    dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }
 
       if (auto copyConstructor = findCopyConstructor()) {
-        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
-                                          src.getAddress(), dest.getAddress());
+        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
+                                    src.getAddress(),
+                                    dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }
@@ -872,16 +862,18 @@ namespace {
                         bool isOutlined) const override {
       if (auto moveConstructor = findMoveConstructor()) {
         destroy(IGF, dest, T, isOutlined);
-        emitCopyWithCopyOrMoveConstructor(IGF, T, moveConstructor,
-                                          src.getAddress(), dest.getAddress());
+        emitCopyWithCopyConstructor(IGF, T, moveConstructor,
+                                    src.getAddress(),
+                                    dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }
 
       if (auto copyConstructor = findCopyConstructor()) {
         destroy(IGF, dest, T, isOutlined);
-        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
-                                          src.getAddress(), dest.getAddress());
+        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
+                                    src.getAddress(),
+                                    dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }