Skip to content

Commit 4cb9dc9

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 6e47a36 commit 4cb9dc9

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
@@ -6778,6 +6778,37 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
67786778
return nullptr;
67796779
}
67806780

6781+
bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) {
6782+
if (!decl->getDefinition())
6783+
return false;
6784+
6785+
if (hasCustomCopyOrMoveConstructor(decl))
6786+
return true;
6787+
6788+
auto checkType = [](clang::QualType t) {
6789+
if (auto recordType = dyn_cast<clang::RecordType>(t.getCanonicalType())) {
6790+
if (auto cxxRecord =
6791+
dyn_cast<clang::CXXRecordDecl>(recordType->getDecl())) {
6792+
return hasCustomCopyOrMoveConstructor(cxxRecord);
6793+
}
6794+
}
6795+
6796+
return false;
6797+
};
6798+
6799+
for (auto field : decl->fields()) {
6800+
if (checkType(field->getType()))
6801+
return true;
6802+
}
6803+
6804+
for (auto base : decl->bases()) {
6805+
if (checkType(base.getType()))
6806+
return true;
6807+
}
6808+
6809+
return false;
6810+
}
6811+
67816812
bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67826813
SafeUseOfCxxDeclDescriptor desc) const {
67836814
const clang::Decl *decl = desc.decl;
@@ -6795,10 +6826,19 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67956826
if (isForeignReferenceType(method->getReturnType()))
67966827
return true;
67976828

6798-
// If it returns a pointer or reference, that's a projection.
6829+
auto parentQualType = method
6830+
->getParent()->getTypeForDecl()->getCanonicalTypeUnqualified();
6831+
6832+
bool parentIsSelfContained =
6833+
hasOwnedValueAttr(method->getParent()) ||
6834+
(!isForeignReferenceType(parentQualType) &&
6835+
anySubobjectsSelfContained(method->getParent()));
6836+
6837+
// If it returns a pointer or reference from an owned parent, that's a
6838+
// projection (unsafe).
67996839
if (method->getReturnType()->isPointerType() ||
68006840
method->getReturnType()->isReferenceType())
6801-
return false;
6841+
return !parentIsSelfContained;
68026842

68036843
// Check if it's one of the known unsafe methods we currently
68046844
// mark as safe by default.
@@ -6813,20 +6853,23 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
68136853
dyn_cast<clang::CXXRecordDecl>(returnType->getDecl())) {
68146854
if (isSwiftClassType(cxxRecordReturnType))
68156855
return true;
6856+
68166857
if (hasIteratorAPIAttr(cxxRecordReturnType) ||
68176858
isIterator(cxxRecordReturnType)) {
6818-
return false;
6859+
return !parentIsSelfContained;
68196860
}
68206861

68216862
// Mark this as safe to help our diganostics down the road.
68226863
if (!cxxRecordReturnType->getDefinition()) {
68236864
return true;
68246865
}
68256866

6826-
if (!hasCustomCopyOrMoveConstructor(cxxRecordReturnType) &&
6867+
// A projection of a view type (such as a string_view) from a self
6868+
// contained parent is a proejction (unsafe).
6869+
if (!anySubobjectsSelfContained(cxxRecordReturnType) &&
68276870
!hasOwnedValueAttr(cxxRecordReturnType) &&
68286871
hasPointerInSubobjects(cxxRecordReturnType)) {
6829-
return false;
6872+
return !parentIsSelfContained;
68306873
}
68316874
}
68326875
}

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