Skip to content

Commit e11a14b

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 24d21f9 commit e11a14b

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
@@ -6743,6 +6743,37 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
67436743
return nullptr;
67446744
}
67456745

6746+
bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) {
6747+
if (!decl->getDefinition())
6748+
return false;
6749+
6750+
if (hasCustomCopyOrMoveConstructor(decl))
6751+
return true;
6752+
6753+
auto checkType = [](clang::QualType t) {
6754+
if (auto recordType = dyn_cast<clang::RecordType>(t.getCanonicalType())) {
6755+
if (auto cxxRecord =
6756+
dyn_cast<clang::CXXRecordDecl>(recordType->getDecl())) {
6757+
return hasCustomCopyOrMoveConstructor(cxxRecord);
6758+
}
6759+
}
6760+
6761+
return false;
6762+
};
6763+
6764+
for (auto field : decl->fields()) {
6765+
if (checkType(field->getType()))
6766+
return true;
6767+
}
6768+
6769+
for (auto base : decl->bases()) {
6770+
if (checkType(base.getType()))
6771+
return true;
6772+
}
6773+
6774+
return false;
6775+
}
6776+
67466777
bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67476778
SafeUseOfCxxDeclDescriptor desc) const {
67486779
const clang::Decl *decl = desc.decl;
@@ -6760,10 +6791,19 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67606791
if (isForeignReferenceType(method->getReturnType()))
67616792
return true;
67626793

6763-
// If it returns a pointer or reference, that's a projection.
6794+
auto parentQualType = method
6795+
->getParent()->getTypeForDecl()->getCanonicalTypeUnqualified();
6796+
6797+
bool parentIsSelfContained =
6798+
hasOwnedValueAttr(method->getParent()) ||
6799+
(!isForeignReferenceType(parentQualType) &&
6800+
anySubobjectsSelfContained(method->getParent()));
6801+
6802+
// If it returns a pointer or reference from an owned parent, that's a
6803+
// projection (unsafe).
67646804
if (method->getReturnType()->isPointerType() ||
67656805
method->getReturnType()->isReferenceType())
6766-
return false;
6806+
return !parentIsSelfContained;
67676807

67686808
// Check if it's one of the known unsafe methods we currently
67696809
// mark as safe by default.
@@ -6778,20 +6818,23 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
67786818
dyn_cast<clang::CXXRecordDecl>(returnType->getDecl())) {
67796819
if (isSwiftClassType(cxxRecordReturnType))
67806820
return true;
6821+
67816822
if (hasIteratorAPIAttr(cxxRecordReturnType) ||
67826823
isIterator(cxxRecordReturnType)) {
6783-
return false;
6824+
return !parentIsSelfContained;
67846825
}
67856826

67866827
// Mark this as safe to help our diganostics down the road.
67876828
if (!cxxRecordReturnType->getDefinition()) {
67886829
return true;
67896830
}
67906831

6791-
if (!hasCustomCopyOrMoveConstructor(cxxRecordReturnType) &&
6832+
// A projection of a view type (such as a string_view) from a self
6833+
// contained parent is a proejction (unsafe).
6834+
if (!anySubobjectsSelfContained(cxxRecordReturnType) &&
67926835
!hasOwnedValueAttr(cxxRecordReturnType) &&
67936836
hasPointerInSubobjects(cxxRecordReturnType)) {
6794-
return false;
6837+
return !parentIsSelfContained;
67956838
}
67966839
}
67976840
}

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