[cxx-interop] Emit diagnostic to mark C++ type as non-copyable

include/swift/AST/DiagnosticsIRGen.def

@@ -80,5 +80,37 @@ ERROR(attr_objc_implementation_resilient_property_deployment_target, none,
 ERROR(unable_to_load_pass_plugin,none,
       "unable to load plugin '%0': '%1'", (StringRef, StringRef))
 
+ERROR(failed_emit_copy, none,
+      "failed to copy %0; did you mean to import %0 as ~Copyable?",
+      (const clang::NamedDecl *))
+
+NOTE(use_requires_expression, none,
+     "use 'requires' (since C++20) to specify the constraints under which the "
+     "copy "
+     "constructor is available",
+     ())
+
+NOTE(annotate_copyable_if, none,
+     "annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify "
+     "that the type is Copyable if <T> is Copyable",
+     ())
+
+NOTE(annotate_non_copyable, none,
+     "annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as "
+     "~Copyable",
+     ())
+
+NOTE(maybe_missing_annotation, none,
+     "one of the types that %0 depends on may need a 'requires' clause (since "
+     "C++20) in the copy constructor, a 'SWIFT_COPYABLE_IF' annotation or a "
+     "'SWIFT_NONCOPYABLE' annotation'",
+     (const clang::NamedDecl *))
+
+NOTE(maybe_missing_parameter, none,
+     "the %select{'requires' clause on the copy constructor "
+     "of|'SWIFT_COPYABLE_IF' annotation on}0 %1 may be missing a "
+     "%select{constraint|parameter}0",
+     (bool, const clang::NamedDecl *))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

lib/IRGen/GenStruct.cpp

@@ -27,6 +27,7 @@
 #include "swift/AST/SubstitutionMap.h"
 #include "swift/AST/Types.h"
 #include "swift/Basic/Assertions.h"
+#include "swift/ClangImporter/ClangImporter.h"
 #include "swift/IRGen/Linking.h"
 #include "swift/SIL/SILFunctionBuilder.h"
 #include "swift/SIL/SILModule.h"
@@ -556,7 +557,8 @@ namespace {
         if (ctor->isCopyConstructor() &&
             // FIXME: Support default arguments (rdar://142414553)
             ctor->getNumParams() == 1 &&
-            ctor->getAccess() == clang::AS_public && !ctor->isDeleted())
+            ctor->getAccess() == clang::AS_public && !ctor->isDeleted() &&
+            !ctor->isIneligibleOrNotSelected())
           return ctor;
       }
       return nullptr;
@@ -570,7 +572,8 @@ namespace {
         if (ctor->isMoveConstructor() &&
             // FIXME: Support default arguments (rdar://142414553)
             ctor->getNumParams() == 1 &&
-            ctor->getAccess() == clang::AS_public && !ctor->isDeleted())
+            ctor->getAccess() == clang::AS_public && !ctor->isDeleted() &&
+            !ctor->isIneligibleOrNotSelected())
           return ctor;
       }
       return nullptr;
@@ -623,8 +626,54 @@ namespace {
       auto fnType = createCXXCopyConstructorFunctionType(IGF, T);
       auto globalDecl =
           clang::GlobalDecl(copyConstructor, clang::Ctor_Complete);
+
+      auto &ctx = IGF.IGM.Context;
+      auto *importer = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
+
+      auto &diagEngine = importer->getClangSema().getDiagnostics();
+      clang::DiagnosticErrorTrap trap(diagEngine);
       auto clangFnAddr =
           IGF.IGM.getAddrOfClangGlobalDecl(globalDecl, NotForDefinition);
+
+      if (trap.hasErrorOccurred()) {
+        SourceLoc copyConstructorLoc =
+            importer->importSourceLocation(copyConstructor->getLocation());
+        auto *recordDecl = copyConstructor->getParent();
+        ctx.Diags.diagnose(copyConstructorLoc, diag::failed_emit_copy,
+                           recordDecl);
+
+        bool hasCopyableIfAttr =
+            recordDecl->hasAttrs() &&
+            llvm::any_of(recordDecl->getAttrs(), [&](clang::Attr *attr) {
+              if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
+                StringRef attrStr = swiftAttr->getAttribute();
+                assert(!attrStr.starts_with("~Copyable") &&
+                       "Trying to emit copy of a type annotated with "
+                       "'SWIFT_NONCOPYABLE'?");
+                if (attrStr.starts_with("copyable_if:"))
+                  return true;
+              }
+              return false;
+            });
+
+        bool hasRequiresClause =
+            copyConstructor->getTrailingRequiresClause() != nullptr;
+
+        if (hasRequiresClause || hasCopyableIfAttr) {
+          ctx.Diags.diagnose(copyConstructorLoc, diag::maybe_missing_annotation,
+                             recordDecl);
+          ctx.Diags.diagnose(copyConstructorLoc, diag::maybe_missing_parameter,
+                             hasCopyableIfAttr, recordDecl);
+        } else {
+          ctx.Diags.diagnose(copyConstructorLoc, diag::use_requires_expression);
+          ctx.Diags.diagnose(copyConstructorLoc, diag::annotate_copyable_if);
+        }
+
+        if (!copyConstructor->isUserProvided()) {
+          ctx.Diags.diagnose(copyConstructorLoc, diag::annotate_non_copyable);
+        }
+      }
+
       auto callee = cast<llvm::Function>(clangFnAddr->stripPointerCasts());
       Signature signature = IGF.IGM.getSignature(fnType, copyConstructor);
       std::string name = "__swift_cxx_copy_ctor" + callee->getName().str();

test/Interop/Cxx/class/noncopyable-irgen.swift

@@ -0,0 +1,112 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t/Inputs/noncopyable.h -verify-additional-prefix TEST1- -D TEST1
+// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t/Inputs/noncopyable.h -verify-additional-prefix TEST2- -D TEST2
+// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t/Inputs/noncopyable.h -verify-additional-prefix TEST3- -D TEST3
+// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t/Inputs/noncopyable.h -verify-additional-prefix TEST4- -D TEST4 -Xcc -std=c++20
+
+//--- Inputs/module.modulemap
+module Test {
+    header "noncopyable.h"
+    requires cplusplus
+}
+
+//--- Inputs/noncopyable.h
+#include "swift/bridging"
+#include <string>
+
+struct NonCopyable {
+    NonCopyable() = default;
+    NonCopyable(const NonCopyable& other) = delete; // expected-note {{'NonCopyable' has been explicitly marked deleted here}}
+    NonCopyable(NonCopyable&& other) = default;
+};
+
+template <typename T>
+struct OwnsT {
+    T element;
+    OwnsT() {}
+    OwnsT(const OwnsT &other) : element(other.element) {} 
+    // expected-error@-1 *{{call to deleted constructor of 'NonCopyable'}}
+    // expected-note@-2 *{{in instantiation of member function 'OwnsT<NonCopyable>::OwnsT' requested here}}
+    // expected-TEST1-error@-3 {{failed to copy 'OwnsT<NonCopyable>'; did you mean to import 'OwnsT<NonCopyable>' as ~Copyable?}}
+    // expected-TEST1-note@-4 {{use 'requires' (since C++20) to specify the constraints under which the copy constructor is available}}
+    // expected-TEST1-note@-5 {{annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify that the type is Copyable if <T> is Copyable}}
+    OwnsT(OwnsT&& other) {}
+};
+
+using OwnsNonCopyable = OwnsT<NonCopyable>;
+
+template <typename T> struct Derived : OwnsT<T> {
+    // expected-TEST2-error@-1 {{failed to copy 'Derived<NonCopyable>'; did you mean to import 'Derived<NonCopyable>' as ~Copyable?}}
+    // expected-TEST2-note@-2 {{use 'requires' (since C++20) to specify the constraints under which the copy constructor is available}}
+    // expected-TEST2-note@-3 {{annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify that the type is Copyable if <T> is Copyable}}
+    // expected-TEST2-note@-4 {{annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable}}
+};
+
+using DerivedNonCopyable = Derived<NonCopyable>;
+
+template <typename T> struct SWIFT_COPYABLE_IF(T) Annotated {
+    T element;
+    Annotated() : element() {}
+    Annotated(const Annotated &other) : element(other.element) {}
+    // expected-TEST3-error@-1 {{failed to copy 'Annotated<OwnsT<NonCopyable>>'; did you mean to import 'Annotated<OwnsT<NonCopyable>>' as ~Copyable?}}
+    // expected-TEST3-note@-2 {{one of the types that 'Annotated<OwnsT<NonCopyable>>' depends on may need a 'requires' clause (since C++20) in the copy constructor, a 'SWIFT_COPYABLE_IF' annotation or a 'SWIFT_NONCOPYABLE' annotation'}}
+    // expected-TEST3-note@-3 {{the 'SWIFT_COPYABLE_IF' annotation on 'Annotated<OwnsT<NonCopyable>>' may be missing a parameter}}
+
+    Annotated(Annotated &&) = default;
+};
+
+using AnnotatedOwnsNonCopyable = Annotated<OwnsT<NonCopyable>>;
+
+#if __cplusplus >= 202002L
+template <typename T> struct Requires {
+    T element;
+    Requires() : element() {}
+    Requires(const Requires &other) requires std::is_copy_constructible_v<T> : element(other.element) {}
+    // expected-TEST4-error@-1 {{failed to copy 'Requires<OwnsT<NonCopyable>>'; did you mean to import 'Requires<OwnsT<NonCopyable>>' as ~Copyable?}}
+    // expected-TEST4-note@-2 {{one of the types that 'Requires<OwnsT<NonCopyable>>' depends on may need a 'requires' clause (since C++20) in the copy constructor, a 'SWIFT_COPYABLE_IF' annotation or a 'SWIFT_NONCOPYABLE' annotation'}}
+    // expected-TEST4-note@-3 {{the 'requires' clause on the copy constructor of 'Requires<OwnsT<NonCopyable>>' may be missing a constraint}}
+
+    Requires(Requires &&) = default;
+};
+
+using RequiresOwnsNonCopyable = Requires<OwnsT<NonCopyable>>;
+#endif
+
+//--- test.swift
+import Test
+import CxxStdlib
+
+func takeCopyable<T: Copyable>(_ x: T) {}
+
+#if TEST1
+func simpleTest() {
+    let s = OwnsNonCopyable()
+    takeCopyable(s) 
+}
+
+#elseif TEST2
+func derived() {
+    let s = DerivedNonCopyable()
+    takeCopyable(s) 
+}
+
+#elseif TEST3
+func annotated() {
+    let s = AnnotatedOwnsNonCopyable()
+    // Annotated has a correct SWIFT_COPYABLE_IF annotation, but OwnsT does not
+    // Since we import OwnsT<NonCopyable> as Copyable (even though it cannot be copy constructible), 
+    // we end up also importing Annotated<OwnsT<NonCopyable>> as Copyable
+    takeCopyable(s)
+}
+
+#elseif TEST4
+func requires() {
+    let s = RequiresOwnsNonCopyable()
+    // Requires makes use of 'requires' correctly, but OwnsT is missing some information
+    // Since we import OwnsT<NonCopyable> as Copyable (even though it cannot be copy constructible), 
+    // we end up also importing Requires<OwnsT<NonCopyable>> as Copyable
+    takeCopyable(s)
+}
+
+#endif