diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index 8e39b3e8f84d1..5bf8ec68b0d79 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/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 diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index c5d9e32c259df..13497a3d573b1 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -8342,17 +8342,16 @@ bool importer::isViewType(const clang::CXXRecordDecl *decl) { return !hasOwnedValueAttr(decl) && hasPointerInSubobjects(decl); } -static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) { - if (decl->hasSimpleCopyConstructor()) - return true; - - 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; - }); +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 hasMoveTypeOperations(const clang::CXXRecordDecl *decl) { @@ -8520,8 +8519,8 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, while (!stack.empty()) { const clang::RecordDecl *recordDecl = stack.back(); stack.pop_back(); - - if (!hasNonCopyableAttr(recordDecl)) { + bool isExplicitlyNonCopyable = hasNonCopyableAttr(recordDecl); + if (!isExplicitlyNonCopyable) { auto injectedStlAnnotation = recordDecl->isInStdNamespace() ? STLConditionalParams.find(recordDecl->getName()) @@ -8548,13 +8547,28 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, const auto cxxRecordDecl = dyn_cast(recordDecl); if (!cxxRecordDecl || !recordDecl->isCompleteDefinition()) { - if (hasNonCopyableAttr(recordDecl)) + if (isExplicitlyNonCopyable) return CxxValueSemanticsKind::MoveOnly; continue; } - bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) && - hasCopyTypeOperations(cxxRecordDecl); + bool isCopyable = !isExplicitlyNonCopyable; + if (!isExplicitlyNonCopyable) { + auto copyCtor = findCopyConstructor(cxxRecordDecl); + isCopyable = copyCtor || cxxRecordDecl->needsImplicitCopyConstructor(); + if ((copyCtor && copyCtor->isDefaulted()) || + cxxRecordDecl->needsImplicitCopyConstructor()) { + // If the copy constructor is implicit/defaulted, we ask Clang to + // generate its definition. The implicitly-defined copy constructor + // performs full member-wise copy. Thus, if any member of this type is + // ~Copyable, the type should also be ~Copyable. + for (auto field : cxxRecordDecl->fields()) + maybePushToStack(field->getType()->getUnqualifiedDesugaredType()); + for (auto base : cxxRecordDecl->bases()) + maybePushToStack(base.getType()->getUnqualifiedDesugaredType()); + } + } + bool isMovable = hasMoveTypeOperations(cxxRecordDecl); if (!hasDestroyTypeOperations(cxxRecordDecl) || @@ -8568,7 +8582,7 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, continue; } - if (hasNonCopyableAttr(cxxRecordDecl) && isMovable) + if (isExplicitlyNonCopyable && isMovable) return CxxValueSemanticsKind::MoveOnly; if (isCopyable) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index cfee7b70209d6..1abc81f8c5588 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -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(decl)); } if (decl->needsImplicitMoveConstructor()) { - moveCtor = clangSema.DeclareImplicitMoveConstructor( + clangSema.DeclareImplicitMoveConstructor( const_cast(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(decl)); - clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor); + if (!dtor->isDeleted() && !dtor->isIneligibleOrNotSelected()) + clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor); } } diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp index d870143d153ea..2c789392fe8a0 100644 --- a/lib/IRGen/GenStruct.cpp +++ b/lib/IRGen/GenStruct.cpp @@ -553,15 +553,7 @@ namespace { const auto *cxxRecordDecl = dyn_cast(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(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(copyConstructor)); + else + importer->getClangSema().DefineImplicitMoveConstructor( + clang::SourceLocation(), + const_cast(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 +#include struct NonCopyable { NonCopyable() = default; - NonCopyable(const NonCopyable& other) = delete; + NonCopyable(const NonCopyable& other) = delete; // expected-note {{'NonCopyable' has been explicitly marked deleted here}} NonCopyable(NonCopyable&& other) = default; }; @@ -79,10 +80,63 @@ struct SWIFT_NONCOPYABLE NonCopyableNonMovable { // expected-note {{record 'NonC NonCopyableNonMovable(NonCopyableNonMovable&& other) = delete; }; +struct ImplicitCopyConstructor { + NonCopyable element; +}; + +template +struct TemplatedImplicitCopyConstructor { + T element; + P *pointer; + R &reference; +}; + +using NonCopyableT = TemplatedImplicitCopyConstructor; +using NonCopyableP = TemplatedImplicitCopyConstructor; +using NonCopyableR = TemplatedImplicitCopyConstructor; + +struct DefaultedCopyConstructor { + NonCopyable element; // expected-note {{copy constructor of 'DefaultedCopyConstructor' is implicitly deleted because field 'element' has a deleted copy constructor}} + DefaultedCopyConstructor(const DefaultedCopyConstructor&) = default; + // expected-warning@-1 {{explicitly defaulted copy constructor is implicitly deleted}} + // expected-note@-2 {{replace 'default' with 'delete'}} + DefaultedCopyConstructor(DefaultedCopyConstructor&&) = default; +}; + +template +struct TemplatedDefaultedCopyConstructor { + T element; + TemplatedDefaultedCopyConstructor(const TemplatedDefaultedCopyConstructor&) = default; + TemplatedDefaultedCopyConstructor(TemplatedDefaultedCopyConstructor&&) = default; +}; + +template +struct DerivedTemplatedDefaultedCopyConstructor : TemplatedDefaultedCopyConstructor {}; + +using CopyableDefaultedCopyConstructor = TemplatedDefaultedCopyConstructor; +using NonCopyableDefaultedCopyConstructor = TemplatedDefaultedCopyConstructor; +using CopyableDerived = DerivedTemplatedDefaultedCopyConstructor; +using NonCopyableDerived = DerivedTemplatedDefaultedCopyConstructor; + template struct SWIFT_COPYABLE_IF(T) DisposableContainer {}; struct POD { int x; float y; }; // special members are implicit, but should be copyable using DisposablePOD = DisposableContainer; // should also be copyable +struct DerivesFromMe : MyPair, std::vector> {}; +struct DerivesFromMeToo : MyPair, DisposableContainer> {}; + +template +struct OneField { + T field; +}; + +template +struct SWIFT_COPYABLE_IF(F, S) NoFields {}; + +struct FieldDependsOnMe { // used to trigger a request cycle + OneField> field; +}; + //--- test.swift import Test import CxxStdlib @@ -133,6 +187,34 @@ func missingLifetimeOperation() { takeCopyable(s) } +func implicitCopyConstructor(i: borrowing ImplicitCopyConstructor, t: borrowing NonCopyableT, p: borrowing NonCopyableP, r: borrowing NonCopyableR) { + takeCopyable(i) // expected-error {{global function 'takeCopyable' requires that 'ImplicitCopyConstructor' conform to 'Copyable'}} + takeCopyable(t) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableT' (aka 'TemplatedImplicitCopyConstructor') conform to 'Copyable'}} + + // References and pointers to non-copyable types are still copyable + takeCopyable(p) + takeCopyable(r) +} + +func defaultCopyConstructor(d: borrowing DefaultedCopyConstructor, d1: borrowing CopyableDefaultedCopyConstructor, d2: borrowing NonCopyableDefaultedCopyConstructor, d3: borrowing CopyableDerived, d4: borrowing NonCopyableDerived) { + takeCopyable(d) // expected-error {{global function 'takeCopyable' requires that 'DefaultedCopyConstructor' conform to 'Copyable'}} + takeCopyable(d1) + takeCopyable(d2) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableDefaultedCopyConstructor' (aka 'TemplatedDefaultedCopyConstructor') conform to 'Copyable'}} + takeCopyable(d3) + takeCopyable(d4) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableDerived' (aka 'DerivedTemplatedDefaultedCopyConstructor') conform to 'Copyable'}} +} + func copyableDisposablePOD(p: DisposablePOD) { takeCopyable(p) } + +func couldCreateCycleOfCxxValueSemanticsRequests() { + let d1 = DerivesFromMe() + takeCopyable(d1) // expected-error {{global function 'takeCopyable' requires that 'DerivesFromMe' conform to 'Copyable'}} + + let d2 = DerivesFromMeToo() + takeCopyable(d2) // expected-error {{global function 'takeCopyable' requires that 'DerivesFromMeToo' conform to 'Copyable'}} + + let d3 = FieldDependsOnMe() + takeCopyable(d3) // expected-error {{global function 'takeCopyable' requires that 'FieldDependsOnMe' conform to 'Copyable'}} +} diff --git a/test/Interop/Cxx/stdlib/Inputs/module.modulemap b/test/Interop/Cxx/stdlib/Inputs/module.modulemap index fb60a00714352..4c5343bc2c8a9 100644 --- a/test/Interop/Cxx/stdlib/Inputs/module.modulemap +++ b/test/Interop/Cxx/stdlib/Inputs/module.modulemap @@ -99,3 +99,9 @@ module CustomSmartPtr { requires cplusplus export * } + +module StdExpected { + header "std-expected.h" + requires cplusplus + export * +} diff --git a/test/Interop/Cxx/stdlib/Inputs/std-expected.h b/test/Interop/Cxx/stdlib/Inputs/std-expected.h new file mode 100644 index 0000000000000..eb760baa78227 --- /dev/null +++ b/test/Interop/Cxx/stdlib/Inputs/std-expected.h @@ -0,0 +1,23 @@ +#ifndef TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H +#define TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H + +#include +#include + +using NonCopyableExpected = std::expected, int>; + +template +class UniqueRef { +public: + std::unique_ptr _field; +}; + +struct Decoder {}; +enum Error { + DoomA, + DoomB +}; + +using DecoderOrError = std::expected, Error>; + +#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H diff --git a/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h b/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h index e6cc6bf0d5ca0..a307b569674f4 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h @@ -89,4 +89,9 @@ struct CountCopies { inline std::unique_ptr getCopyCountedUniquePtr() { return std::make_unique(); } +struct HasUniqueIntVector { + HasUniqueIntVector() = default; + std::vector> x; +}; + #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H diff --git a/test/Interop/Cxx/stdlib/Inputs/std-vector.h b/test/Interop/Cxx/stdlib/Inputs/std-vector.h index 4d70fec7c78ad..403d883f86b82 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-vector.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-vector.h @@ -40,4 +40,10 @@ struct NonCopyable { using VectorOfNonCopyable = std::vector; using VectorOfPointer = std::vector; +struct HasVector { + std::vector field; +}; + +struct BaseHasVector : HasVector {}; + #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_VECTOR_H diff --git a/test/Interop/Cxx/stdlib/use-std-expected-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-expected-typechecker.swift new file mode 100644 index 0000000000000..1c5fe02bf76e0 --- /dev/null +++ b/test/Interop/Cxx/stdlib/use-std-expected-typechecker.swift @@ -0,0 +1,22 @@ +// RUN: not %target-swift-frontend %s -typecheck -I %S/Inputs -cxx-interoperability-mode=default -Xcc -std=c++23 -diagnostic-style llvm 2>&1 | %FileCheck %s + +// TODO not yet supported with libstdc++ +// XFAIL: OS=linux-gnu + +// https://github.com/apple/swift/issues/70226 +// UNSUPPORTED: OS=windows-msvc + +import StdExpected +import CxxStdlib + +func takeCopyable(_ x: T) {} + +let nonCopExpected = NonCopyableExpected() +takeCopyable(nonCopExpected) +// CHECK: error: global function 'takeCopyable' requires that 'NonCopyableExpected' (aka {{.*}}) conform to 'Copyable' + +let doe = DecoderOrError() +takeCopyable(doe) +// CHECK: error: global function 'takeCopyable' requires that 'DecoderOrError' (aka {{.*}}) conform to 'Copyable' + +// CHECK-NOT: error diff --git a/test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift index 636ba05653689..e0b0e34b2ea50 100644 --- a/test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift +++ b/test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift @@ -10,3 +10,7 @@ func takeCopyable(_ x: T) {} let vecUniquePtr = getVectorNonCopyableUniquePtr() takeCopyable(vecUniquePtr) // CHECK: error: global function 'takeCopyable' requires that 'std{{.*}}vector{{.*}}unique_ptr{{.*}}NonCopyable{{.*}}' conform to 'Copyable' + +let uniqueIntVec = HasUniqueIntVector() +takeCopyable(uniqueIntVec) +// CHECK: error: global function 'takeCopyable' requires that 'HasUniqueIntVector' conform to 'Copyable' diff --git a/test/Interop/Cxx/stdlib/use-std-vector-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-vector-typechecker.swift index debc99acc2a2b..010d87e4ad20a 100644 --- a/test/Interop/Cxx/stdlib/use-std-vector-typechecker.swift +++ b/test/Interop/Cxx/stdlib/use-std-vector-typechecker.swift @@ -29,3 +29,11 @@ let vecFloat = VectorOfFloat() takeCopyable(vecFloat) takeCxxVector(vecFloat) // CHECK-NOT: error + +let hasVector = HasVector() +takeCopyable(hasVector) +// CHECK: error: global function 'takeCopyable' requires that 'HasVector' conform to 'Copyable' + +let baseHasVector = BaseHasVector() +takeCopyable(baseHasVector) +// CHECK: error: global function 'takeCopyable' requires that 'BaseHasVector' conform to 'Copyable'