Skip to content

Commit 7092c0b

Browse files
committed
[cxx-interop] Prevent usage in Swift of C++ copy constructor with default args
1 parent 09003d6 commit 7092c0b

File tree

6 files changed

+82
-1
lines changed

6 files changed

+82
-1
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7991,6 +7991,14 @@ static bool isSufficientlyTrivial(const clang::CXXRecordDecl *decl) {
79917991
return true;
79927992
}
79937993

7994+
static bool hasNonFirstDefaultArg(const clang::CXXConstructorDecl *ctor) {
7995+
if (ctor->getNumParams() < 2)
7996+
return false;
7997+
7998+
auto lastParam = ctor->parameters().back();
7999+
return lastParam->hasDefaultArg();
8000+
}
8001+
79948002
/// Checks if a record provides the required value type lifetime operations
79958003
/// (copy and destroy).
79968004
static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
@@ -8007,6 +8015,7 @@ static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
80078015
// struct.
80088016
return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
80098017
return ctor->isCopyConstructor() && !ctor->isDeleted() &&
8018+
!hasNonFirstDefaultArg(ctor) &&
80108019
ctor->getAccess() == clang::AccessSpecifier::AS_public;
80118020
});
80128021
}

lib/IRGen/GenStruct.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,12 +541,20 @@ namespace {
541541
FixedTypeInfo, ClangFieldInfo> {
542542
const clang::RecordDecl *ClangDecl;
543543

544+
bool hasNonFirstDefaultArg(const clang::CXXConstructorDecl *ctor) const {
545+
if (ctor->getNumParams() < 2)
546+
return false;
547+
548+
auto lastParam = ctor->parameters().back();
549+
return lastParam->hasDefaultArg();
550+
}
551+
544552
const clang::CXXConstructorDecl *findCopyConstructor() const {
545553
const auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(ClangDecl);
546554
if (!cxxRecordDecl)
547555
return nullptr;
548556
for (auto ctor : cxxRecordDecl->ctors()) {
549-
if (ctor->isCopyConstructor() &&
557+
if (ctor->isCopyConstructor() && !hasNonFirstDefaultArg(ctor) &&
550558
ctor->getAccess() == clang::AS_public && !ctor->isDeleted())
551559
return ctor;
552560
}

test/Interop/Cxx/class/Inputs/type-classification.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,16 @@ struct __attribute__((swift_attr("~Copyable"))) StructCopyableMovableAnnotatedNo
262262
~StructCopyableMovableAnnotatedNonCopyable() = default;
263263
};
264264

265+
struct HasCopyConstructorWithDefaultArgs {
266+
int value;
267+
HasCopyConstructorWithDefaultArgs(int value) : value(value) {}
268+
269+
HasCopyConstructorWithDefaultArgs(
270+
const HasCopyConstructorWithDefaultArgs &other, int value = 1)
271+
: value(other.value + value) {}
272+
273+
HasCopyConstructorWithDefaultArgs(HasCopyConstructorWithDefaultArgs &&) =
274+
default;
275+
};
276+
265277
#endif // TEST_INTEROP_CXX_CLASS_INPUTS_TYPE_CLASSIFICATION_H

test/Interop/Cxx/class/type-classification-typechecker.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ func testAnnotated() {
2222
_ = v
2323
}
2424

25+
func testNonCopyable() {
26+
let x = HasCopyConstructorWithDefaultArgs(5)
27+
let v = copy x // expected-error {{'copy' cannot be applied to noncopyable types}}
28+
_ = v
29+
}
30+
2531
test()
2632
testField()
2733
testAnnotated()
34+
testNonCopyable()

test/Interop/Cxx/value-witness-table/Inputs/copy-constructors.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,30 @@ struct HasNonTrivialDefaultCopyConstructor {
2323
const HasNonTrivialDefaultCopyConstructor &) = default;
2424
};
2525

26+
struct HasCopyConstructorWithDefaultArgs {
27+
int value;
28+
HasCopyConstructorWithDefaultArgs(int value) : value(value) {}
29+
30+
HasCopyConstructorWithDefaultArgs(
31+
const HasCopyConstructorWithDefaultArgs &other, int value = 1)
32+
: value(other.value + value) {}
33+
34+
HasCopyConstructorWithDefaultArgs(HasCopyConstructorWithDefaultArgs &&) =
35+
default;
36+
};
37+
38+
struct HasCopyConstructorWithOneParameterWithDefaultArg {
39+
int numCopies;
40+
41+
HasCopyConstructorWithOneParameterWithDefaultArg(int numCopies)
42+
: numCopies(numCopies) {}
43+
44+
HasCopyConstructorWithOneParameterWithDefaultArg(
45+
const HasCopyConstructorWithOneParameterWithDefaultArg &other =
46+
HasCopyConstructorWithOneParameterWithDefaultArg{1})
47+
: numCopies(other.numCopies + 1) {}
48+
};
49+
2650
// Make sure that we don't crash on struct templates with copy-constructors.
2751
template <typename T> struct S {
2852
S(S const &) {}

test/Interop/Cxx/value-witness-table/copy-constructors-execution.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
//
33
// REQUIRES: executable_test
44

5+
// FIXME this test breaks the SIL optimizer in Linux
6+
57
import CopyConstructors
68
import StdlibUnittest
79

@@ -48,4 +50,23 @@ CXXCopyConstructorTestSuite.test("Default copy constructor, member with user-def
4850
expectTrue(result.0.box.numCopies + result.1.box.numCopies > 0)
4951
}
5052

53+
CXXCopyConstructorTestSuite.test("Copy constructor with default arguments") {
54+
// When in the presence of a C++ copy constructor with default args, we make the type non-copyable
55+
let originalObj = HasCopyConstructorWithDefaultArgs(5)
56+
expectEqual(originalObj.value, 5)
57+
58+
// move originalObj
59+
let newObj = originalObj
60+
expectEqual(newObj.value, 5)
61+
}
62+
63+
CXXCopyConstructorTestSuite.test("Copy constructor with one parameter that has a default argument") {
64+
// If the C++ copy constructor has exactly one param and it has a default argument, ignore the default argument and import the type as copyable to Swift
65+
66+
let original = HasCopyConstructorWithOneParameterWithDefaultArg(0)
67+
let copy = original
68+
expectEqual(original.numCopies, 0)
69+
expectEqual(copy.numCopies, 1)
70+
}
71+
5172
runAllTests()

0 commit comments

Comments
 (0)