diff --git a/include/swift/AST/DiagnosticsIRGen.def b/include/swift/AST/DiagnosticsIRGen.def index 4f93def59a63f..e01cd868d5827 100644 --- a/include/swift/AST/DiagnosticsIRGen.def +++ b/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()' in C++ to specify " + "that the type is Copyable if 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" diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp index 64465238062ae..6816689777e03 100644 --- a/lib/IRGen/GenStruct.cpp +++ b/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(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(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(clangFnAddr->stripPointerCasts()); Signature signature = IGF.IGM.getSignature(fnType, copyConstructor); std::string name = "__swift_cxx_copy_ctor" + callee->getName().str(); diff --git a/test/Interop/Cxx/class/noncopyable-irgen.swift b/test/Interop/Cxx/class/noncopyable-irgen.swift new file mode 100644 index 0000000000000..a4928a93b2ae3 --- /dev/null +++ b/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%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}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%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}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%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}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%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}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 + +struct NonCopyable { + NonCopyable() = default; + NonCopyable(const NonCopyable& other) = delete; // expected-note {{'NonCopyable' has been explicitly marked deleted here}} + NonCopyable(NonCopyable&& other) = default; +}; + +template +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::OwnsT' requested here}} + // expected-TEST1-error@-3 {{failed to copy 'OwnsT'; did you mean to import 'OwnsT' 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()' in C++ to specify that the type is Copyable if is Copyable}} + OwnsT(OwnsT&& other) {} +}; + +using OwnsNonCopyable = OwnsT; + +template struct Derived : OwnsT { + // expected-TEST2-error@-1 {{failed to copy 'Derived'; did you mean to import 'Derived' 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()' in C++ to specify that the type is Copyable if is Copyable}} + // expected-TEST2-note@-4 {{annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable}} +}; + +using DerivedNonCopyable = Derived; + +template 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>'; did you mean to import 'Annotated>' as ~Copyable?}} + // expected-TEST3-note@-2 {{one of the types that 'Annotated>' 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>' may be missing a parameter}} + + Annotated(Annotated &&) = default; +}; + +using AnnotatedOwnsNonCopyable = Annotated>; + +#if __cplusplus >= 202002L +template struct Requires { + T element; + Requires() : element() {} + Requires(const Requires &other) requires std::is_copy_constructible_v : element(other.element) {} + // expected-TEST4-error@-1 {{failed to copy 'Requires>'; did you mean to import 'Requires>' as ~Copyable?}} + // expected-TEST4-note@-2 {{one of the types that 'Requires>' 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>' may be missing a constraint}} + + Requires(Requires &&) = default; +}; + +using RequiresOwnsNonCopyable = Requires>; +#endif + +//--- test.swift +import Test +import CxxStdlib + +func takeCopyable(_ 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 as Copyable (even though it cannot be copy constructible), + // we end up also importing Annotated> 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 as Copyable (even though it cannot be copy constructible), + // we end up also importing Requires> as Copyable + takeCopyable(s) +} + +#endif