averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #85642 from susmonteiro/susmonteiro/reapply-implicit-constructors

Browse Source 
[cxx-interop] Reapply implicitly defined copy and move constructors
This commit is contained in:
Susana Monteiro
2025-12-02 14:23:00 +00:00
committed by GitHub
parent ce082bcc83 fbfcd4d241
commit fd6e6b35a6
12 changed files with 238 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
include/swift/ClangImporter/ClangImporter.h
View File

@@ -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

48
lib/ClangImporter/ClangImporter.cpp
View File

@@ -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<clang::CXXRecordDecl>(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)

26
lib/ClangImporter/ImportDecl.cpp
View File

@@ -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<clang::CXXRecordDecl *>(decl));
}
if (decl->needsImplicitMoveConstructor()) {
moveCtor = clangSema.DeclareImplicitMoveConstructor(
clangSema.DeclareImplicitMoveConstructor(
const_cast<clang::CXXRecordDecl *>(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<clang::CXXRecordDecl *>(decl));
clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor);
if (!dtor->isDeleted() && !dtor->isIneligibleOrNotSelected())
clangSema.DefineImplicitDestructor(clang::SourceLocation(), dtor);
}
}

66
lib/IRGen/GenStruct.cpp
View File

@@ -553,15 +553,7 @@ namespace {
const auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(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<ClangImporter *>(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<clang::CXXConstructorDecl *>(copyConstructor));
else
importer->getClangSema().DefineImplicitMoveConstructor(
clang::SourceLocation(),
const_cast<clang::CXXConstructorDecl *>(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<AddressOnlyCXXClangRecordTypeInfo, FixedTypeInfo,
@@ -824,9 +836,9 @@ namespace {
SILType T, bool isOutlined) const override {
if (auto copyConstructor = findCopyConstructor()) {
destroy(IGF, destAddr, T, isOutlined);
emitCopyWithCopyConstructor(IGF, T, copyConstructor,
srcAddr.getAddress(),
destAddr.getAddress());
emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
srcAddr.getAddress(),
destAddr.getAddress());
return;
}
StructTypeInfoBase<AddressOnlyCXXClangRecordTypeInfo, FixedTypeInfo,
@@ -838,17 +850,15 @@ namespace {
SILType T, bool isOutlined,
bool zeroizeIfSensitive) const override {
if (auto moveConstructor = findMoveConstructor()) {
emitCopyWithCopyConstructor(IGF, T, moveConstructor,
src.getAddress(),
dest.getAddress());
emitCopyWithCopyOrMoveConstructor(IGF, T, moveConstructor,
src.getAddress(), dest.getAddress());
destroy(IGF, src, T, isOutlined);
return;
}
if (auto copyConstructor = findCopyConstructor()) {
emitCopyWithCopyConstructor(IGF, T, copyConstructor,
src.getAddress(),
dest.getAddress());
emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
src.getAddress(), dest.getAddress());
destroy(IGF, src, T, isOutlined);
return;
}
@@ -862,18 +872,16 @@ namespace {
bool isOutlined) const override {
if (auto moveConstructor = findMoveConstructor()) {
destroy(IGF, dest, T, isOutlined);
emitCopyWithCopyConstructor(IGF, T, moveConstructor,
src.getAddress(),
dest.getAddress());
emitCopyWithCopyOrMoveConstructor(IGF, T, moveConstructor,
src.getAddress(), dest.getAddress());
destroy(IGF, src, T, isOutlined);
return;
}
if (auto copyConstructor = findCopyConstructor()) {
destroy(IGF, dest, T, isOutlined);
emitCopyWithCopyConstructor(IGF, T, copyConstructor,
src.getAddress(),
dest.getAddress());
emitCopyWithCopyOrMoveConstructor(IGF, T, copyConstructor,
src.getAddress(), dest.getAddress());
destroy(IGF, src, T, isOutlined);
return;
}

88
test/Interop/Cxx/class/noncopyable-typechecker.swift
View File

@@ -1,7 +1,7 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -Xcc -std=c++20 -verify-additional-prefix cpp20- -D CPP20 -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-ignore-unrelated
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -Xcc -std=c++20 -verify-additional-prefix cpp20- -D CPP20 -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-ignore-unrelated
//--- Inputs/module.modulemap
module Test {
@@ -12,10 +12,11 @@ module Test {
//--- Inputs/noncopyable.h
#include "swift/bridging"
#include <string>
#include <vector>
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 <typename T, typename P, typename R>
struct TemplatedImplicitCopyConstructor {
T element;
P *pointer;
R &reference;
};
using NonCopyableT = TemplatedImplicitCopyConstructor<NonCopyable, int, int>;
using NonCopyableP = TemplatedImplicitCopyConstructor<int, NonCopyable, int>;
using NonCopyableR = TemplatedImplicitCopyConstructor<int, int, NonCopyable>;
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<typename T>
struct TemplatedDefaultedCopyConstructor {
T element;
TemplatedDefaultedCopyConstructor(const TemplatedDefaultedCopyConstructor&) = default;
TemplatedDefaultedCopyConstructor(TemplatedDefaultedCopyConstructor&&) = default;
};
template<typename T>
struct DerivedTemplatedDefaultedCopyConstructor : TemplatedDefaultedCopyConstructor<T> {};
using CopyableDefaultedCopyConstructor = TemplatedDefaultedCopyConstructor<NonCopyableP>;
using NonCopyableDefaultedCopyConstructor = TemplatedDefaultedCopyConstructor<NonCopyable>;
using CopyableDerived = DerivedTemplatedDefaultedCopyConstructor<NonCopyableR>;
using NonCopyableDerived = DerivedTemplatedDefaultedCopyConstructor<NonCopyable>;
template<typename T> struct SWIFT_COPYABLE_IF(T) DisposableContainer {};
struct POD { int x; float y; }; // special members are implicit, but should be copyable
using DisposablePOD = DisposableContainer<POD>; // should also be copyable
struct DerivesFromMe : MyPair<DisposableContainer<DerivesFromMe>, std::vector<NonCopyable>> {};
struct DerivesFromMeToo : MyPair<std::vector<NonCopyable>, DisposableContainer<DerivesFromMe>> {};
template <typename T>
struct OneField {
T field;
};
template <typename F, typename S>
struct SWIFT_COPYABLE_IF(F, S) NoFields {};
struct FieldDependsOnMe { // used to trigger a request cycle
OneField<NoFields<FieldDependsOnMe, NonCopyable>> 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<NonCopyable, CInt, CInt>') 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<NonCopyable>') conform to 'Copyable'}}
takeCopyable(d3)
takeCopyable(d4) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableDerived' (aka 'DerivedTemplatedDefaultedCopyConstructor<NonCopyable>') 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'}}
}

6
test/Interop/Cxx/stdlib/Inputs/module.modulemap
View File

@@ -99,3 +99,9 @@ module CustomSmartPtr {
requires cplusplus
export *
}
module StdExpected {
header "std-expected.h"
requires cplusplus
export *
}

23
test/Interop/Cxx/stdlib/Inputs/std-expected.h Normal file
View File

@@ -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 <memory>
#include <expected>
using NonCopyableExpected = std::expected<std::unique_ptr<bool>, int>;
template<typename T>
class UniqueRef {
public:
std::unique_ptr<T> _field;
};
struct Decoder {};
enum Error {
DoomA,
DoomB
};
using DecoderOrError = std::expected<UniqueRef<Decoder>, Error>;
#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H

5
test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h
View File

@@ -89,4 +89,9 @@ struct CountCopies {
inline std::unique_ptr<CountCopies> getCopyCountedUniquePtr() { return std::make_unique<CountCopies>(); }
struct HasUniqueIntVector {
HasUniqueIntVector() = default;
std::vector<std::unique_ptr<int>> x;
};
#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H

6
test/Interop/Cxx/stdlib/Inputs/std-vector.h
View File

@@ -40,4 +40,10 @@ struct NonCopyable {
using VectorOfNonCopyable = std::vector<NonCopyable>;
using VectorOfPointer = std::vector<NonCopyable *>;
struct HasVector {
std::vector<NonCopyable> field;
};
struct BaseHasVector : HasVector {};
#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_VECTOR_H

22
test/Interop/Cxx/stdlib/use-std-expected-typechecker.swift Normal file
View File

@@ -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 <expected> 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<T: Copyable>(_ 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

4
test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift
View File

@@ -10,3 +10,7 @@ func takeCopyable<T: Copyable>(_ 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'

8
test/Interop/Cxx/stdlib/use-std-vector-typechecker.swift
View File

@@ -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'