Skip to content

Commit b311c63

Browse files
fahadnayyarfahadnayyar
authored
[cxx-interop] Remove SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT annotation (swiftlang#81329)
This patch removes the `SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT` annotation while maintaining the support for `SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT`. These type-level annotations were initially introduced in [PR-81093](swiftlang#81093) to reduce the annotation burden in large C++ codebases where many C++ APIs returning `SWIFT_SHARED_REFERENCE` types are exposed to Swift. ### Motivation The original goal was to make C++ interop more ergonomic by allowing type-level defaults for ownership conventions for`SWIFT_SHARED_REFERENCE` types . However, defaulting to retained return values (+1) seems to be problematic and poses memory safety risks. ### Why we’re removing `SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT` - **Memory safety risks:** Defaulting to retained can potentially lead to use-after-free bugs when the API implementation actually returns `unowned` (`+0`). These errors are subtle and can be hard to debug or discover, particularly in the absence of explicit API-level `SWIFT_RETURNS_(UN)RETAINED` annotations. - **Risky transitive behavior:** If a `SWIFT_SHARED_REFERENCE` type is annotated with `SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT`, any new C++ API returning this type will inherit the retained behavior by default—even if the API's actual return behavior is unretained. Unless explicitly overridden with `SWIFT_RETURNS_UNRETAINED`, this can introduce a silent mismatch in ownership expectations and lead to use-after-free bugs. This is especially risky in large or evolving codebases where such defaults may be overlooked. - **Simpler multiple inheritance semantics:** With only one type-level default (`SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT`), we avoid complications that can arise when multiple base classes specify conflicting ownership defaults. This simplifies reasoning about behavior in class hierarchies and avoids ambiguity when Swift determines the ownership convention for inherited APIs. ### Why we’re keeping `SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT` - It still enables projects to suppress warnings for unannotated C++ APIs returning `SWIFT_SHARED_REFERENCE` types, helping to reduce noise while maintaining clarity. - It encourages explicitness for retained behavior. Developers must annotate retained return values with `SWIFT_RETURNS_RETAINED`, making ownership intent clearer and safer. - The worst-case outcome of assuming unretained when the return is actually retained is a memory leak, which is more tolerable and easier to debug than a use-after-free. - Having a single default mechanism improves clarity for documentation, diagnostics, and long-term maintenance of Swift/C++ interop code.
1 parent 93882f2 commit b311c63

File tree

6 files changed

+39
-94
lines changed

6 files changed

+39
-94
lines changed

‎lib/ClangImporter/ClangImporter.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8809,8 +8809,7 @@ swift::importer::getCxxRefConventionWithAttrs(const clang::Decl *decl) {
88098809
if (const clang::RecordDecl *record =
88108810
ptrType->getPointeeType()->getAsRecordDecl()) {
88118811
return matchSwiftAttrConsideringInheritance<RC>(
8812-
record, {{"returned_as_unretained_by_default", RC::Unowned},
8813-
{"returned_as_retained_by_default", RC::Owned}});
8812+
record, {{"returned_as_unretained_by_default", RC::Unowned}});
88148813
}
88158814
}
88168815

‎lib/ClangImporter/ImportDecl.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3656,8 +3656,7 @@ namespace {
36563656
}
36573657

36583658
if (importer::matchSwiftAttrOnRecordPtr<bool>(
3659-
retType, {{"returned_as_retained_by_default", true},
3660-
{"returned_as_unretained_by_default", true}})) {
3659+
retType, {{"returned_as_unretained_by_default", true}})) {
36613660
unannotatedAPIWarningNeeded = false;
36623661
}
36633662

‎lib/ClangImporter/SwiftBridging/swift/bridging‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -191,23 +191,6 @@
191191
#define SWIFT_RETURNS_UNRETAINED \
192192
__attribute__((swift_attr("returns_unretained")))
193193

194-
/// Applied to a C++ foreign reference type annotated with
195-
/// SWIFT_SHARED_REFERENCE. Indicates that C++ APIs returning this type are
196-
/// assumed to return an owned (+1) value by default, unless explicitly annotated with
197-
/// SWIFT_RETURNS_UNRETAINED.
198-
///
199-
/// For example:
200-
/// ```c++
201-
/// struct SWIFT_SHARED_REFERENCE(retainFoo, releaseFoo)
202-
/// SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT
203-
/// Foo { ... };
204-
/// ```
205-
///
206-
/// In Swift, C++ APIs returning `Foo*` will be assumed to return an owned
207-
/// value.
208-
#define SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT \
209-
__attribute__((swift_attr("returned_as_retained_by_default")))
210-
211194
/// Applied to a C++ foreign reference type annotated with
212195
/// SWIFT_SHARED_REFERENCE. Indicates that C++ APIs returning this type are
213196
/// assumed to return an unowned (+0) value by default, unless explicitly annotated

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

Lines changed: 15 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -331,13 +331,6 @@ struct Derived : Base {
331331
SWIFT_BEGIN_NULLABILITY_ANNOTATIONS
332332

333333
namespace DefaultOwnershipConventionOnCXXForeignRefType {
334-
struct __attribute__((swift_attr("import_reference")))
335-
__attribute__((swift_attr("retain:defRetain1")))
336-
__attribute__((swift_attr("release:defRelease1")))
337-
__attribute__((swift_attr("returned_as_retained_by_default"))) RefTyDefRetained {};
338-
339-
RefTyDefRetained *returnRefTyDefRetained() { return new RefTyDefRetained(); }
340-
341334
struct __attribute__((swift_attr("import_reference")))
342335
__attribute__((swift_attr("retain:defRetain2")))
343336
__attribute__((swift_attr("release:defRelease2")))
@@ -348,11 +341,6 @@ RefTyDefUnretained *returnRefTyDefUnretained() {
348341
}
349342
} // namespace DefaultOwnershipConventionOnCXXForeignRefType
350343

351-
void defRetain1(
352-
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefRetained *v) {};
353-
void defRelease1(
354-
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefRetained *v) {};
355-
356344
void defRetain2(
357345
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefUnretained *v) {};
358346
void defRelease2(
@@ -372,25 +360,11 @@ RefTyDefUnretained *returnRefTyDefUnretainedAnnotatedRetained()
372360
return new RefTyDefUnretained();
373361
}
374362

375-
struct __attribute__((swift_attr("import_reference")))
376-
__attribute__((swift_attr("retain:defaultRetain2")))
377-
__attribute__((swift_attr("release:defaultRelease2")))
378-
__attribute__((swift_attr("returned_as_retained_by_default"))) RefTyDefRetained {};
379-
380-
RefTyDefRetained *returnRefTyDefRetained() { return new RefTyDefRetained(); }
381-
RefTyDefRetained *returnRefTyDefRetainedAnnotatedUnRetained()
382-
__attribute__((swift_attr("returns_unretained"))) {
383-
return new RefTyDefRetained();
384-
}
385-
386363
} // namespace FunctionAnnotationHasPrecedence
387364

388365
void defaultRetain1(FunctionAnnotationHasPrecedence::RefTyDefUnretained *v) {};
389366
void defaultRelease1(FunctionAnnotationHasPrecedence::RefTyDefUnretained *v) {};
390367

391-
void defaultRetain2(FunctionAnnotationHasPrecedence::RefTyDefRetained *v) {};
392-
void defaultRelease2(FunctionAnnotationHasPrecedence::RefTyDefRetained *v) {};
393-
394368
namespace DefaultOwnershipSuppressUnannotatedAPIWarning {
395369

396370
struct __attribute__((swift_attr("import_reference")))
@@ -402,26 +376,26 @@ RefType *returnRefType() { return new RefType(); } // expected-warning {{'return
402376
struct __attribute__((swift_attr("import_reference")))
403377
__attribute__((swift_attr("retain:dretain")))
404378
__attribute__((swift_attr("release:drelease")))
405-
__attribute__((swift_attr("returned_as_retained_by_default"))) RefTyDefRetained {};
379+
__attribute__((swift_attr("returned_as_unretained_by_default"))) RefTyDefUnretained {};
406380

407-
RefTyDefRetained *returnRefTyDefRetained() { return new RefTyDefRetained(); }
381+
RefTyDefUnretained *returnRefTyDefUnretainedd() { return new RefTyDefUnretained(); }
408382

409383
} // namespace DefaultOwnershipSuppressUnannotatedAPIWarning
410384

411385
void rretain(DefaultOwnershipSuppressUnannotatedAPIWarning::RefType *v) {};
412386
void rrelease(DefaultOwnershipSuppressUnannotatedAPIWarning::RefType *v) {};
413387

414388
void dretain(
415-
DefaultOwnershipSuppressUnannotatedAPIWarning::RefTyDefRetained *v) {};
389+
DefaultOwnershipSuppressUnannotatedAPIWarning::RefTyDefUnretained *v) {};
416390
void drelease(
417-
DefaultOwnershipSuppressUnannotatedAPIWarning::RefTyDefRetained *v) {};
391+
DefaultOwnershipSuppressUnannotatedAPIWarning::RefTyDefUnretained *v) {};
418392

419393
namespace DefaultOwnershipInheritance {
420394

421395
struct __attribute__((swift_attr("import_reference")))
422396
__attribute__((swift_attr("retain:baseRetain")))
423397
__attribute__((swift_attr("release:baseRelease")))
424-
__attribute__((swift_attr("returned_as_retained_by_default"))) BaseType {};
398+
__attribute__((swift_attr("returned_as_unretained_by_default"))) BaseType {};
425399

426400
struct __attribute__((swift_attr("import_reference")))
427401
__attribute__((swift_attr("retain:derivedRetain")))
@@ -433,16 +407,9 @@ __attribute__((swift_attr("retain:derivedRetain2")))
433407
__attribute__((swift_attr("release:derivedRelease2"))) DerivedType2
434408
: public DerivedType {};
435409

436-
struct __attribute__((swift_attr("import_reference")))
437-
__attribute__((swift_attr("retain:derivedRetain3")))
438-
__attribute__((swift_attr("release:derivedRelease3")))
439-
__attribute__((swift_attr("returned_as_unretained_by_default"))) DerivedOverride
440-
: public DerivedType {};
441-
442-
BaseType *returnBaseType() { return new BaseType(); }
443-
DerivedType *returnDerivedType() { return new DerivedType(); }
444-
DerivedType2 *returnDerivedType2() { return new DerivedType2(); }
445-
DerivedOverride *returnDerivedOverride() { return new DerivedOverride(); }
410+
BaseType *createBaseType() { return new BaseType(); }
411+
DerivedType *createDerivedType() { return new DerivedType(); }
412+
DerivedType2 *createDerivedType2() { return new DerivedType2(); }
446413

447414
struct __attribute__((swift_attr("import_reference")))
448415
__attribute__((swift_attr("retain:bRetain")))
@@ -453,10 +420,15 @@ __attribute__((swift_attr("retain:dRetain")))
453420
__attribute__((swift_attr("release:dRelease"))) DerivedTypeNonDefault
454421
: public BaseTypeNonDefault {};
455422

456-
BaseTypeNonDefault *returnBaseTypeNonDefault() { // expected-warning {{'returnBaseTypeNonDefault' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
423+
BaseTypeNonDefault *createBaseTypeNonDefault() { // expected-warning {{'createBaseTypeNonDefault' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
457424
return new BaseTypeNonDefault();
458425
}
459-
DerivedTypeNonDefault *returnDerivedTypeNonDefault() { // expected-warning {{'returnDerivedTypeNonDefault' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
426+
DerivedTypeNonDefault *createDerivedTypeNonDefault() { // expected-warning {{'createDerivedTypeNonDefault' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
427+
return new DerivedTypeNonDefault();
428+
}
429+
430+
DerivedTypeNonDefault *createDerivedTypeNonDefaultUnretained()
431+
__attribute__((swift_attr("returns_unretained"))) {
460432
return new DerivedTypeNonDefault();
461433
}
462434

@@ -471,9 +443,6 @@ void derivedRelease(DefaultOwnershipInheritance::DerivedType *v) {};
471443
void derivedRetain2(DefaultOwnershipInheritance::DerivedType2 *v) {};
472444
void derivedRelease2(DefaultOwnershipInheritance::DerivedType2 *v) {};
473445

474-
void derivedRetain3(DefaultOwnershipInheritance::DerivedOverride *v) {};
475-
void derivedRelease3(DefaultOwnershipInheritance::DerivedOverride *v) {};
476-
477446
void bRetain(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {};
478447
void bRelease(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {};
479448

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,14 @@ let f = FunctionVoidToFRTStruct()
3232
let frt = f()
3333
let nonFrt = NonFRTStruct()
3434
let nonFrtLocalVar1 = global_function_returning_templated_retrun_frt_owned(nonFrt)
35+
let _ = DefaultOwnershipConventionOnCXXForeignRefType.returnRefTyDefUnretained()
36+
let _ = FunctionAnnotationHasPrecedence.returnRefTyDefUnretained()
37+
let _ = FunctionAnnotationHasPrecedence.returnRefTyDefUnretainedAnnotatedRetained()
3538
let _ = DefaultOwnershipSuppressUnannotatedAPIWarning.returnRefType()
36-
let _ = DefaultOwnershipSuppressUnannotatedAPIWarning.returnRefTyDefRetained()
37-
let _ = DefaultOwnershipInheritance.returnBaseType()
38-
let _ = DefaultOwnershipInheritance.returnDerivedType()
39-
let _ = DefaultOwnershipInheritance.returnDerivedType2()
40-
let _ = DefaultOwnershipInheritance.returnBaseTypeNonDefault()
41-
let _ = DefaultOwnershipInheritance.returnDerivedTypeNonDefault()
39+
let _ = DefaultOwnershipSuppressUnannotatedAPIWarning.returnRefTyDefUnretainedd()
40+
let _ = DefaultOwnershipInheritance.createBaseType()
41+
let _ = DefaultOwnershipInheritance.createDerivedType()
42+
let _ = DefaultOwnershipInheritance.createDerivedType2()
43+
let _ = DefaultOwnershipInheritance.createBaseTypeNonDefault()
44+
let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefault()
45+
let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefaultUnretained()

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

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,6 @@ func testVirtualMethods(base: Base, derived: Derived) {
249249
}
250250

251251
func testDefaultOwnershipAnnotation() {
252-
let _ = DefaultOwnershipConventionOnCXXForeignRefType.returnRefTyDefRetained()
253-
// CHECK: function_ref {{.*}}returnRefTyDefRetained{{.*}} : $@convention(c) () -> @owned DefaultOwnershipConventionOnCXXForeignRefType.RefTyDefRetained
254-
255252
let _ = DefaultOwnershipConventionOnCXXForeignRefType.returnRefTyDefUnretained()
256253
// CHECK: function_ref {{.*}}returnRefTyDefUnretained{{.*}} : $@convention(c) () -> DefaultOwnershipConventionOnCXXForeignRefType.RefTyDefUnretained
257254

@@ -261,27 +258,21 @@ func testDefaultOwnershipAnnotation() {
261258
let _ = FunctionAnnotationHasPrecedence.returnRefTyDefUnretainedAnnotatedRetained()
262259
// CHECK: function_ref {{.*}}returnRefTyDefUnretainedAnnotatedRetained{{.*}} : $@convention(c) () -> @owned FunctionAnnotationHasPrecedence.RefTyDefUnretained
263260

264-
let _ = FunctionAnnotationHasPrecedence.returnRefTyDefRetained()
265-
// CHECK: function_ref {{.*}}returnRefTyDefRetained{{.*}} : $@convention(c) () -> @owned FunctionAnnotationHasPrecedence.RefTyDefRetained
266-
267-
let _ = FunctionAnnotationHasPrecedence.returnRefTyDefRetainedAnnotatedUnRetained()
268-
// CHECK: function_ref {{.*}}returnRefTyDefRetainedAnnotatedUnRetained{{.*}} : $@convention(c) () -> FunctionAnnotationHasPrecedence.RefTyDefRetained
269-
270-
let _ = DefaultOwnershipInheritance.returnBaseType()
271-
// CHECK: function_ref {{.*}}returnBaseType{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.BaseType
261+
let _ = DefaultOwnershipInheritance.createBaseType()
262+
// CHECK: function_ref {{.*}}createBaseType{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.BaseType
272263

273-
let _ = DefaultOwnershipInheritance.returnDerivedType()
274-
// CHECK: function_ref {{.*}}returnDerivedType{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.DerivedType
264+
let _ = DefaultOwnershipInheritance.createDerivedType()
265+
// CHECK: function_ref {{.*}}createDerivedType{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedType
275266

276-
let _ = DefaultOwnershipInheritance.returnDerivedType2()
277-
// CHECK: function_ref {{.*}}returnDerivedType2{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.DerivedType2
267+
let _ = DefaultOwnershipInheritance.createDerivedType2()
268+
// CHECK: function_ref {{.*}}createDerivedType2{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedType2
278269

279-
let _ = DefaultOwnershipInheritance.returnDerivedOverride()
280-
// CHECK: function_ref {{.*}}returnDerivedOverride{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedOverride
270+
let _ = DefaultOwnershipInheritance.createBaseTypeNonDefault()
271+
// CHECK: function_ref {{.*}}createBaseTypeNonDefault{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.BaseTypeNonDefault
281272

282-
let _ = DefaultOwnershipInheritance.returnBaseTypeNonDefault()
283-
// CHECK: function_ref {{.*}}returnBaseTypeNonDefault{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.BaseTypeNonDefault
273+
let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefault()
274+
// CHECK: function_ref {{.*}}createDerivedTypeNonDefault{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.DerivedTypeNonDefault
284275

285-
let _ = DefaultOwnershipInheritance.returnDerivedTypeNonDefault()
286-
// CHECK: function_ref {{.*}}returnDerivedTypeNonDefault{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedTypeNonDefault
276+
let _ = DefaultOwnershipInheritance.createDerivedTypeNonDefaultUnretained()
277+
// CHECK: function_ref {{.*}}createDerivedTypeNonDefaultUnretained{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedTypeNonDefault
287278
}

0 commit comments

Comments
 (0)