Skip to content

Commit fbbec48

Browse files
committed
[cxx-interop] Check the presence of copy constructor correctly
This removes a longstanding workaround in the import logic for C++ structs: Swift assumed that if a C++ struct has no copy constructor that is explicitly deleted, then the struct is copyable. This is not actually correct. This replaces the workaround with a proper check for the presence of a C++ copy constructor. rdar://136838485
1 parent f66a2b1 commit fbbec48

File tree

4 files changed

+41
-8
lines changed

4 files changed

+41
-8
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7619,18 +7619,22 @@ static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
76197619
if (decl->isInStdNamespace() && decl->getIdentifier() &&
76207620
decl->getName() == "_Optional_construct_base")
76217621
return true;
7622+
// Hack for std::vector::const_iterator from libstdc++, which uses an extra
7623+
// parameter on its copy constructor, which has a defaulted enable_if value.
7624+
auto namespaceContext = dyn_cast_or_null<clang::NamespaceDecl>(
7625+
decl->getEnclosingNamespaceContext());
7626+
if (namespaceContext && namespaceContext->getIdentifier() &&
7627+
namespaceContext->getName() == "__gnu_cxx" && decl->getIdentifier() &&
7628+
decl->getName() == "__normal_iterator")
7629+
return true;
76227630

76237631
// If we have no way of copying the type we can't import the class
76247632
// at all because we cannot express the correct semantics as a swift
76257633
// struct.
7626-
if (llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
7627-
return ctor->isCopyConstructor() &&
7628-
(ctor->isDeleted() || ctor->getAccess() != clang::AS_public);
7629-
}))
7630-
return false;
7631-
7632-
// TODO: this should probably check to make sure we actually have a copy ctor.
7633-
return true;
7634+
return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
7635+
return ctor->isCopyConstructor() && !ctor->isDeleted() &&
7636+
ctor->getAccess() == clang::AccessSpecifier::AS_public;
7637+
});
76347638
}
76357639

76367640
static bool hasMoveTypeOperations(const clang::CXXRecordDecl *decl) {

test/Interop/Cxx/class/Inputs/constructors.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,25 @@ struct TemplatedConstructorWithExtraArg {
7171
TemplatedConstructorWithExtraArg(T value, U other) { }
7272
};
7373

74+
struct TemplatedCopyConstructor {
75+
int x = 0;
76+
77+
TemplatedCopyConstructor(int x) : x(x) {}
78+
79+
template <class T>
80+
TemplatedCopyConstructor(const T &value) : x(value.x) {}
81+
};
82+
83+
struct TemplatedCopyConstructorWithExtraArg {
84+
int x = 0;
85+
86+
TemplatedCopyConstructorWithExtraArg(int x) : x(x) {}
87+
88+
template <class T>
89+
TemplatedCopyConstructorWithExtraArg(const T &value, int add = 0)
90+
: x(value.x + add) {}
91+
};
92+
7493
struct __attribute__((swift_attr("import_unsafe")))
7594
HasUserProvidedCopyConstructor {
7695
int numCopies;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
// RUN: %target-swift-ide-test -print-module -module-to-print=Constructors -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
22

3+
// CHECK: struct TemplatedCopyConstructor
4+
// CHECK: struct TemplatedCopyConstructorWithExtraArg
5+
36
// Make sure we don't import non-copyable types because we will have no way to
47
// represent and copy/move these in swift with correct semantics.
58
// CHECK-NOT: DeletedCopyConstructor

test/Interop/Cxx/class/constructors-typechecker.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import Constructors
44

5+
func takesCopyable<T: Copyable>(_ x: T.Type) {}
6+
57
let explicit = ExplicitDefaultConstructor()
68

79
let implicit = ImplicitDefaultConstructor()
@@ -12,3 +14,8 @@ let onlyCopyAndMove = CopyAndMoveConstructor() // expected-warning {{'init()' is
1214
let deletedExplicitly = DefaultConstructorDeleted() // expected-error {{missing argument for parameter 'a' in call}}
1315

1416
let withArg = ConstructorWithParam(42)
17+
18+
let _ = TemplatedCopyConstructor(123)
19+
let _ = TemplatedCopyConstructorWithExtraArg(123)
20+
takesCopyable(TemplatedCopyConstructor.self)
21+
takesCopyable(TemplatedCopyConstructorWithExtraArg.self)

0 commit comments

Comments
 (0)