Skip to content

Commit 266be88

Browse files
susmonteirosusmonteiro
committed
[cxx-interop] Implicitly defined copy constructors
1 parent 90e3d1c commit 266be88

File tree

8 files changed

+143
-45
lines changed

8 files changed

+143
-45
lines changed

‎include/swift/ClangImporter/ClangImporter.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,12 @@ ValueDecl *getImportedMemberOperator(const DeclBaseName &name,
733733
/// as permissive as the input C++ access.
734734
AccessLevel convertClangAccess(clang::AccessSpecifier access);
735735

736+
/// Lookup and return the copy constructor of \a decl
737+
///
738+
/// Returns nullptr if \a decl doesn't have a valid copy constructor
739+
const clang::CXXConstructorDecl *
740+
findCopyConstructor(const clang::CXXRecordDecl *decl);
741+
736742
/// Read file IDs from 'private_fileid' Swift attributes on a Clang decl.
737743
///
738744
/// May return >1 fileID when a decl is annotated more than once, which should

‎lib/ClangImporter/ClangImporter.cpp‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8218,16 +8218,39 @@ bool importer::isViewType(const clang::CXXRecordDecl *decl) {
82188218
return !hasOwnedValueAttr(decl) && hasPointerInSubobjects(decl);
82198219
}
82208220

8221-
static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
8222-
if (decl->hasSimpleCopyConstructor())
8223-
return true;
8221+
const clang::CXXConstructorDecl *
8222+
importer::findCopyConstructor(const clang::CXXRecordDecl *decl) {
8223+
for (auto ctor : decl->ctors()) {
8224+
if (ctor->isCopyConstructor() &&
8225+
// FIXME: Support default arguments (rdar://142414553)
8226+
ctor->getNumParams() == 1 && ctor->getAccess() == clang::AS_public &&
8227+
!ctor->isDeleted() && !ctor->isIneligibleOrNotSelected())
8228+
return ctor;
8229+
}
8230+
return nullptr;
8231+
}
82248232

8225-
return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
8226-
return ctor->isCopyConstructor() && !ctor->isDeleted() &&
8227-
!ctor->isIneligibleOrNotSelected() &&
8228-
// FIXME: Support default arguments (rdar://142414553)
8229-
ctor->getNumParams() == 1 &&
8230-
ctor->getAccess() == clang::AccessSpecifier::AS_public;
8233+
static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl,
8234+
ClangImporter::Implementation *importerImpl) {
8235+
if (!decl->hasSimpleCopyConstructor()) {
8236+
auto *copyCtor = findCopyConstructor(decl);
8237+
if (!copyCtor)
8238+
return false;
8239+
8240+
if (!copyCtor->isDefaulted())
8241+
return true;
8242+
}
8243+
8244+
// If the copy constructor is defaulted or implicitly declared, we should only
8245+
// import the type as copyable if all its fields are also copyable
8246+
// FIXME: we should also look at the bases
8247+
return llvm::none_of(decl->fields(), [&](clang::FieldDecl *field) {
8248+
if (const auto *rd = field->getType()->getAsRecordDecl()) {
8249+
return (!field->getType()->isReferenceType() &&
8250+
!field->getType()->isPointerType() &&
8251+
recordHasMoveOnlySemantics(rd, importerImpl));
8252+
}
8253+
return false;
82318254
});
82328255
}
82338256

@@ -8423,8 +8446,8 @@ CxxValueSemantics::evaluate(Evaluator &evaluator,
84238446
return CxxValueSemanticsKind::Copyable;
84248447
}
84258448

8426-
bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) &&
8427-
hasCopyTypeOperations(cxxRecordDecl);
8449+
bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) &&
8450+
hasCopyTypeOperations(cxxRecordDecl, importerImpl);
84288451
bool isMovable = hasMoveTypeOperations(cxxRecordDecl);
84298452

84308453
if (!hasDestroyTypeOperations(cxxRecordDecl) || (!isCopyable && !isMovable)) {

‎lib/ClangImporter/ImportDecl.cpp‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,15 @@ bool importer::recordHasReferenceSemantics(
185185
return semanticsKind == CxxRecordSemanticsKind::Reference;
186186
}
187187

188+
bool importer::recordHasMoveOnlySemantics(
189+
const clang::RecordDecl *decl,
190+
ClangImporter::Implementation *importerImpl) {
191+
auto semanticsKind = evaluateOrDefault(
192+
importerImpl->SwiftContext.evaluator,
193+
CxxValueSemantics({decl->getTypeForDecl(), importerImpl}), {});
194+
return semanticsKind == CxxValueSemanticsKind::MoveOnly;
195+
}
196+
188197
bool importer::hasImmortalAttrs(const clang::RecordDecl *decl) {
189198
return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
190199
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
@@ -2093,10 +2102,7 @@ namespace {
20932102
}
20942103

20952104
bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl) {
2096-
auto semanticsKind = evaluateOrDefault(
2097-
Impl.SwiftContext.evaluator,
2098-
CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {});
2099-
return semanticsKind == CxxValueSemanticsKind::MoveOnly;
2105+
return importer::recordHasMoveOnlySemantics(decl, &Impl);
21002106
}
21012107

21022108
void markReturnsUnsafeNonescapable(AbstractFunctionDecl *fd) {
@@ -2275,15 +2281,6 @@ namespace {
22752281
loc, ArrayRef<InheritedEntry>(), nullptr, dc);
22762282
Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;
22772283

2278-
if (recordHasMoveOnlySemantics(decl)) {
2279-
if (decl->isInStdNamespace() && decl->getName() == "promise") {
2280-
// Do not import std::promise.
2281-
return nullptr;
2282-
}
2283-
result->getAttrs().add(new (Impl.SwiftContext)
2284-
MoveOnlyAttr(/*Implicit=*/true));
2285-
}
2286-
22872284
// FIXME: Figure out what to do with superclasses in C++. One possible
22882285
// solution would be to turn them into members and add conversion
22892286
// functions.
@@ -2527,6 +2524,15 @@ namespace {
25272524
cxxRecordDecl->ctors().empty());
25282525
}
25292526

2527+
if (recordHasMoveOnlySemantics(decl)) {
2528+
if (decl->isInStdNamespace() && decl->getName() == "promise") {
2529+
// Do not import std::promise.
2530+
return nullptr;
2531+
}
2532+
result->getAttrs().add(new (Impl.SwiftContext)
2533+
MoveOnlyAttr(/*Implicit=*/true));
2534+
}
2535+
25302536
// TODO: builtin "zeroInitializer" does not work with non-escapable
25312537
// types yet. Don't generate an initializer.
25322538
if (hasZeroInitializableStorage && needsEmptyInitializer &&
@@ -3126,11 +3132,10 @@ namespace {
31263132
// instantiate its copy constructor.
31273133
bool isExplicitlyNonCopyable = hasNonCopyableAttr(decl);
31283134

3129-
clang::CXXConstructorDecl *copyCtor = nullptr;
31303135
clang::CXXConstructorDecl *moveCtor = nullptr;
31313136
clang::CXXConstructorDecl *defaultCtor = nullptr;
31323137
if (decl->needsImplicitCopyConstructor() && !isExplicitlyNonCopyable) {
3133-
copyCtor = clangSema.DeclareImplicitCopyConstructor(
3138+
clangSema.DeclareImplicitCopyConstructor(
31343139
const_cast<clang::CXXRecordDecl *>(decl));
31353140
}
31363141
if (decl->needsImplicitMoveConstructor()) {
@@ -3151,10 +3156,7 @@ namespace {
31513156
// Note: we use "doesThisDeclarationHaveABody" here because
31523157
// that's what "DefineImplicitCopyConstructor" checks.
31533158
!declCtor->doesThisDeclarationHaveABody()) {
3154-
if (declCtor->isCopyConstructor()) {
3155-
if (!copyCtor && !isExplicitlyNonCopyable)
3156-
copyCtor = declCtor;
3157-
} else if (declCtor->isMoveConstructor()) {
3159+
if (declCtor->isMoveConstructor()) {
31583160
if (!moveCtor)
31593161
moveCtor = declCtor;
31603162
} else if (declCtor->isDefaultConstructor()) {
@@ -3164,11 +3166,6 @@ namespace {
31643166
}
31653167
}
31663168
}
3167-
if (copyCtor && !isExplicitlyNonCopyable &&
3168-
!decl->isAnonymousStructOrUnion()) {
3169-
clangSema.DefineImplicitCopyConstructor(clang::SourceLocation(),
3170-
copyCtor);
3171-
}
31723169
if (moveCtor && !decl->isAnonymousStructOrUnion()) {
31733170
clangSema.DefineImplicitMoveConstructor(clang::SourceLocation(),
31743171
moveCtor);

‎lib/ClangImporter/ImporterImpl.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1974,6 +1974,11 @@ namespace importer {
19741974
bool recordHasReferenceSemantics(const clang::RecordDecl *decl,
19751975
ClangImporter::Implementation *importerImpl);
19761976

1977+
/// Returns true if the given C/C++ record should be imported as non-copyable into
1978+
/// Swift.
1979+
bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl,
1980+
ClangImporter::Implementation *importerImpl);
1981+
19771982
/// Whether this is a forward declaration of a type. We ignore forward
19781983
/// declarations in certain cases, and instead process the real declarations.
19791984
bool isForwardDeclOfType(const clang::Decl *decl);

‎lib/IRGen/GenStruct.cpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -553,15 +553,7 @@ namespace {
553553
const auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(ClangDecl);
554554
if (!cxxRecordDecl)
555555
return nullptr;
556-
for (auto ctor : cxxRecordDecl->ctors()) {
557-
if (ctor->isCopyConstructor() &&
558-
// FIXME: Support default arguments (rdar://142414553)
559-
ctor->getNumParams() == 1 &&
560-
ctor->getAccess() == clang::AS_public && !ctor->isDeleted() &&
561-
!ctor->isIneligibleOrNotSelected())
562-
return ctor;
563-
}
564-
return nullptr;
556+
return importer::findCopyConstructor(cxxRecordDecl);
565557
}
566558

567559
const clang::CXXConstructorDecl *findMoveConstructor() const {
@@ -629,7 +621,18 @@ namespace {
629621

630622
auto &ctx = IGF.IGM.Context;
631623
auto *importer = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
632-
624+
625+
if (copyConstructor->isDefaulted() &&
626+
copyConstructor->getAccess() == clang::AS_public &&
627+
!copyConstructor->isDeleted() &&
628+
// Note: we use "doesThisDeclarationHaveABody" here because
629+
// that's what "DefineImplicitCopyConstructor" checks.
630+
!copyConstructor->doesThisDeclarationHaveABody()) {
631+
importer->getClangSema().DefineImplicitCopyConstructor(
632+
clang::SourceLocation(),
633+
const_cast<clang::CXXConstructorDecl *>(copyConstructor));
634+
}
635+
633636
auto &diagEngine = importer->getClangSema().getDiagnostics();
634637
clang::DiagnosticErrorTrap trap(diagEngine);
635638
auto clangFnAddr =

‎test/Interop/Cxx/class/noncopyable-typechecker.swift‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,42 @@ struct SWIFT_NONCOPYABLE NonCopyableNonMovable { // expected-note {{record 'NonC
7979
NonCopyableNonMovable(NonCopyableNonMovable&& other) = delete;
8080
};
8181

82+
struct ImplicitCopyConstructor {
83+
NonCopyable element;
84+
};
85+
86+
template <typename T, typename P, typename R>
87+
struct TemplatedImplicitCopyConstructor {
88+
T element;
89+
P *pointer;
90+
R &reference;
91+
};
92+
93+
using NonCopyableT = TemplatedImplicitCopyConstructor<NonCopyable, int, int>;
94+
using NonCopyableP = TemplatedImplicitCopyConstructor<int, NonCopyable, int>;
95+
using NonCopyableR = TemplatedImplicitCopyConstructor<int, int, NonCopyable>;
96+
97+
struct DefaultedCopyConstructor {
98+
NonCopyable element;
99+
DefaultedCopyConstructor(const DefaultedCopyConstructor&) = default;
100+
DefaultedCopyConstructor(DefaultedCopyConstructor&&) = default;
101+
};
102+
103+
template<typename T>
104+
struct TemplatedDefaultedCopyConstructor {
105+
T element;
106+
TemplatedDefaultedCopyConstructor(const TemplatedDefaultedCopyConstructor&) = default;
107+
TemplatedDefaultedCopyConstructor(TemplatedDefaultedCopyConstructor&&) = default;
108+
};
109+
110+
template<typename T>
111+
struct DerivedTemplatedDefaultedCopyConstructor : TemplatedDefaultedCopyConstructor<T> {};
112+
113+
using CopyableDefaultedCopyConstructor = TemplatedDefaultedCopyConstructor<NonCopyableP>;
114+
using NonCopyableDefaultedCopyConstructor = TemplatedDefaultedCopyConstructor<NonCopyable>;
115+
using CopyableDerived = DerivedTemplatedDefaultedCopyConstructor<NonCopyableR>;
116+
using NonCopyableDerived = DerivedTemplatedDefaultedCopyConstructor<NonCopyable>;
117+
82118

83119
//--- test.swift
84120
import Test
@@ -129,3 +165,20 @@ func missingLifetimeOperation() {
129165
let s = NonCopyableNonMovable() // expected-error {{cannot find 'NonCopyableNonMovable' in scope}}
130166
takeCopyable(s)
131167
}
168+
169+
func implicitCopyConstructor(i: borrowing ImplicitCopyConstructor, t: borrowing NonCopyableT, p: borrowing NonCopyableP, r: borrowing NonCopyableR) {
170+
takeCopyable(i) // expected-error {{global function 'takeCopyable' requires that 'ImplicitCopyConstructor' conform to 'Copyable'}}
171+
takeCopyable(t) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableT' (aka 'TemplatedImplicitCopyConstructor<NonCopyable, CInt, CInt>') conform to 'Copyable'}}
172+
173+
// References and pointers to non-copyable types are still copyable
174+
takeCopyable(p)
175+
takeCopyable(r)
176+
}
177+
178+
func defaultCopyConstructor(d: borrowing DefaultedCopyConstructor, d1: borrowing CopyableDefaultedCopyConstructor, d2: borrowing NonCopyableDefaultedCopyConstructor, d3: borrowing CopyableDerived, d4: borrowing NonCopyableDerived) {
179+
takeCopyable(d) // expected-error {{global function 'takeCopyable' requires that 'DefaultedCopyConstructor' conform to 'Copyable'}}
180+
takeCopyable(d1)
181+
takeCopyable(d2) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableDefaultedCopyConstructor' (aka 'TemplatedDefaultedCopyConstructor<NonCopyable>') conform to 'Copyable'}}
182+
takeCopyable(d3)
183+
takeCopyable(d4) // expected-error {{global function 'takeCopyable' requires that 'NonCopyableDerived' (aka 'DerivedTemplatedDefaultedCopyConstructor<NonCopyable>') conform to 'Copyable'}}
184+
}

‎test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,9 @@ struct CountCopies {
8989

9090
inline std::unique_ptr<CountCopies> getCopyCountedUniquePtr() { return std::make_unique<CountCopies>(); }
9191

92+
struct HasUniqueIntVector {
93+
HasUniqueIntVector() = default;
94+
std::vector<std::unique_ptr<int>> x;
95+
};
96+
9297
#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H

‎test/Interop/Cxx/stdlib/use-std-unique-ptr-typechecker.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,9 @@ func takeCopyable<T: Copyable>(_ x: T) {}
1010
let vecUniquePtr = getVectorNonCopyableUniquePtr()
1111
takeCopyable(vecUniquePtr)
1212
// CHECK: error: global function 'takeCopyable' requires that 'std{{.*}}vector{{.*}}unique_ptr{{.*}}NonCopyable{{.*}}' conform to 'Copyable'
13+
14+
let uniqueIntVec = HasUniqueIntVector()
15+
takeCopyable(uniqueIntVec)
16+
// CHECK: error: global function 'takeCopyable' requires that 'HasUniqueIntVector' conform to 'Copyable'
17+
18+

0 commit comments

Comments
 (0)