Skip to content

Commit 4a45436

Browse files
zoecarverzoecarver
committed
[cxx-interop] Only mark projections of self-contained types as unsafe.
Projections of trivial types and view types aren't unsafe. This matches what was described in the vision document.
1 parent 5c4d626 commit 4a45436

File tree

6 files changed

+189
-6
lines changed

6 files changed

+189
-6
lines changed

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6769,6 +6769,37 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
67696769
return nullptr;
67706770
}
67716771

6772+
bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) {
6773+
if (!decl->getDefinition())
6774+
return false;
6775+
6776+
if (hasCustomCopyOrMoveConstructor(decl))
6777+
return true;
6778+
6779+
auto checkType = [](clang::QualType t) {
6780+
if (auto recordType = dyn_cast<clang::RecordType>(t.getCanonicalType())) {
6781+
if (auto cxxRecord =
6782+
dyn_cast<clang::CXXRecordDecl>(recordType->getDecl())) {
6783+
return hasCustomCopyOrMoveConstructor(cxxRecord);
6784+
}
6785+
}
6786+
6787+
return false;
6788+
};
6789+
6790+
for (auto field : decl->fields()) {
6791+
if (checkType(field->getType()))
6792+
return true;
6793+
}
6794+
6795+
for (auto base : decl->bases()) {
6796+
if (checkType(base.getType()))
6797+
return true;
6798+
}
6799+
6800+
return false;
6801+
}
6802+
67726803
bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67736804
SafeUseOfCxxDeclDescriptor desc) const {
67746805
const clang::Decl *decl = desc.decl;
@@ -6786,10 +6817,19 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67866817
if (isForeignReferenceType(method->getReturnType()))
67876818
return true;
67886819

6789-
// If it returns a pointer or reference, that's a projection.
6820+
auto parentQualType = method
6821+
->getParent()->getTypeForDecl()->getCanonicalTypeUnqualified();
6822+
6823+
bool parentIsSelfContained =
6824+
hasOwnedValueAttr(method->getParent()) ||
6825+
(!isForeignReferenceType(parentQualType) &&
6826+
anySubobjectsSelfContained(method->getParent()));
6827+
6828+
// If it returns a pointer or reference from an owned parent, that's a
6829+
// projection (unsafe).
67906830
if (method->getReturnType()->isPointerType() ||
67916831
method->getReturnType()->isReferenceType())
6792-
return false;
6832+
return !parentIsSelfContained;
67936833

67946834
// Check if it's one of the known unsafe methods we currently
67956835
// mark as safe by default.
@@ -6804,20 +6844,23 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
68046844
dyn_cast<clang::CXXRecordDecl>(returnType->getDecl())) {
68056845
if (isSwiftClassType(cxxRecordReturnType))
68066846
return true;
6847+
68076848
if (hasIteratorAPIAttr(cxxRecordReturnType) ||
68086849
isIterator(cxxRecordReturnType)) {
6809-
return false;
6850+
return !parentIsSelfContained;
68106851
}
68116852

68126853
// Mark this as safe to help our diganostics down the road.
68136854
if (!cxxRecordReturnType->getDefinition()) {
68146855
return true;
68156856
}
68166857

6817-
if (!hasCustomCopyOrMoveConstructor(cxxRecordReturnType) &&
6858+
// A projection of a view type (such as a string_view) from a self
6859+
// contained parent is a proejction (unsafe).
6860+
if (!anySubobjectsSelfContained(cxxRecordReturnType) &&
68186861
!hasOwnedValueAttr(cxxRecordReturnType) &&
68196862
hasPointerInSubobjects(cxxRecordReturnType)) {
6820-
return false;
6863+
return !parentIsSelfContained;
68216864
}
68226865
}
68236866
}

‎test/Interop/Cxx/class/fixit-add-safe-to-import-self-contained.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ module Test {
1212
struct Ptr { int *p; };
1313

1414
struct X {
15+
X(const X&);
16+
1517
int *test() { }
1618
Ptr other() { }
1719
};

‎test/Interop/Cxx/class/method/Inputs/ambiguous_methods.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ struct Unsafe {
5252
};
5353

5454
struct HasAmbiguousUnsafeMethods {
55+
HasAmbiguousUnsafeMethods(const HasAmbiguousUnsafeMethods&);
5556
Unsafe getUnsafe() const { return Unsafe(); }
5657
Unsafe getUnsafe() { return Unsafe(); }
5758
};

‎test/Interop/Cxx/class/method/Inputs/module.modulemap

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,11 @@ module Methods {
66
module AmbiguousMethods {
77
header "ambiguous_methods.h"
88
requires cplusplus
9-
}
9+
}
10+
11+
module UnsafeProjections {
12+
header "unsafe-projections.h"
13+
requires cplusplus
14+
15+
export *
16+
}

‎test/Interop/Cxx/class/method/Inputs/unsafe-projections.h

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
#ifndef TEST_INTEROP_CXX_CLASS_METHOD_UNSAFE_PROJECTIONS_H
2+
#define TEST_INTEROP_CXX_CLASS_METHOD_UNSAFE_PROJECTIONS_H
3+
4+
#include <string>
5+
6+
struct NestedSelfContained;
7+
struct Empty;
8+
struct SelfContained;
9+
10+
struct View {
11+
void *ptr;
12+
13+
void *data() const;
14+
void *empty() const;
15+
std::string name() const;
16+
NestedSelfContained nested() const;
17+
};
18+
19+
struct SelfContained {
20+
void *ptr;
21+
SelfContained(const SelfContained&);
22+
23+
std::string name() const;
24+
SelfContained selfContained() const;
25+
NestedSelfContained nested() const;
26+
Empty empty() const;
27+
int value() const;
28+
View view() const;
29+
int *pointer() const;
30+
};
31+
32+
struct NestedSelfContained {
33+
SelfContained member;
34+
35+
std::string name() const;
36+
SelfContained selfContained() const;
37+
NestedSelfContained nested() const;
38+
Empty empty() const;
39+
int value() const;
40+
View view() const;
41+
int *pointer() const;
42+
};
43+
44+
struct InheritSelfContained: SelfContained {
45+
std::string name() const;
46+
SelfContained selfContained() const;
47+
NestedSelfContained nested() const;
48+
Empty empty() const;
49+
int value() const;
50+
View view() const;
51+
int *pointer() const;
52+
};
53+
54+
struct __attribute__((swift_attr("import_owned"))) ExplicitSelfContained {
55+
void *ptr;
56+
57+
void *pointer() const;
58+
View view() const;
59+
};
60+
61+
struct Empty {
62+
Empty empty() const;
63+
void *pointer() const;
64+
SelfContained selfContained() const;
65+
};
66+
67+
struct IntPair {
68+
int a; int b;
69+
70+
int first() const;
71+
void *pointer() const;
72+
SelfContained selfContained() const;
73+
};
74+
75+
#endif // TEST_INTEROP_CXX_CLASS_METHOD_UNSAFE_PROJECTIONS_H

‎test/Interop/Cxx/class/method/unsafe-projections.swift

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %target-swift-ide-test -print-module -module-to-print=UnsafeProjections -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
2+
3+
// CHECK: struct View {
4+
// CHECK: func data() -> UnsafeMutableRawPointer!
5+
// CHECK: func empty() -> UnsafeMutableRawPointer!
6+
// CHECK: func name() -> std{{.*}}string
7+
// CHECK: func nested() -> NestedSelfContained
8+
// CHECK: }
9+
10+
// CHECK: struct SelfContained {
11+
// CHECK: func name() -> std{{.*}}string
12+
// CHECK: func selfContained() -> SelfContained
13+
// CHECK: func nested() -> NestedSelfContained
14+
// CHECK: func empty() -> Empty
15+
// CHECK: func value() -> Int32
16+
// CHECK: func __viewUnsafe() -> View
17+
// CHECK: func __pointerUnsafe() -> UnsafeMutablePointer<Int32>!
18+
// CHECK: }
19+
20+
// CHECK: struct NestedSelfContained {
21+
// CHECK: func name() -> std{{.*}}string
22+
// CHECK: func selfContained() -> SelfContained
23+
// CHECK: func nested() -> NestedSelfContained
24+
// CHECK: func empty() -> Empty
25+
// CHECK: func value() -> Int32
26+
// CHECK: func __viewUnsafe() -> View
27+
// CHECK: func __pointerUnsafe() -> UnsafeMutablePointer<Int32>!
28+
// CHECK: }
29+
30+
// CHECK: struct InheritSelfContained {
31+
// CHECK: func name() -> std{{.*}}string
32+
// CHECK: func selfContained() -> SelfContained
33+
// CHECK: func nested() -> NestedSelfContained
34+
// CHECK: func empty() -> Empty
35+
// CHECK: func value() -> Int32
36+
// CHECK: func __viewUnsafe() -> View
37+
// CHECK: func __pointerUnsafe() -> UnsafeMutablePointer<Int32>!
38+
// CHECK: }
39+
40+
// CHECK: struct ExplicitSelfContained {
41+
// CHECK: func __pointerUnsafe() -> UnsafeMutableRawPointer!
42+
// CHECK: func __viewUnsafe() -> View
43+
// CHECK: }
44+
45+
// CHECK: struct Empty {
46+
// CHECK: func empty() -> Empty
47+
// CHECK: func pointer() -> UnsafeMutableRawPointer!
48+
// CHECK: func selfContained() -> SelfContained
49+
// CHECK: }
50+
51+
// CHECK: struct IntPair {
52+
// CHECK: func first() -> Int32
53+
// CHECK: func pointer() -> UnsafeMutableRawPointer!
54+
// CHECK: func selfContained() -> SelfContained
55+
// CHECK: }

0 commit comments

Comments
 (0)