Skip to content

Commit 56304f0

Browse files
fahadnayyarfahadnayyar
authored
Merge pull request #77998 from swiftlang/fahadnayyar/6.1-frt-return-unannotated-warning-improve
🍒 [cxx-interop] Improve the warnings for unannotated c++ APIs returning SWIFT_SHARED_REFERENCE types
2 parents 0d34057 + 7dc2caf commit 56304f0

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
@@ -263,23 +263,28 @@ ERROR(failed_base_method_call_synthesis,none,
263263
(ValueDecl *, ValueDecl *))
264264

265265
ERROR(both_returns_retained_returns_unretained, none,
266-
"%0 cannot be annotated with both swift_attr('returns_retained') and "
267-
"swift_attr('returns_unretained') attributes",
266+
"%0 cannot be annotated with both SWIFT_RETURNS_RETAINED and "
267+
"SWIFT_RETURNS_UNRETAINED",
268268
(const clang::NamedDecl *))
269269

270270
ERROR(returns_retained_or_returns_unretained_for_non_cxx_frt_values, none,
271-
"%0 cannot be annotated with either swift_attr('returns_retained') or "
272-
"swift_attr('returns_unretained') attribute because it is not returning "
273-
"a 'SWIFT_SHARED_REFERENCE' type",
271+
"%0 cannot be annotated with either SWIFT_RETURNS_RETAINED or "
272+
"SWIFT_RETURNS_UNRETAINED because it is not returning "
273+
"a SWIFT_SHARED_REFERENCE type",
274274
(const clang::NamedDecl *))
275275

276276
// TODO: make this case an error in next cxx-interop versions rdar://138806722
277-
WARNING(
278-
no_returns_retained_returns_unretained, none,
279-
"%0 is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated "
280-
"with either swift_attr('returns_retained') or "
281-
"swift_attr('returns_unretained') attributes",
282-
(const clang::NamedDecl *))
277+
WARNING(no_returns_retained_returns_unretained, none,
278+
"%0 should be annotated with either SWIFT_RETURNS_RETAINED or "
279+
"SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE",
280+
(const clang::NamedDecl *))
281+
282+
WARNING(returns_retained_returns_unretained_on_overloaded_operator, none,
283+
"SWIFT_RETURNS_RETAINED and SWIFT_RETURNS_UNRETAINED is not supported "
284+
"yet for overloaded C++ %0. Overloaded C++ operators always "
285+
"return "
286+
"SWIFT_SHARED_REFERENCE types as owned ",
287+
(const clang::NamedDecl *))
283288

284289
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
285290
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
@@ -3348,8 +3348,15 @@ namespace {
33483348
return property->getParsedAccessor(AccessorKind::Set);
33493349
}
33503350

3351-
// Emit diagnostics for incorrect usage of "returns_unretained" and
3352-
// "returns_unretained" attributes
3351+
checkBridgingAttrs(decl);
3352+
3353+
return importFunctionDecl(decl, importedName, correctSwiftName,
3354+
std::nullopt);
3355+
}
3356+
3357+
/// Emit diagnostics for incorrect usage of SWIFT_RETURNS_RETAINED and
3358+
/// SWIFT_RETURNS_UNRETAINED
3359+
void checkBridgingAttrs(const clang::FunctionDecl *decl) {
33533360
bool returnsRetainedAttrIsPresent = false;
33543361
bool returnsUnretainedAttrIsPresent = false;
33553362
if (decl->hasAttrs()) {
@@ -3371,8 +3378,41 @@ namespace {
33713378
decl);
33723379
} else if (!returnsRetainedAttrIsPresent &&
33733380
!returnsUnretainedAttrIsPresent) {
3374-
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3375-
decl);
3381+
bool unannotatedCxxAPIWarningNeeded = false;
3382+
if (!isa<clang::CXXDeductionGuideDecl>(decl)) {
3383+
if (const auto *methodDecl = dyn_cast<clang::CXXMethodDecl>(decl)) {
3384+
// FIXME: In the future we should support SWIFT_RETURNS_RETAINED
3385+
// and SWIFT_RETURNS_UNRETAINED for overloaded C++ operators
3386+
// returning SWIFT_SHARED_REFERENCE types
3387+
if (!isa<clang::CXXConstructorDecl>(methodDecl) &&
3388+
!isa<clang::CXXDestructorDecl>(methodDecl) &&
3389+
!methodDecl->isOverloadedOperator() &&
3390+
methodDecl->isUserProvided()) {
3391+
// normal c++ member method
3392+
unannotatedCxxAPIWarningNeeded = true;
3393+
}
3394+
} else {
3395+
// global or top-level function
3396+
unannotatedCxxAPIWarningNeeded = true;
3397+
}
3398+
}
3399+
3400+
if (unannotatedCxxAPIWarningNeeded) {
3401+
HeaderLoc loc(decl->getLocation());
3402+
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
3403+
decl);
3404+
}
3405+
} else if (const auto *methodDecl =
3406+
dyn_cast<clang::CXXMethodDecl>(decl)) {
3407+
// Warning for annotated overloaded C++ operators as they currently
3408+
// follow Swift method's convention and always return owned.
3409+
if (methodDecl->isOverloadedOperator()) {
3410+
Impl.diagnose(
3411+
loc,
3412+
diag::
3413+
returns_retained_returns_unretained_on_overloaded_operator,
3414+
decl);
3415+
}
33763416
}
33773417
} else {
33783418
if (returnsRetainedAttrIsPresent || returnsUnretainedAttrIsPresent) {
@@ -3383,9 +3423,6 @@ namespace {
33833423
decl);
33843424
}
33853425
}
3386-
3387-
return importFunctionDecl(decl, importedName, correctSwiftName,
3388-
std::nullopt);
33893426
}
33903427

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