Skip to content

Commit 7b8b33e

Browse files
authored
[cxx-interop] Allow SWIFT_RETURNS_(UN)RETAINED on template class member functions returning T* (#84375)
When importing C++ template classes like`Ref<T>` that have methods returning `T*`, we face the following situation when `T` is a `SWIFT_SHARED_REFERENCE` type: 1. Without `SWIFT_RETURNS_(UN)RETAINED` annotation: Swift compiler would emit a warning (currently under experimental-feature flag `WarnUnannotatedReturnOfCxxFrt`) _"cannot infer the ownership of the returned value" when T is a SWIFT_SHARED_REFERENCE type_ 2. With annotation: Compiler rejects it with this error: _"cannot be annotated... not returning a SWIFT_SHARED_REFERENCE type"_ This affects WebGPU's smart pointer types (`WTF::Ref<T>, WTF::RefPtr<T>`) and similar patterns in other C++ codebases. In this patch I am fixing the logic for diagnostic `returns_retained_or_returns_unretained_for_non_cxx_frt_values`. I'm also making it a warning instead of an error to minimize the risk, as this diagnostic has been a hindrance to the adoption of these annotations in real codebases when templated functions and types are involved. (Refer to [PR-78968](#78968)) rdar://160862498
1 parent 45a9f3a commit 7b8b33e

File tree

6 files changed

+71
-10
lines changed

6 files changed

+71
-10
lines changed

include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,8 @@ ERROR(both_returns_retained_returns_unretained, none,
299299
ERROR(redundant_conformance_protocol,none,
300300
"redundant conformance of %0 to protocol '%1'", (Type, StringRef))
301301

302-
ERROR(returns_retained_or_returns_unretained_for_non_cxx_frt_values, none,
303-
"%0 cannot be annotated with either SWIFT_RETURNS_RETAINED or "
302+
WARNING(returns_retained_or_returns_unretained_for_non_cxx_frt_values, none,
303+
"%0 should not be annotated with SWIFT_TURNS_RETAINED or "
304304
"SWIFT_RETURNS_UNRETAINED because it is not returning "
305305
"a SWIFT_SHARED_REFERENCE type",
306306
(const clang::NamedDecl *))

lib/ClangImporter/ImportDecl.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3620,10 +3620,17 @@ namespace {
36203620
} else {
36213621
if (attrInfo.hasRetainAttr()) {
36223622
if (const auto *functionDecl = dyn_cast<clang::FunctionDecl>(decl)) {
3623-
if (functionDecl->isTemplateInstantiation()) {
3623+
// Skip diagnostics for template instantiations and template-dependent
3624+
// return types. We cannot determine at import time whether T or T* will
3625+
// be instantiated as a SWIFT_SHARED_REFERENCE type or not, so we avoid
3626+
// emitting potentially incorrect warnings for these cases.
3627+
if (functionDecl->isTemplateInstantiation() ||
3628+
functionDecl->getReturnType()->isInstantiationDependentType() ||
3629+
functionDecl->getReturnType()->isDependentType()) {
36243630
return;
36253631
}
36263632
}
3633+
36273634
Impl.diagnose(
36283635
loc,
36293636
diag::

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

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,38 +191,38 @@ FRTStruct *_Nonnull global_function_returning_cxx_frt_with_annotations()
191191
// C++ APIs returning non-cxx-frts (for testing diagnostics)
192192
struct StructWithAPIsReturningNonCxxFrt {
193193
static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrt();
194-
static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrtWithAnnotation() // expected-error {{'StaticMethodReturningNonCxxFrtWithAnnotation' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
194+
static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrtWithAnnotation() // expected-warning {{'StaticMethodReturningNonCxxFrtWithAnnotation' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
195195
__attribute__((swift_attr("returns_retained")));
196196
};
197197

198198
NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt();
199-
NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt_with_annotations() // expected-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}}
199+
NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt_with_annotations() // expected-warning {{'global_function_returning_non_cxx_frt_with_annotations' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
200200
__attribute__((swift_attr("returns_retained")));
201201

202202
// C++ APIs returning SWIFT_IMMORTAL_REFERENCE types (for testing diagnostics)
203203
struct StructWithAPIsReturningImmortalReference {
204204
static ImmortalRefStruct *_Nonnull StaticMethodReturningImmortalReference();
205205
static ImmortalRefStruct
206-
*_Nonnull StaticMethodReturningImmortalReferenceWithAnnotation() // expected-error {{'StaticMethodReturningImmortalReferenceWithAnnotation' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
206+
*_Nonnull StaticMethodReturningImmortalReferenceWithAnnotation() // expected-warning {{'StaticMethodReturningImmortalReferenceWithAnnotation' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
207207
__attribute__((swift_attr("returns_retained")));
208208
};
209209

210210
ImmortalRefStruct *_Nonnull global_function_returning_immortal_reference();
211211
ImmortalRefStruct
212-
*_Nonnull global_function_returning_immortal_reference_with_annotations() // expected-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}}
212+
*_Nonnull global_function_returning_immortal_reference_with_annotations() // expected-warning {{'global_function_returning_immortal_reference_with_annotations' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
213213
__attribute__((swift_attr("returns_retained")));
214214

215215
// C++ APIs returning SWIFT_UNSAFE_REFERENCE types (for testing diagnostics)
216216
struct StructWithAPIsReturningUnsafeReference {
217217
static UnsafeRefStruct *_Nonnull StaticMethodReturningUnsafeReference();
218218
static UnsafeRefStruct
219-
*_Nonnull StaticMethodReturningUnsafeReferenceWithAnnotation() // expected-error {{'StaticMethodReturningUnsafeReferenceWithAnnotation' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
219+
*_Nonnull StaticMethodReturningUnsafeReferenceWithAnnotation() // expected-warning {{'StaticMethodReturningUnsafeReferenceWithAnnotation' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
220220
__attribute__((swift_attr("returns_retained")));
221221
};
222222

223223
UnsafeRefStruct *_Nonnull global_function_returning_unsafe_reference();
224224
UnsafeRefStruct
225-
*_Nonnull global_function_returning_unsafe_reference_with_annotations() // expected-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}}
225+
*_Nonnull global_function_returning_unsafe_reference_with_annotations() // expected-warning {{'global_function_returning_unsafe_reference_with_annotations' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
226226
__attribute__((swift_attr("returns_retained")));
227227

228228
// Global/free C++ functions returning non-FRT
@@ -479,4 +479,40 @@ void retain_SourceLocCacheB(SourceLocationCaching::TypeB *) {}
479479
void release_SourceLocCacheB(SourceLocationCaching::TypeB *) {}
480480

481481

482+
template <typename T>
483+
class RefTemplate {
484+
public:
485+
// These should now be importable with the fix
486+
T* ptr() const __attribute__((swift_attr("returns_retained"))) {
487+
return nullptr;
488+
}
489+
490+
T* get() const __attribute__((swift_attr("returns_unretained"))) {
491+
return nullptr;
492+
}
493+
494+
T* value() const {
495+
return nullptr;
496+
}
497+
};
498+
499+
template <>
500+
class RefTemplate<FRTStruct> {
501+
public:
502+
FRTStruct* ptr() const __attribute__((swift_attr("returns_retained"))) {
503+
return new FRTStruct;
504+
}
505+
506+
FRTStruct* get() const __attribute__((swift_attr("returns_unretained"))) {
507+
static FRTStruct instance;
508+
return &instance;
509+
}
510+
511+
FRTStruct* value() const { // expected-note {{'value()' is defined here}}
512+
return new FRTStruct;
513+
}
514+
};
515+
516+
using FRTStructRef = RefTemplate<FRTStruct>;
517+
482518
SWIFT_END_NULLABILITY_ANNOTATIONS

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,8 @@ let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefault() // expected-wa
4747
let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefaultUnretained()
4848
let _ = SourceLocationCaching.FactoryA.make() // expected-warning {{cannot infer the ownership of the returned value, annotate 'make()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}}
4949
let _ = SourceLocationCaching.FactoryB.make() // expected-warning {{cannot infer the ownership of the returned value, annotate 'make()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}}
50+
51+
let refTemplate = FRTStructRef()
52+
let templatePtr = refTemplate.ptr()
53+
let templateGet = refTemplate.get()
54+
let templateValue = refTemplate.value() // expected-warning {{cannot infer the ownership of the returned value, annotate 'value()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,16 @@ func testDefaultOwnershipAnnotation() {
276276
let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefaultUnretained()
277277
// CHECK: function_ref {{.*}}createDerivedTypeNonDefaultUnretained{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedTypeNonDefault
278278
}
279+
280+
func testTemplateMemberFunctions() {
281+
let refTemplate = FRTStructRef()
282+
283+
let ptr = refTemplate.ptr()
284+
// CHECK: function_ref {{.*}}ptr{{.*}} : $@convention(cxx_method) (@in_guaranteed RefTemplate<FRTStruct>) -> @owned FRTStruct
285+
286+
let get = refTemplate.get()
287+
// CHECK: function_ref {{.*}}get{{.*}} : $@convention(cxx_method) (@in_guaranteed RefTemplate<FRTStruct>) -> FRTStruct
288+
289+
let value = refTemplate.value()
290+
// CHECK: function_ref {{.*}}value{{.*}} : $@convention(cxx_method) (@in_guaranteed RefTemplate<FRTStruct>) -> FRTStruct
291+
}

test/Interop/Cxx/objc-correctness/Inputs/cxx-frt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ void releaseCxxRefType(CxxRefType *_Nonnull b) {}
2424
+ (struct CxxRefType *)objCMethodReturningFRTBothAnnotations // expected-error {{'objCMethodReturningFRTBothAnnotations' cannot be annotated with both SWIFT_RETURNS_RETAINED and SWIFT_RETURNS_UNRETAINED}}
2525
__attribute__((swift_attr("returns_unretained")))
2626
__attribute__((swift_attr("returns_retained")));
27-
+ (struct CxxValType *)objCMethodReturningNonCxxFrtAnannotated // expected-error {{'objCMethodReturningNonCxxFrtAnannotated' cannot be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
27+
+ (struct CxxValType *)objCMethodReturningNonCxxFrtAnannotated // expected-warning {{'objCMethodReturningNonCxxFrtAnannotated' should not be annotated with SWIFT_TURNS_RETAINED or SWIFT_RETURNS_UNRETAINED because it is not returning a SWIFT_SHARED_REFERENCE type}}
2828
__attribute__((swift_attr("returns_retained")));
2929

3030
@end

0 commit comments

Comments
 (0)