Skip to content

Commit 4bab2a8

Browse files
fahadnayyarfahadnayyar
authored
Merge pull request #77397 from fahadnayyar/frt-return-unannotated-warning-improve
[cxx-interop] Improve the warnings for unannotated c++ APIs returning SWIFT_SHARED_REFERENCE types
2 parents 1b869ef + 2e50175 commit 4bab2a8

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
@@ -3333,8 +3333,15 @@ namespace {
33333333
return property->getParsedAccessor(AccessorKind::Set);
33343334
}
33353335

3336-
// Emit diagnostics for incorrect usage of "returns_unretained" and
3337-
// "returns_unretained" attributes
3336+
checkBridgingAttrs(decl);
3337+
3338+
return importFunctionDecl(decl, importedName, correctSwiftName,
3339+
std::nullopt);
3340+
}
3341+
3342+
/// Emit diagnostics for incorrect usage of SWIFT_RETURNS_RETAINED and
3343+
/// SWIFT_RETURNS_UNRETAINED
3344+
void checkBridgingAttrs(const clang::FunctionDecl *decl) {
33383345
bool returnsRetainedAttrIsPresent = false;
33393346
bool returnsUnretainedAttrIsPresent = false;
33403347
if (decl->hasAttrs()) {
@@ -3356,8 +3363,41 @@ namespace {
33563363
decl);
33573364
} else if (!returnsRetainedAttrIsPresent &&
33583365
!returnsUnretainedAttrIsPresent) {
3359-
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3360-
decl);
3366+
bool unannotatedCxxAPIWarningNeeded = false;
3367+
if (!isa<clang::CXXDeductionGuideDecl>(decl)) {
3368+
if (const auto *methodDecl = dyn_cast<clang::CXXMethodDecl>(decl)) {
3369+
// FIXME: In the future we should support SWIFT_RETURNS_RETAINED
3370+
// and SWIFT_RETURNS_UNRETAINED for overloaded C++ operators
3371+
// returning SWIFT_SHARED_REFERENCE types
3372+
if (!isa<clang::CXXConstructorDecl>(methodDecl) &&
3373+
!isa<clang::CXXDestructorDecl>(methodDecl) &&
3374+
!methodDecl->isOverloadedOperator() &&
3375+
methodDecl->isUserProvided()) {
3376+
// normal c++ member method
3377+
unannotatedCxxAPIWarningNeeded = true;
3378+
}
3379+
} else {
3380+
// global or top-level function
3381+
unannotatedCxxAPIWarningNeeded = true;
3382+
}
3383+
}
3384+
3385+
if (unannotatedCxxAPIWarningNeeded) {
3386+
HeaderLoc loc(decl->getLocation());
3387+
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3388+
decl);
3389+
}
3390+
} else if (const auto *methodDecl =
3391+
dyn_cast<clang::CXXMethodDecl>(decl)) {
3392+
// Warning for annotated overloaded C++ operators as they currently
3393+
// follow Swift method's convention and always return owned.
3394+
if (methodDecl->isOverloadedOperator()) {
3395+
Impl.diagnose(
3396+
loc,
3397+
diag::
3398+
returns_retained_returns_unretained_on_overloaded_operator,
3399+
decl);
3400+
}
33613401
}
33623402
} else {
33633403
if (returnsRetainedAttrIsPresent || returnsUnretainedAttrIsPresent) {
@@ -3368,9 +3408,6 @@ namespace {
33683408
decl);
33693409
}
33703410
}
3371-
3372-
return importFunctionDecl(decl, importedName, correctSwiftName,
3373-
std::nullopt);
33743411
}
33753412

33763413
/// 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)