[cxx-interop] Reapply implicitly defined copy and move constructors

include/swift/ClangImporter/ClangImporter.h

@@ -737,6 +737,12 @@ 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

include/swift/ClangImporter/ClangImporterRequests.h

@@ -575,15 +575,7 @@ SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc);
 // When a reference type is copied, the pointer’s value is copied rather than
 // the object’s storage. This means reference types can be imported as
 // copyable to Swift, even when they are non-copyable in C++.
-enum class CxxValueSemanticsKind {
-  Unknown,
-  Copyable,
-  MoveOnly,
-  // A record that is either not copyable/movable or not destructible.
-  MissingLifetimeOperation,
-  // A record that has no copy and no move operations
-  UnavailableConstructors,
-};
+enum class CxxValueSemanticsKind { Unknown, Copyable, MoveOnly };
 
 struct CxxValueSemanticsDescriptor final {
   const clang::Type *type;

lib/ClangImporter/ImportDecl.cpp

@@ -2103,11 +2103,11 @@ namespace {
       return importer::recordHasReferenceSemantics(decl, &Impl);
     }
 
-    bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl) {
+    bool recordIsCopyable(const clang::RecordDecl *decl) {
       auto semanticsKind = evaluateOrDefault(
           Impl.SwiftContext.evaluator,
           CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {});
-      return semanticsKind == CxxValueSemanticsKind::MoveOnly;
+      return semanticsKind == CxxValueSemanticsKind::Copyable;
     }
 
     void markReturnsUnsafeNonescapable(AbstractFunctionDecl *fd) {
@@ -2286,7 +2286,7 @@ namespace {
             loc, ArrayRef<InheritedEntry>(), nullptr, dc);
       Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;
 
-      if (recordHasMoveOnlySemantics(decl)) {
+      if (!recordIsCopyable(decl)) {
         if (decl->isInStdNamespace() && decl->getName() == "promise") {
           // Do not import std::promise.
           return nullptr;
@@ -3106,15 +3106,13 @@ 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) {
-          copyCtor = clangSema.DeclareImplicitCopyConstructor(
+          clangSema.DeclareImplicitCopyConstructor(
               const_cast<clang::CXXRecordDecl *>(decl));
         }
         if (decl->needsImplicitMoveConstructor()) {
-          moveCtor = clangSema.DeclareImplicitMoveConstructor(
+          clangSema.DeclareImplicitMoveConstructor(
               const_cast<clang::CXXRecordDecl *>(decl));
         }
         if (decl->needsImplicitDefaultConstructor()) {
@@ -3131,28 +3129,13 @@ namespace {
                 // Note: we use "doesThisDeclarationHaveABody" here because
                 // that's what "DefineImplicitCopyConstructor" checks.
                 !declCtor->doesThisDeclarationHaveABody()) {
-              if (declCtor->isCopyConstructor()) {
-                if (!copyCtor && !isExplicitlyNonCopyable)
-                  copyCtor = declCtor;
-              } else if (declCtor->isMoveConstructor()) {
-                if (!moveCtor)
-                  moveCtor = declCtor;
-              } else if (declCtor->isDefaultConstructor()) {
+              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);
@@ -3161,7 +3144,8 @@ namespace {
         if (decl->needsImplicitDestructor()) {
           auto dtor = clangSema.DeclareImplicitDestructor(
               const_cast<clang::CXXRecordDecl *>(decl));
-          clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor);
+          if (!dtor->isDeleted() && !dtor->isIneligibleOrNotSelected())
+            clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor);
         }
       }
 
@@ -3170,8 +3154,7 @@ namespace {
       auto valueSemanticsKind = evaluateOrDefault(
           Impl.SwiftContext.evaluator,
           CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {});
-      if (valueSemanticsKind == CxxValueSemanticsKind::MissingLifetimeOperation ||
-          valueSemanticsKind == CxxValueSemanticsKind::UnavailableConstructors) {
+      if (valueSemanticsKind == CxxValueSemanticsKind::Unknown) {
 
         HeaderLoc loc(decl->getLocation());
         if (hasUnsafeAPIAttr(decl))
@@ -3184,12 +3167,6 @@ namespace {
           Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_iterator",
                         decl->getNameAsString());
 
-        if (valueSemanticsKind == CxxValueSemanticsKind::UnavailableConstructors) {
-          Impl.addImportDiagnostic(
-              decl, Diagnostic(diag::record_unsupported_default_args),
-              decl->getLocation());
-        }
-
         Impl.addImportDiagnostic(
             decl,
             Diagnostic(diag::record_not_automatically_importable,
@@ -3426,7 +3403,7 @@ namespace {
         auto semanticsKind = evaluateOrDefault(
             Impl.SwiftContext.evaluator,
             CxxValueSemantics({parent->getTypeForDecl(), &Impl}), {});
-        if (semanticsKind == CxxValueSemanticsKind::MissingLifetimeOperation)
+        if (semanticsKind == CxxValueSemanticsKind::Unknown)
           return nullptr;
       }
 

lib/ClangImporter/SwiftDeclSynthesizer.cpp

@@ -3074,8 +3074,7 @@ FuncDecl *SwiftDeclSynthesizer::findExplicitDestroy(
       ctx.evaluator, 
       CxxValueSemantics({clangType->getTypeForDecl(), &ImporterImpl}), {});
 
-  if (valueSemanticsKind != CxxValueSemanticsKind::Copyable &&
-      valueSemanticsKind != CxxValueSemanticsKind::MoveOnly)
+  if (valueSemanticsKind == CxxValueSemanticsKind::Unknown)
     return nullptr;
 
   auto cxxRecordSemanticsKind = evaluateOrDefault(

lib/IRGen/GenStruct.cpp

@@ -553,15 +553,7 @@ namespace {
       const auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(ClangDecl);
       if (!cxxRecordDecl)
         return nullptr;
-      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;
+      return importer::findCopyConstructor(cxxRecordDecl);
     }
 
     const clang::CXXConstructorDecl *findMoveConstructor() const {
@@ -619,7 +611,7 @@ namespace {
           /*invocation subs*/ SubstitutionMap(), IGF.IGM.Context);
     }
 
-    void emitCopyWithCopyConstructor(
+    void emitCopyWithCopyOrMoveConstructor(
         IRGenFunction &IGF, SILType T,
         const clang::CXXConstructorDecl *copyConstructor, llvm::Value *src,
         llvm::Value *dest) const {
@@ -629,7 +621,27 @@ 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 =
@@ -809,9 +821,9 @@ namespace {
                             Address srcAddr, SILType T,
                             bool isOutlined) const override {
       if (auto copyConstructor = findCopyConstructor()) {
-        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
-                                    srcAddr.getAddress(),
-                                    destAddr.getAddress());
+        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
+                                          srcAddr.getAddress(),
+                                          destAddr.getAddress());
         return;
       }
       StructTypeInfoBase<AddressOnlyCXXClangRecordTypeInfo, FixedTypeInfo,
@@ -824,9 +836,9 @@ namespace {
                         SILType T, bool isOutlined) const override {
       if (auto copyConstructor = findCopyConstructor()) {
         destroy(IGF, destAddr, T, isOutlined);
-        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
-                                    srcAddr.getAddress(),
-                                    destAddr.getAddress());
+        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
+                                          srcAddr.getAddress(),
+                                          destAddr.getAddress());
         return;
       }
       StructTypeInfoBase<AddressOnlyCXXClangRecordTypeInfo, FixedTypeInfo,
@@ -838,17 +850,15 @@ namespace {
                             SILType T, bool isOutlined,
                             bool zeroizeIfSensitive) const override {
       if (auto moveConstructor = findMoveConstructor()) {
-        emitCopyWithCopyConstructor(IGF, T, moveConstructor,
-                                    src.getAddress(),
-                                    dest.getAddress());
+        emitCopyWithCopyOrMoveConstructor(IGF, T, moveConstructor,
+                                          src.getAddress(), dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }
 
       if (auto copyConstructor = findCopyConstructor()) {
-        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
-                                    src.getAddress(),
-                                    dest.getAddress());
+        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
+                                          src.getAddress(), dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }
@@ -862,18 +872,16 @@ namespace {
                         bool isOutlined) const override {
       if (auto moveConstructor = findMoveConstructor()) {
         destroy(IGF, dest, T, isOutlined);
-        emitCopyWithCopyConstructor(IGF, T, moveConstructor,
-                                    src.getAddress(),
-                                    dest.getAddress());
+        emitCopyWithCopyOrMoveConstructor(IGF, T, moveConstructor,
+                                          src.getAddress(), dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }
 
       if (auto copyConstructor = findCopyConstructor()) {
         destroy(IGF, dest, T, isOutlined);
-        emitCopyWithCopyConstructor(IGF, T, copyConstructor,
-                                    src.getAddress(),
-                                    dest.getAddress());
+        emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
+                                          src.getAddress(), dest.getAddress());
         destroy(IGF, src, T, isOutlined);
         return;
       }