Skip to content

Commit 2e50175

Browse files
fahadnayyarfahadnayyar
committed
[cxx-interop] Improve the warnings for unannotated c++ APIs returning SWIFT_SHARED_REFERENCE types
1 parent c690fef commit 2e50175

File tree

5 files changed

+156
-32
lines changed

5 files changed

+156
-32
lines changed

‎include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -259,23 +259,28 @@ ERROR(failed_base_method_call_synthesis,none,
259259
(ValueDecl *, ValueDecl *))
260260

261261
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",
262+
"%0 cannot be annotated with both SWIFT_RETURNS_RETAINED and "
263+
"SWIFT_RETURNS_UNRETAINED",
264264
(const clang::NamedDecl *))
265265

266266
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",
267+
"%0 cannot be annotated with either SWIFT_RETURNS_RETAINED or "
268+
"SWIFT_RETURNS_UNRETAINED because it is not returning "
269+
"a SWIFT_SHARED_REFERENCE type",
270270
(const clang::NamedDecl *))
271271

272272
// 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 *))
273+
WARNING(no_returns_retained_returns_unretained, none,
274+
"%0 should be annotated with either SWIFT_RETURNS_RETAINED or "
275+
"SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE",
276+
(const clang::NamedDecl *))
277+
278+
WARNING(returns_retained_returns_unretained_on_overloaded_operator, none,
279+
"SWIFT_RETURNS_RETAINED and SWIFT_RETURNS_UNRETAINED is not supported "
280+
"yet for overloaded C++ %0. Overloaded C++ operators always "
281+
"return "
282+
"SWIFT_SHARED_REFERENCE types as owned ",
283+
(const clang::NamedDecl *))
279284

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

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3336,8 +3336,15 @@ namespace {
33363336
return property->getParsedAccessor(AccessorKind::Set);
33373337
}
33383338

3339-
// Emit diagnostics for incorrect usage of "returns_unretained" and
3340-
// "returns_unretained" attributes
3339+
checkBridgingAttrs(decl);
3340+
3341+
return importFunctionDecl(decl, importedName, correctSwiftName,
3342+
std::nullopt);
3343+
}
3344+
3345+
/// Emit diagnostics for incorrect usage of SWIFT_RETURNS_RETAINED and
3346+
/// SWIFT_RETURNS_UNRETAINED
3347+
void checkBridgingAttrs(const clang::FunctionDecl *decl) {
33413348
bool returnsRetainedAttrIsPresent = false;
33423349
bool returnsUnretainedAttrIsPresent = false;
33433350
if (decl->hasAttrs()) {
@@ -3359,8 +3366,41 @@ namespace {
33593366
decl);
33603367
} else if (!returnsRetainedAttrIsPresent &&
33613368
!returnsUnretainedAttrIsPresent) {
3362-
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3363-
decl);
3369+
bool unannotatedCxxAPIWarningNeeded = false;
3370+
if (!isa<clang::CXXDeductionGuideDecl>(decl)) {
3371+
if (const auto *methodDecl = dyn_cast<clang::CXXMethodDecl>(decl)) {
3372+
// FIXME: In the future we should support SWIFT_RETURNS_RETAINED
3373+
// and SWIFT_RETURNS_UNRETAINED for overloaded C++ operators
3374+
// returning SWIFT_SHARED_REFERENCE types
3375+
if (!isa<clang::CXXConstructorDecl>(methodDecl) &&
3376+
!isa<clang::CXXDestructorDecl>(methodDecl) &&
3377+
!methodDecl->isOverloadedOperator() &&
3378+
methodDecl->isUserProvided()) {
3379+
// normal c++ member method
3380+
unannotatedCxxAPIWarningNeeded = true;
3381+
}
3382+
} else {
3383+
// global or top-level function
3384+
unannotatedCxxAPIWarningNeeded = true;
3385+
}
3386+
}
3387+
3388+
if (unannotatedCxxAPIWarningNeeded) {
3389+
HeaderLoc loc(decl->getLocation());
3390+
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3391+
decl);
3392+
}
3393+
} else if (const auto *methodDecl =
3394+
dyn_cast<clang::CXXMethodDecl>(decl)) {
3395+
// Warning for annotated overloaded C++ operators as they currently
3396+
// follow Swift method's convention and always return owned.
3397+
if (methodDecl->isOverloadedOperator()) {
3398+
Impl.diagnose(
3399+
loc,
3400+
diag::
3401+
returns_retained_returns_unretained_on_overloaded_operator,
3402+
decl);
3403+
}
33643404
}
33653405
} else {
33663406
if (returnsRetainedAttrIsPresent || returnsUnretainedAttrIsPresent) {
@@ -3371,9 +3411,6 @@ namespace {
33713411
decl);
33723412
}
33733413
}
3374-
3375-
return importFunctionDecl(decl, importedName, correctSwiftName,
3376-
std::nullopt);
33773414
}
33783415

33793416
/// Handles special functions such as subscripts and dereference operators.

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#pragma once
2+
#include <functional>
23

34
// FRT or SWIFT_SHARED_REFERENCE type
45
struct FRTStruct {
@@ -280,3 +281,48 @@ template <typename T>
280281
FRTStruct
281282
*_Nonnull global_templated_function_returning_FRT_create_with_attr_returns_unretained(
282283
T a) __attribute__((swift_attr("returns_unretained")));
284+
285+
template <typename T>
286+
T global_function_returning_templated_retrun_frt(T);
287+
288+
template <typename T>
289+
T global_function_returning_templated_retrun_frt_owned(T)
290+
__attribute__((swift_attr("returns_retained")));
291+
292+
struct __attribute__((swift_attr("import_reference")))
293+
__attribute__((swift_attr("retain:retain_FRTOverloadedOperators")))
294+
__attribute__((swift_attr(
295+
"release:release_FRTOverloadedOperators"))) FRTOverloadedOperators {
296+
297+
public:
298+
FRTOverloadedOperators *_Nonnull
299+
operator+(const FRTOverloadedOperators &other)
300+
__attribute__((swift_attr("returns_unretained")));
301+
FRTOverloadedOperators *_Nonnull
302+
operator-(const FRTOverloadedOperators &other);
303+
};
304+
305+
FRTOverloadedOperators *_Nonnull returnFRTOverloadedOperators();
306+
307+
void retain_FRTOverloadedOperators(FRTOverloadedOperators *_Nonnull v);
308+
void release_FRTOverloadedOperators(FRTOverloadedOperators *_Nonnull v);
309+
310+
using FunctionVoidToFRTStruct =
311+
std::function<FRTStruct *_Nonnull(void)> __attribute__((
312+
swift_attr("returns_retained")));
313+
314+
struct Base {
315+
public:
316+
virtual FRTStruct *_Nonnull VirtualMethodReturningFRTOwned()
317+
__attribute__((swift_attr("returns_retained")));
318+
virtual FRTStruct *_Nonnull VirtualMethodReturningFRTUnowned()
319+
__attribute__((swift_attr("returns_unretained")));
320+
};
321+
322+
struct Derived : Base {
323+
public:
324+
FRTStruct *_Nonnull VirtualMethodReturningFRTOwned() override
325+
__attribute__((swift_attr("returns_retained")));
326+
FRTStruct *_Nonnull VirtualMethodReturningFRTUnowned() override
327+
__attribute__((swift_attr("returns_unretained")));
328+
};

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

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,42 +2,54 @@
22
// RUN: not %target-swift-frontend -typecheck -I %S/Inputs %s -cxx-interoperability-mode=upcoming-swift 2>&1 | %FileCheck %s
33

44
import FunctionsAndMethodsReturningFRT
5+
import CxxStdlib
56

67
let frtLocalVar1 = global_function_returning_FRT_with_both_attrs_returns_retained_returns_unretained()
7-
// CHECK: error: 'global_function_returning_FRT_with_both_attrs_returns_retained_returns_unretained' cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes
8+
// CHECK: error: 'global_function_returning_FRT_with_both_attrs_returns_retained_returns_unretained' cannot be annotated with both SWIFT_RETURNS_RETAINED and SWIFT_RETURNS_UNRETAINED
89

910
let frtLocalVar2 = StructWithStaticMethodsReturningFRTWithBothAttributesReturnsRetainedAndReturnsUnretained.StaticMethodReturningFRT()
10-
// CHECK: error: 'StaticMethodReturningFRT' cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes
11-
11+
// CHECK: error: 'StaticMethodReturningFRT' cannot be annotated with both SWIFT_RETURNS_RETAINED and SWIFT_RETURNS_UNRETAINED
1212
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
13+
// CHECK: warning: 'StaticMethodReturningCxxFrt' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE
1414
let frtLocalVar4 = StructWithAPIsReturningCxxFrt.StaticMethodReturningCxxFrtWithAnnotation()
1515

1616
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
17+
// CHECK: warning: 'global_function_returning_cxx_frt' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE
1818
let frtLocalVar6 = global_function_returning_cxx_frt_with_annotations()
1919

2020
let frtLocalVar7 = StructWithAPIsReturningNonCxxFrt.StaticMethodReturningNonCxxFrt()
2121
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
22+
// CHECK: error: 'StaticMethodReturningNonCxxFrtWithAnnotation' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type
2323

2424
let frtLocalVar9 = global_function_returning_non_cxx_frt()
2525
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
26+
// CHECK: error: 'global_function_returning_non_cxx_frt_with_annotations' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type
2727

2828
let frtLocalVar11 = StructWithAPIsReturningImmortalReference.StaticMethodReturningImmortalReference()
2929
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
30+
// CHECK: error: 'StaticMethodReturningImmortalReferenceWithAnnotation' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type
3131

3232
let frtLocalVar13 = global_function_returning_immortal_reference()
3333
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
34+
// CHECK: error: 'global_function_returning_immortal_reference_with_annotations' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type
3535

3636
let frtLocalVar15 = StructWithAPIsReturningUnsafeReference.StaticMethodReturningUnsafeReference()
3737
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
38+
// CHECK: error: 'StaticMethodReturningUnsafeReferenceWithAnnotation' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type
3939

4040
let frtLocalVar17 = global_function_returning_unsafe_reference()
4141
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-
42+
// CHECK: error: 'global_function_returning_unsafe_reference_with_annotations' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type
43+
44+
let x = returnFRTOverloadedOperators()
45+
// CHECK: warning: 'returnFRTOverloadedOperators' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE
46+
let y = returnFRTOverloadedOperators()
47+
// CHECK: warning: 'returnFRTOverloadedOperators' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE
48+
let z = x + y
49+
// CHECK: warning: SWIFT_RETURNS_RETAINED and SWIFT_RETURNS_UNRETAINED is not supported yet for overloaded C++ 'operator+'. Overloaded C++ operators always return SWIFT_SHARED_REFERENCE types as owned
50+
let w = x - y
51+
// CHECK-NOT: warning: 'operator-' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE
52+
53+
let f = FunctionVoidToFRTStruct()
54+
let frt = f()
55+
// CHECK-NOT: warning: 'operator()' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -diagnostic-style llvm %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
1+
// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -disable-availability-checking -diagnostic-style llvm %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
22

33
import FunctionsAndMethodsReturningFRT
44

@@ -185,7 +185,7 @@ func testStaticMethodsReturningNonFRT() {
185185
// CHECK: function_ref @{{.*}}StaticMethodReturningNonFRT_copy{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>
186186
}
187187

188-
func testtFreeFunctionsTemplated() {
188+
func testtFreeFunctionsTemplated(frt : FRTStruct) {
189189
let frtLocalVar1 : Int = 1;
190190

191191
let frtLocalVar2 = global_templated_function_returning_FRT(frtLocalVar1)
@@ -221,4 +221,28 @@ func testtFreeFunctionsTemplated() {
221221
let frtLocalVar12 = global_templated_function_returning_FRT_create_with_attr_returns_unretained(frtLocalVar1)
222222
// CHECK: function_ref @{{.*}}global_templated_function_returning_FRT_create_with_attr_returns_unretained{{.*}} : $@convention(c) (Int) -> FRTStruct
223223

224+
let frtLocalVar13 = global_function_returning_templated_retrun_frt(frt)
225+
// CHECK: function_ref @{{.*}}global_function_returning_templated_retrun_frt{{.*}} : $@convention(c) (FRTStruct) -> FRTStruct
226+
227+
let frtLocalVar14 = global_function_returning_templated_retrun_frt_owned(frt)
228+
// CHECK: function_ref @{{.*}}global_function_returning_templated_retrun_frt_owned{{.*}} : $@convention(c) (FRTStruct) -> @owned FRTStruct
229+
224230
}
231+
232+
func testVirtualMethods(base: Base, derived: Derived) {
233+
var mutableBase = base
234+
var mutableDerived = derived
235+
236+
var frt1 = mutableBase.VirtualMethodReturningFRTUnowned()
237+
// CHECK: function_ref @{{.*}}VirtualMethodReturningFRTUnowned{{.*}} : $@convention(cxx_method) (@inout Base) -> FRTStruct
238+
239+
var frt2 = mutableDerived.VirtualMethodReturningFRTUnowned()
240+
// CHECK: function_ref @{{.*}}VirtualMethodReturningFRTUnowned{{.*}} : $@convention(cxx_method) (@inout Derived) -> FRTStruct
241+
242+
var frt3 = mutableBase.VirtualMethodReturningFRTOwned()
243+
// CHECK: function_ref @{{.*}}VirtualMethodReturningFRTOwned{{.*}} : $@convention(cxx_method) (@inout Base) -> @owned FRTStruct
244+
245+
var frt4 = mutableDerived.VirtualMethodReturningFRTOwned()
246+
// CHECK: function_ref @{{.*}}VirtualMethodReturningFRTOwned{{.*}} : $@convention(cxx_method) (@inout Derived) -> @owned FRTStruct
247+
}
248+

0 commit comments

Comments
 (0)