Merge pull request #83402 from egorzhdan/egorzhdan/honor-non-copyable-attr

[cxx-interop] Avoid trying to instantiate copy constructors of explicitly non-copyable structs

Author: egorzhdan

lib/ClangImporter/ClangImporter.cpp

@@ -8009,7 +8009,7 @@ bool importer::hasIteratorAPIAttr(const clang::Decl *decl) {
   return hasSwiftAttribute(decl, "import_iterator");
 }
 
-static bool hasNonCopyableAttr(const clang::RecordDecl *decl) {
+bool importer::hasNonCopyableAttr(const clang::RecordDecl *decl) {
   return hasSwiftAttribute(decl, "~Copyable");
 }
 

lib/ClangImporter/ImportDecl.cpp

@@ -3005,10 +3005,14 @@ namespace {
             clangSema.DefineImplicitDefaultConstructor(clang::SourceLocation(),
                                                        ctor);
         }
+        // If the C++ struct is annotated as non-copyable, we should not try to
+        // instantiate its copy constructor.
+        bool isExplicitlyNonCopyable = hasNonCopyableAttr(decl);
+
         clang::CXXConstructorDecl *copyCtor = nullptr;
         clang::CXXConstructorDecl *moveCtor = nullptr;
         clang::CXXConstructorDecl *defaultCtor = nullptr;
-        if (decl->needsImplicitCopyConstructor()) {
+        if (decl->needsImplicitCopyConstructor() && !isExplicitlyNonCopyable) {
           copyCtor = clangSema.DeclareImplicitCopyConstructor(
               const_cast<clang::CXXRecordDecl *>(decl));
         }
@@ -3031,7 +3035,7 @@ namespace {
                 // that's what "DefineImplicitCopyConstructor" checks.
                 !declCtor->doesThisDeclarationHaveABody()) {
               if (declCtor->isCopyConstructor()) {
-                if (!copyCtor)
+                if (!copyCtor && !isExplicitlyNonCopyable)
                   copyCtor = declCtor;
               } else if (declCtor->isMoveConstructor()) {
                 if (!moveCtor)
@@ -3043,7 +3047,7 @@ namespace {
             }
           }
         }
-        if (copyCtor) {
+        if (copyCtor && !isExplicitlyNonCopyable) {
           clangSema.DefineImplicitCopyConstructor(clang::SourceLocation(),
                                                   copyCtor);
         }

lib/ClangImporter/ImporterImpl.h

@@ -2137,7 +2137,7 @@ bool hasUnsafeAPIAttr(const clang::Decl *decl);
 bool hasIteratorAPIAttr(const clang::Decl *decl);
 
 bool hasNonEscapableAttr(const clang::RecordDecl *decl);
-
+bool hasNonCopyableAttr(const clang::RecordDecl *decl);
 bool hasEscapableAttr(const clang::RecordDecl *decl);
 
 bool isViewType(const clang::CXXRecordDecl *decl);

test/Interop/Cxx/templates/Inputs/module.modulemap

@@ -172,3 +172,8 @@ module ManySpecializations {
   header "many-specializations.h"
   requires cplusplus
 }
+
+module UninstantiatableSpecialMembers {
+  header "uninstantiatable-special-members.h"
+  requires cplusplus
+}

test/Interop/Cxx/templates/Inputs/uninstantiatable-special-members.h

@@ -0,0 +1,27 @@
+template <class T>
+struct __attribute__((swift_attr("~Copyable"))) HasUninstantiatableCopyConstructor {
+  template <class U>
+  void scaryPoison(U u) {
+    U::doesNotExist(u);
+  }
+
+  int value;
+  HasUninstantiatableCopyConstructor(int value) : value(value) {}
+  HasUninstantiatableCopyConstructor(
+      const HasUninstantiatableCopyConstructor &other) {
+    scaryPoison(other);
+  }
+  HasUninstantiatableCopyConstructor(
+      HasUninstantiatableCopyConstructor &&other) = default;
+};
+
+typedef HasUninstantiatableCopyConstructor<int> NonCopyableInst;
+
+template <class T>
+struct __attribute__((swift_attr("~Copyable"))) DerivedUninstantiatableCopyConstructor : HasUninstantiatableCopyConstructor<T> {
+  DerivedUninstantiatableCopyConstructor(int value) : HasUninstantiatableCopyConstructor<T>(value) {}
+  DerivedUninstantiatableCopyConstructor(const DerivedUninstantiatableCopyConstructor &other) = default;
+  DerivedUninstantiatableCopyConstructor(DerivedUninstantiatableCopyConstructor &&other) = default;
+};
+
+typedef DerivedUninstantiatableCopyConstructor<int> DerivedNonCopyableInst;

test/Interop/Cxx/templates/uninstantiatable-special-members-irgen.swift

@@ -0,0 +1,8 @@
+// RUN: %target-swift-emit-irgen %s -cxx-interoperability-mode=default -I %S/Inputs | %FileCheck %s
+
+import UninstantiatableSpecialMembers
+
+let nonCopyableValue = NonCopyableInst(123)
+let nonCopyableDerived = DerivedNonCopyableInst(567)
+
+// CHECK-NOT: scaryPoison

test/Interop/Cxx/templates/uninstantiatable-special-members-module-interface.swift

@@ -0,0 +1,9 @@
+// RUN: %target-swift-ide-test -print-module -module-to-print=UninstantiatableSpecialMembers -I %S/Inputs -source-filename=x -cxx-interoperability-mode=upcoming-swift | %FileCheck %s
+
+// CHECK: struct HasUninstantiatableCopyConstructor<CInt> {
+// CHECK: }
+// CHECK: typealias NonCopyableInst = HasUninstantiatableCopyConstructor<CInt>
+
+// CHECK: struct DerivedUninstantiatableCopyConstructor<CInt> {
+// CHECK: }
+// CHECK: typealias DerivedNonCopyableInst = DerivedUninstantiatableCopyConstructor<CInt>

test/Interop/Cxx/templates/uninstantiatable-special-members-typechecker.swift

@@ -0,0 +1,9 @@
+// RUN: %target-typecheck-verify-swift -cxx-interoperability-mode=default -I %S/Inputs
+
+import UninstantiatableSpecialMembers
+
+let nonCopyableValue = NonCopyableInst(123)
+let copy1 = copy nonCopyableValue // expected-error {{'copy' cannot be applied to noncopyable types}}
+
+let nonCopyableDerived = DerivedNonCopyableInst(567)
+let copy2 = copy nonCopyableDerived // expected-error {{'copy' cannot be applied to noncopyable types}}