Skip to content

Commit e640ed6

Browse files
fahadnayyarfahadnayyar
committed
[cxx-interop] Warning unannotated C++ APIs returning SWIFT_SHARED_REFERENCE types
1 parent 008e6ee commit e640ed6

File tree

7 files changed

+163
-34
lines changed

7 files changed

+163
-34
lines changed

‎include/swift/AST/DiagnosticsClangImporter.def‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,24 @@ ERROR(failed_base_method_call_synthesis,none,
258258
"failed to synthesize call to the base method %0 of type %0",
259259
(ValueDecl *, ValueDecl *))
260260

261-
ERROR(both_returns_retained_returns_unretained,none,
262-
"%0 cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes", (const clang::NamedDecl*))
261+
ERROR(both_returns_retained_returns_unretained, none,
262+
"%0 cannot be annotated with both swift_attr('returns_retained') and "
263+
"swift_attr('returns_unretained') attributes",
264+
(const clang::NamedDecl *))
265+
266+
ERROR(returns_retained_or_returns_unretained_for_non_cxx_frt_values, none,
267+
"%0 cannot be annotated with either swift_attr('returns_retained') or "
268+
"swift_attr('returns_unretained') attribute because it is not returning "
269+
"a 'SWIFT_SHARED_REFERENCE' type",
270+
(const clang::NamedDecl *))
271+
272+
// TODO: make this case an error in next cxx-interop versions rdar://138806722
273+
WARNING(
274+
no_returns_retained_returns_unretained, none,
275+
"%0 is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated "
276+
"with either swift_attr('returns_retained') or "
277+
"swift_attr('returns_unretained') attributes",
278+
(const clang::NamedDecl *))
263279

264280
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
265281
NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (const clang::NamedDecl*))

‎include/swift/ClangImporter/ClangImporter.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,9 @@ bool isCxxConstReferenceType(const clang::Type *type);
705705
/// Determine whether this typedef is a CF type.
706706
bool isCFTypeDecl(const clang::TypedefNameDecl *Decl);
707707

708+
/// Determine whether type is a c++ foreign reference type.
709+
bool isForeignReferenceTypeWithoutImmortalAttrs(const clang::QualType type);
710+
708711
/// Determine the imported CF type for the given typedef-name, or the empty
709712
/// string if this is not an imported CF type name.
710713
llvm::StringRef getCFTypeName(const clang::TypedefNameDecl *decl);

‎lib/ClangImporter/ClangImporter.cpp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7446,6 +7446,31 @@ static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
74467446
});
74477447
}
74487448

7449+
// TODO: Move all these utility functions in a new file ClangImporterUtils.h
7450+
// rdar://138803759
7451+
static bool hasImmortalAtts(const clang::RecordDecl *decl) {
7452+
return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
7453+
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
7454+
return swiftAttr->getAttribute() == "retain:immortal" ||
7455+
swiftAttr->getAttribute() == "release:immortal";
7456+
return false;
7457+
});
7458+
}
7459+
7460+
// Is this a pointer to a foreign reference type.
7461+
bool importer::isForeignReferenceTypeWithoutImmortalAttrs(const clang::QualType type) {
7462+
if (!type->isPointerType())
7463+
return false;
7464+
7465+
auto pointeeType =
7466+
dyn_cast<clang::RecordType>(type->getPointeeType().getCanonicalType());
7467+
if (pointeeType == nullptr)
7468+
return false;
7469+
7470+
return hasImportAsRefAttr(pointeeType->getDecl()) &&
7471+
!hasImmortalAtts(pointeeType->getDecl());
7472+
}
7473+
74497474
// Is this a pointer to a foreign reference type.
74507475
static bool isForeignReferenceType(const clang::QualType type) {
74517476
if (!type->isPointerType())

‎lib/ClangImporter/ImportDecl.cpp‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "swift/AST/ConformanceLookup.h"
2626
#include "swift/AST/Decl.h"
2727
#include "swift/AST/DiagnosticsClangImporter.h"
28+
#include "swift/ClangImporter/ClangImporter.h"
2829
#include "swift/AST/ExistentialLayout.h"
2930
#include "swift/AST/Expr.h"
3031
#include "swift/AST/GenericEnvironment.h"
@@ -2666,7 +2667,7 @@ namespace {
26662667
Impl.diagnoseTopLevelValue(
26672668
DeclName(Impl.SwiftContext.getIdentifier(releaseOperation.name)));
26682669
}
2669-
}else if (releaseOperation.kind ==
2670+
} else if (releaseOperation.kind ==
26702671
CustomRefCountingOperationResult::tooManyFound) {
26712672
HeaderLoc loc(decl->getLocation());
26722673
Impl.diagnose(loc,
@@ -3335,13 +3336,11 @@ namespace {
33353336
return property->getParsedAccessor(AccessorKind::Set);
33363337
}
33373338

3338-
// If a C++ decl is annotated with both swift_attr("returns_retained") and
3339-
// swift_attr("returns_unretained") then emit an error in the swift
3340-
// compiler. Note: this error is not emitted in the clang compiler because
3341-
// these attributes are used only for swift interop.
3339+
// Emit diagnostics for incorrect usage of "returns_unretained" and
3340+
// "returns_unretained" attributes
3341+
bool returnsRetainedAttrIsPresent = false;
3342+
bool returnsUnretainedAttrIsPresent = false;
33423343
if (decl->hasAttrs()) {
3343-
bool returnsRetainedAttrIsPresent = false;
3344-
bool returnsUnretainedAttrIsPresent = false;
33453344
for (const auto *attr : decl->getAttrs()) {
33463345
if (const auto *swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
33473346
if (swiftAttr->getAttribute() == "returns_unretained") {
@@ -3351,11 +3350,25 @@ namespace {
33513350
}
33523351
}
33533352
}
3353+
}
33543354

3355+
HeaderLoc loc(decl->getLocation());
3356+
if (isForeignReferenceTypeWithoutImmortalAttrs(decl->getReturnType())) {
33553357
if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
3356-
HeaderLoc loc(decl->getLocation());
33573358
Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
33583359
decl);
3360+
} else if (!returnsRetainedAttrIsPresent &&
3361+
!returnsUnretainedAttrIsPresent) {
3362+
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3363+
decl);
3364+
}
3365+
} else {
3366+
if (returnsRetainedAttrIsPresent || returnsUnretainedAttrIsPresent) {
3367+
Impl.diagnose(
3368+
loc,
3369+
diag::
3370+
returns_retained_or_returns_unretained_for_non_cxx_frt_values,
3371+
decl);
33593372
}
33603373
}
33613374

‎test/Interop/Cxx/foreign-reference/Inputs/cxx-functions-and-methods-returning-frt.h‎

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -158,28 +158,79 @@ struct
158158
__attribute__((swift_attr("returns_unretained")));
159159
};
160160

161+
// A c++ struct not annotated with SWIFT_SHARED_REFERENCE,
162+
// SWIFT_IMMORTAL_REFERENCE or SWIFT_UNSAFE_REFERENCE
161163
struct NonFRTStruct {};
162164

165+
// A c++ struct annotated with SWIFT_IMMORTAL_REFERENCE
166+
struct ImmortalRefStruct {
167+
} __attribute__((swift_attr("import_reference")))
168+
__attribute__((swift_attr("retain:immortal")))
169+
__attribute__((swift_attr("release:immortal")));
170+
171+
// A c++ struct annotated with SWIFT_UNSAFE_REFERENCE
172+
struct UnsafeRefStruct {
173+
} __attribute__((swift_attr("import_reference")))
174+
__attribute__((swift_attr("retain:immortal")))
175+
__attribute__((swift_attr("release:immortal")))
176+
__attribute__((swift_attr("unsafe")));
177+
178+
// C++ APIs returning cxx frts (for testing diagnostics)
179+
struct StructWithAPIsReturningCxxFrt {
180+
static FRTStruct *_Nonnull StaticMethodReturningCxxFrt();
181+
static FRTStruct *_Nonnull StaticMethodReturningCxxFrtWithAnnotation()
182+
__attribute__((swift_attr("returns_retained")));
183+
};
184+
185+
FRTStruct *_Nonnull global_function_returning_cxx_frt();
186+
FRTStruct *_Nonnull global_function_returning_cxx_frt_with_annotations()
187+
__attribute__((swift_attr("returns_retained")));
188+
189+
// C++ APIs returning non-cxx-frts (for testing diagnostics)
190+
struct StructWithAPIsReturningNonCxxFrt {
191+
static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrt();
192+
static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrtWithAnnotation()
193+
__attribute__((swift_attr("returns_retained")));
194+
};
195+
196+
NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt();
197+
NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt_with_annotations()
198+
__attribute__((swift_attr("returns_retained")));
199+
200+
// C++ APIs returning SWIFT_IMMORTAL_REFERENCE types (for testing diagnostics)
201+
struct StructWithAPIsReturningImmortalReference {
202+
static ImmortalRefStruct *_Nonnull StaticMethodReturningImmortalReference();
203+
static ImmortalRefStruct
204+
*_Nonnull StaticMethodReturningImmortalReferenceWithAnnotation()
205+
__attribute__((swift_attr("returns_retained")));
206+
};
207+
208+
ImmortalRefStruct *_Nonnull global_function_returning_immortal_reference();
209+
ImmortalRefStruct
210+
*_Nonnull global_function_returning_immortal_reference_with_annotations()
211+
__attribute__((swift_attr("returns_retained")));
212+
213+
// C++ APIs returning SWIFT_UNSAFE_REFERENCE types (for testing diagnostics)
214+
struct StructWithAPIsReturningUnsafeReference {
215+
static UnsafeRefStruct *_Nonnull StaticMethodReturningUnsafeReference();
216+
static UnsafeRefStruct
217+
*_Nonnull StaticMethodReturningUnsafeReferenceWithAnnotation()
218+
__attribute__((swift_attr("returns_retained")));
219+
};
220+
221+
UnsafeRefStruct *_Nonnull global_function_returning_unsafe_reference();
222+
UnsafeRefStruct
223+
*_Nonnull global_function_returning_unsafe_reference_with_annotations()
224+
__attribute__((swift_attr("returns_retained")));
225+
163226
// Global/free C++ functions returning non-FRT
164227
NonFRTStruct *_Nonnull global_function_returning_non_FRT();
165-
NonFRTStruct
166-
*_Nonnull global_function_returning_non_FRT_with_attr_returns_retained()
167-
__attribute__((swift_attr("returns_retained")));
168-
NonFRTStruct
169-
*_Nonnull global_function_returning_non_FRT_with_attr_returns_unretained()
170-
__attribute__((swift_attr("returns_unretained")));
171228
NonFRTStruct *_Nonnull global_function_returning_non_FRT_create();
172229
NonFRTStruct *_Nonnull global_function_returning_non_FRT_copy();
173230

174231
// Struct having static method returning non-FRT
175232
struct StructWithStaticMethodsReturningNonFRT {
176233
static NonFRTStruct *_Nonnull StaticMethodReturningNonFRT();
177-
static NonFRTStruct
178-
*_Nonnull StaticMethodReturningNonFRTWithAttrReturnsRetained()
179-
__attribute__((swift_attr("returns_retained")));
180-
static NonFRTStruct
181-
*_Nonnull StaticMethodReturningNonFRTWithAttrReturnsUnretained()
182-
__attribute__((swift_attr("returns_unretained")));
183234
static NonFRTStruct *_Nonnull StaticMethodReturningNonFRT_create();
184235
static NonFRTStruct *_Nonnull StaticMethodReturningNonFRT_copy();
185236
};

‎test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,36 @@ let frtLocalVar1 = global_function_returning_FRT_with_both_attrs_returns_retaine
88

99
let frtLocalVar2 = StructWithStaticMethodsReturningFRTWithBothAttributesReturnsRetainedAndReturnsUnretained.StaticMethodReturningFRT()
1010
// CHECK: error: 'StaticMethodReturningFRT' cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes
11+
12+
let frtLocalVar3 = StructWithAPIsReturningCxxFrt.StaticMethodReturningCxxFrt()
13+
// CHECK: warning: 'StaticMethodReturningCxxFrt' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes
14+
let frtLocalVar4 = StructWithAPIsReturningCxxFrt.StaticMethodReturningCxxFrtWithAnnotation()
15+
16+
let frtLocalVar5 = global_function_returning_cxx_frt()
17+
// CHECK: warning: 'global_function_returning_cxx_frt' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes
18+
let frtLocalVar6 = global_function_returning_cxx_frt_with_annotations()
19+
20+
let frtLocalVar7 = StructWithAPIsReturningNonCxxFrt.StaticMethodReturningNonCxxFrt()
21+
let frtLocalVar8 = StructWithAPIsReturningNonCxxFrt.StaticMethodReturningNonCxxFrtWithAnnotation()
22+
// CHECK: error: 'StaticMethodReturningNonCxxFrtWithAnnotation' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
23+
24+
let frtLocalVar9 = global_function_returning_non_cxx_frt()
25+
let frtLocalVar10 = global_function_returning_non_cxx_frt_with_annotations()
26+
// CHECK: error: 'global_function_returning_non_cxx_frt_with_annotations' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
27+
28+
let frtLocalVar11 = StructWithAPIsReturningImmortalReference.StaticMethodReturningImmortalReference()
29+
let frtLocalVar12 = StructWithAPIsReturningImmortalReference.StaticMethodReturningImmortalReferenceWithAnnotation()
30+
// CHECK: error: 'StaticMethodReturningImmortalReferenceWithAnnotation' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
31+
32+
let frtLocalVar13 = global_function_returning_immortal_reference()
33+
let frtLocalVar14 = global_function_returning_immortal_reference_with_annotations()
34+
// CHECK: error: 'global_function_returning_immortal_reference_with_annotations' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
35+
36+
let frtLocalVar15 = StructWithAPIsReturningUnsafeReference.StaticMethodReturningUnsafeReference()
37+
let frtLocalVar16 = StructWithAPIsReturningUnsafeReference.StaticMethodReturningUnsafeReferenceWithAnnotation()
38+
// CHECK: error: 'StaticMethodReturningUnsafeReferenceWithAnnotation' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
39+
40+
let frtLocalVar17 = global_function_returning_unsafe_reference()
41+
let frtLocalVar18 = global_function_returning_unsafe_reference_with_annotations()
42+
// CHECK: error: 'global_function_returning_unsafe_reference_with_annotations' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
43+

‎test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes.swift‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,6 @@ func testFreeFunctionsReturningNonFRT() {
167167
let frtLocalVar1 = global_function_returning_non_FRT()
168168
// CHECK: function_ref @{{.*}}global_function_returning_non_FRT{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
169169

170-
let frtLocalVar2 = global_function_returning_non_FRT_with_attr_returns_retained()
171-
// CHECK: function_ref @{{.*}}global_function_returning_non_FRT_with_attr_returns_retained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
172-
173-
let frtLocalVar3 = global_function_returning_non_FRT_with_attr_returns_unretained()
174-
// CHECK: function_ref @{{.*}}global_function_returning_non_FRT_with_attr_returns_unretained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
175-
176170
let frtLocalVar4 = global_function_returning_non_FRT_create()
177171
// CHECK: function_ref @{{.*}}global_function_returning_non_FRT_create{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
178172

@@ -184,12 +178,6 @@ func testStaticMethodsReturningNonFRT() {
184178
let frtLocalVar1 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRT()
185179
// CHECK: function_ref @{{.*}}StaticMethodReturningNonFRT{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
186180

187-
let frtLocalVar2 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRTWithAttrReturnsRetained()
188-
// CHECK: function_ref @{{.*}}StaticMethodReturningNonFRTWithAttrReturnsRetained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
189-
190-
let frtLocalVar3 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRTWithAttrReturnsUnretained()
191-
// CHECK: function_ref @{{.*}}StaticMethodReturningNonFRTWithAttrReturnsUnretained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
192-
193181
let frtLocalVar4 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRT_create()
194182
// CHECK: function_ref @{{.*}}StaticMethodReturningNonFRT_create{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
195183

0 commit comments

Comments
 (0)