Skip to content

Commit 474a7cd

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 2abfa25 commit 474a7cd

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
@@ -6789,6 +6789,37 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
67896789
return nullptr;
67906790
}
67916791

6792+
bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) {
6793+
if (!decl->getDefinition())
6794+
return false;
6795+
6796+
if (hasCustomCopyOrMoveConstructor(decl))
6797+
return true;
6798+
6799+
auto checkType = [](clang::QualType t) {
6800+
if (auto recordType = dyn_cast<clang::RecordType>(t.getCanonicalType())) {
6801+
if (auto cxxRecord =
6802+
dyn_cast<clang::CXXRecordDecl>(recordType->getDecl())) {
6803+
return hasCustomCopyOrMoveConstructor(cxxRecord);
6804+
}
6805+
}
6806+
6807+
return false;
6808+
};
6809+
6810+
for (auto field : decl->fields()) {
6811+
if (checkType(field->getType()))
6812+
return true;
6813+
}
6814+
6815+
for (auto base : decl->bases()) {
6816+
if (checkType(base.getType()))
6817+
return true;
6818+
}
6819+
6820+
return false;
6821+
}
6822+
67926823
bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67936824
SafeUseOfCxxDeclDescriptor desc) const {
67946825
const clang::Decl *decl = desc.decl;
@@ -6806,10 +6837,19 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
68066837
if (isForeignReferenceType(method->getReturnType()))
68076838
return true;
68086839

6809-
// If it returns a pointer or reference, that's a projection.
6840+
auto parentQualType = method
6841+
->getParent()->getTypeForDecl()->getCanonicalTypeUnqualified();
6842+
6843+
bool parentIsSelfContained =
6844+
hasOwnedValueAttr(method->getParent()) ||
6845+
(!isForeignReferenceType(parentQualType) &&
6846+
anySubobjectsSelfContained(method->getParent()));
6847+
6848+
// If it returns a pointer or reference from an owned parent, that's a
6849+
// projection (unsafe).
68106850
if (method->getReturnType()->isPointerType() ||
68116851
method->getReturnType()->isReferenceType())
6812-
return false;
6852+
return !parentIsSelfContained;
68136853

68146854
// Check if it's one of the known unsafe methods we currently
68156855
// mark as safe by default.
@@ -6824,20 +6864,23 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
68246864
dyn_cast<clang::CXXRecordDecl>(returnType->getDecl())) {
68256865
if (isSwiftClassType(cxxRecordReturnType))
68266866
return true;
6867+
68276868
if (hasIteratorAPIAttr(cxxRecordReturnType) ||
68286869
isIterator(cxxRecordReturnType)) {
6829-
return false;
6870+
return !parentIsSelfContained;
68306871
}
68316872

68326873
// Mark this as safe to help our diganostics down the road.
68336874
if (!cxxRecordReturnType->getDefinition()) {
68346875
return true;
68356876
}
68366877

6837-
if (!hasCustomCopyOrMoveConstructor(cxxRecordReturnType) &&
6878+
// A projection of a view type (such as a string_view) from a self
6879+
// contained parent is a proejction (unsafe).
6880+
if (!anySubobjectsSelfContained(cxxRecordReturnType) &&
68386881
!hasOwnedValueAttr(cxxRecordReturnType) &&
68396882
hasPointerInSubobjects(cxxRecordReturnType)) {
6840-
return false;
6883+
return !parentIsSelfContained;
68416884
}
68426885
}
68436886
}

‎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)