diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index 5fe44dc8a7afe..2c6dcceb6662a 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/include/swift/ClangImporter/ClangImporter.h @@ -733,12 +733,6 @@ 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 758ca73a84259..6694955b31392 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -8322,39 +8322,16 @@ bool importer::isViewType(const clang::CXXRecordDecl *decl) { return !hasOwnedValueAttr(decl) && hasPointerInSubobjects(decl); } -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 hasCopyTypeOperations(const clang::CXXRecordDecl *decl, - ClangImporter::Implementation *importerImpl) { - if (!decl->hasSimpleCopyConstructor()) { - auto *copyCtor = findCopyConstructor(decl); - if (!copyCtor) - return false; - - if (!copyCtor->isDefaulted()) - return true; - } +static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) { + if (decl->hasSimpleCopyConstructor()) + return true; - // If the copy constructor is defaulted or implicitly declared, we should only - // import the type as copyable if all its fields are also copyable - // FIXME: we should also look at the bases - return llvm::none_of(decl->fields(), [&](clang::FieldDecl *field) { - if (const auto *rd = field->getType()->getAsRecordDecl()) { - return (!field->getType()->isReferenceType() && - !field->getType()->isPointerType() && - recordHasMoveOnlySemantics(rd, importerImpl)); - } - return false; + 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; }); } @@ -8553,8 +8530,8 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, return CxxValueSemanticsKind::Copyable; } - bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) && - hasCopyTypeOperations(cxxRecordDecl, importerImpl); + bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) && + hasCopyTypeOperations(cxxRecordDecl); bool isMovable = hasMoveTypeOperations(cxxRecordDecl); if (!hasDestroyTypeOperations(cxxRecordDecl) || (!isCopyable && !isMovable)) { diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 406a91d370633..3a5312f4ce622 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -186,15 +186,6 @@ bool importer::recordHasReferenceSemantics( return semanticsKind == CxxRecordSemanticsKind::Reference; } -bool importer::recordHasMoveOnlySemantics( - const clang::RecordDecl *decl, - ClangImporter::Implementation *importerImpl) { - auto semanticsKind = evaluateOrDefault( - importerImpl->SwiftContext.evaluator, - CxxValueSemantics({decl->getTypeForDecl(), importerImpl}), {}); - return semanticsKind == CxxValueSemanticsKind::MoveOnly; -} - bool importer::hasImmortalAttrs(const clang::RecordDecl *decl) { return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) { if (auto swiftAttr = dyn_cast(attr)) @@ -2104,7 +2095,10 @@ namespace { } bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl) { - return importer::recordHasMoveOnlySemantics(decl, &Impl); + auto semanticsKind = evaluateOrDefault( + Impl.SwiftContext.evaluator, + CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {}); + return semanticsKind == CxxValueSemanticsKind::MoveOnly; } void markReturnsUnsafeNonescapable(AbstractFunctionDecl *fd) { @@ -2283,9 +2277,13 @@ namespace { loc, ArrayRef(), nullptr, dc); Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result; - if (decl->isInStdNamespace() && decl->getName() == "promise" && recordHasMoveOnlySemantics(decl)) { - // Do not import std::promise. - return nullptr; + if (recordHasMoveOnlySemantics(decl)) { + if (decl->isInStdNamespace() && decl->getName() == "promise") { + // Do not import std::promise. + return nullptr; + } + result->addAttribute(new (Impl.SwiftContext) + MoveOnlyAttr(/*Implicit=*/true)); } // FIXME: Figure out what to do with superclasses in C++. One possible @@ -2532,11 +2530,6 @@ namespace { cxxRecordDecl->ctors().empty()); } - if (recordHasMoveOnlySemantics(decl)) { - result->getAttrs().add(new (Impl.SwiftContext) - MoveOnlyAttr(/*Implicit=*/true)); - } - // TODO: builtin "zeroInitializer" does not work with non-escapable // types yet. Don't generate an initializer. if (hasZeroInitializableStorage && needsEmptyInitializer && @@ -3132,13 +3125,15 @@ 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) { - clangSema.DeclareImplicitCopyConstructor( + copyCtor = clangSema.DeclareImplicitCopyConstructor( const_cast(decl)); } if (decl->needsImplicitMoveConstructor()) { - clangSema.DeclareImplicitMoveConstructor( + moveCtor = clangSema.DeclareImplicitMoveConstructor( const_cast(decl)); } if (decl->needsImplicitDefaultConstructor()) { @@ -3155,13 +3150,28 @@ namespace { // Note: we use "doesThisDeclarationHaveABody" here because // that's what "DefineImplicitCopyConstructor" checks. !declCtor->doesThisDeclarationHaveABody()) { - if (declCtor->isDefaultConstructor()) { + if (declCtor->isCopyConstructor()) { + if (!copyCtor && !isExplicitlyNonCopyable) + copyCtor = declCtor; + } else if (declCtor->isMoveConstructor()) { + if (!moveCtor) + moveCtor = declCtor; + } else 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); @@ -3170,8 +3180,7 @@ namespace { if (decl->needsImplicitDestructor()) { auto dtor = clangSema.DeclareImplicitDestructor( const_cast(decl)); - if (!dtor->isDeleted() && !dtor->isIneligibleOrNotSelected()) - clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor); + clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor); } } diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index 82d24be75bbf3..e17f4d8d20d6e 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -1974,11 +1974,6 @@ namespace importer { bool recordHasReferenceSemantics(const clang::RecordDecl *decl, ClangImporter::Implementation *importerImpl); -/// Returns true if the given C/C++ record should be imported as non-copyable into -/// Swift. -bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl, - ClangImporter::Implementation *importerImpl); - /// Whether this is a forward declaration of a type. We ignore forward /// declarations in certain cases, and instead process the real declarations. bool isForwardDeclOfType(const clang::Decl *decl); diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp index 2c789392fe8a0..d870143d153ea 100644 --- a/lib/IRGen/GenStruct.cpp +++ b/lib/IRGen/GenStruct.cpp @@ -553,7 +553,15 @@ namespace { const auto *cxxRecordDecl = dyn_cast(ClangDecl); if (!cxxRecordDecl) return nullptr; - return importer::findCopyConstructor(cxxRecordDecl); + 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; } const clang::CXXConstructorDecl *findMoveConstructor() const { @@ -611,7 +619,7 @@ namespace { /*invocation subs*/ SubstitutionMap(), IGF.IGM.Context); } - void emitCopyWithCopyOrMoveConstructor( + void emitCopyWithCopyConstructor( IRGenFunction &IGF, SILType T, const clang::CXXConstructorDecl *copyConstructor, llvm::Value *src, llvm::Value *dest) const { @@ -621,27 +629,7 @@ 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 = @@ -821,9 +809,9 @@ namespace { Address srcAddr, SILType T, bool isOutlined) const override { if (auto copyConstructor = findCopyConstructor()) { - emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor, - srcAddr.getAddress(), - destAddr.getAddress()); + emitCopyWithCopyConstructor(IGF, T, copyConstructor, + srcAddr.getAddress(), + destAddr.getAddress()); return; } StructTypeInfoBase -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 @@ -171,23 +133,6 @@ 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) } diff --git a/test/Interop/Cxx/stdlib/Inputs/module.modulemap b/test/Interop/Cxx/stdlib/Inputs/module.modulemap index 4c5343bc2c8a9..fb60a00714352 100644 --- a/test/Interop/Cxx/stdlib/Inputs/module.modulemap +++ b/test/Interop/Cxx/stdlib/Inputs/module.modulemap @@ -99,9 +99,3 @@ 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 deleted file mode 100644 index eb760baa78227..0000000000000 --- a/test/Interop/Cxx/stdlib/Inputs/std-expected.h +++ /dev/null @@ -1,23 +0,0 @@ -#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 a307b569674f4..e6cc6bf0d5ca0 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h @@ -89,9 +89,4 @@ 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/use-std-expected-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-expected-typechecker.swift deleted file mode 100644 index fbad78df61af4..0000000000000 --- a/test/Interop/Cxx/stdlib/use-std-expected-typechecker.swift +++ /dev/null @@ -1,24 +0,0 @@ -// 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++ - -// rdar://164027738 -// UNSUPPORTED: 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 51e3945c7d854..636ba05653689 100644 --- a/test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift +++ b/test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift @@ -10,9 +10,3 @@ 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' - -