Skip to content

Commit e7bbd4b

Browse files
authored
[cxx-interop] Introduce type-level annotations to specify default ownership convention for C++ foreign reference return values (#81093)
In Swift 6.1, we introduced `SWIFT_RETURNS_RETAINED` and `SWIFT_RETURNS_UNRETAINED` annotations for C++ APIs to explicitly specify the ownership convention of `SWIFT_SHARED_REFERENCE` type return values. Currently the Swift compiler emits warnings for unannotated C++ APIs returning `SWIFT_SHARED_REFERENCE` types. We've received some feedback that people are finding these warnings useful to get a reminder to annotate their APIs. While this improves correctness , it also imposes a high annotation burden on adopters — especially in large C++ codebases. This patch addresses that burden by introducing two new type-level annotations: - `SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT` - `SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT` These annotations allow developers to specify a default ownership convention for all C++ APIs returning a given `SWIFT_SHARED_REFERENCE`-annotated type, unless explicitly overridden at the API by using `SWIFT_RETURNS_RETAINED` or `SWIFT_RETURNS_UNRETAINED`. If a C++ class inherits from a base class annotated with `SWIFT_RETURNED_AS_RETAINED_BY_DEFAULT` or `SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT`, the derived class automatically inherits the default ownership convention unless it is explicitly overridden. This strikes a balance between safety/correctness and usability: - It avoids the need to annotate every API individually. - It retains the ability to opt out of the default at the API level when needed. - To verify correctness, the user can just remove the `SWIFT_RETURNED_AS_(UN)RETAINED_BY_DEFAULT` annotation from that type and they will start seeing the warnings on all the unannotated C++ APIs returning that `SWIFT_SHARED_REFERENCE` type. They can add `SWIFT_RETURNS_(UN)RETAINED` annotation at each API in which they want a different behaviour than the default. Then they can reintroduce the `SWIFT_RETURNED_AS_(UN)RETAINED_BY_DEFAULT` at the type level to suppress the warnings on remaining unannotated APIs. A global default ownership convention (like always return `unretained`/`unowned`) was considered but it would weaken the diagnostic signal and remove valuable guardrails that help detect use-after-free bugs and memory leaks in absence of `SWIFT_RETURNS_(UN)RETAINED` annotations. In the absence of these annotations when Swift emits the unannotated API warning, the current fallback behavior (e.g. relying on heuristics based on API name such as `"create"`, `"copy"`, `"get"`) is derived from Objective-C interop but is ill-suited for C++, which has no consistent naming patterns for ownership semantics. Several codebases are expected to have project-specific conventions, such as defaulting to unretained except for factory methods and constructors. A type-level default seems like the most precise and scalable mechanism to support such patterns. It integrates cleanly with existing `SWIFT_SHARED_REFERENCE` usage and provides a per-type opt-in mechanism without global silencing of ownership diagnostics. This addition improves ergonomics while preserving the safety benefits of explicit annotations and diagnostics. rdar://145453509
1 parent c249357 commit e7bbd4b

File tree

8 files changed

+367
-14
lines changed

8 files changed

+367
-14
lines changed

include/swift/ClangImporter/ClangImporter.h

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "swift/AST/Attr.h"
2020
#include "swift/AST/AttrKind.h"
2121
#include "swift/AST/ClangModuleLoader.h"
22+
#include "clang/AST/Attr.h"
2223
#include "clang/Basic/Specifiers.h"
2324
#include "llvm/Support/VirtualFileSystem.h"
2425

@@ -70,6 +71,7 @@ namespace dependencies {
7071
}
7172

7273
namespace swift {
74+
enum class ResultConvention : uint8_t;
7375
class ASTContext;
7476
class CompilerInvocation;
7577
class ClangImporterOptions;
@@ -802,6 +804,88 @@ bool isClangNamespace(const DeclContext *dc);
802804
/// specialized class templates are instead imported as unspecialized.
803805
bool isSymbolicCircularBase(const clang::CXXRecordDecl *templatedClass,
804806
const clang::RecordDecl *base);
807+
808+
/// Match a `[[swift_attr("...")]]` annotation on the given Clang decl.
809+
///
810+
/// \param decl The Clang declaration to inspect.
811+
/// \param patterns List of (attribute name, value) pairs.
812+
/// \returns The value for the first matching attribute, or `std::nullopt`.
813+
template <typename T>
814+
std::optional<T>
815+
matchSwiftAttr(const clang::Decl *decl,
816+
llvm::ArrayRef<std::pair<llvm::StringRef, T>> patterns) {
817+
if (!decl || !decl->hasAttrs())
818+
return std::nullopt;
819+
820+
for (const auto *attr : decl->getAttrs()) {
821+
if (const auto *swiftAttr = llvm::dyn_cast<clang::SwiftAttrAttr>(attr)) {
822+
for (const auto &p : patterns) {
823+
if (swiftAttr->getAttribute() == p.first)
824+
return p.second;
825+
}
826+
}
827+
}
828+
return std::nullopt;
829+
}
830+
831+
/// Like `matchSwiftAttr`, but also searches C++ base classes.
832+
///
833+
/// \param decl The Clang declaration to inspect.
834+
/// \param patterns List of (attribute name, value) pairs.
835+
/// \returns The matched value from this decl or its bases, or `std::nullopt`.
836+
template <typename T>
837+
std::optional<T> matchSwiftAttrConsideringInheritance(
838+
const clang::Decl *decl,
839+
llvm::ArrayRef<std::pair<llvm::StringRef, T>> patterns) {
840+
if (!decl)
841+
return std::nullopt;
842+
843+
if (auto match = matchSwiftAttr<T>(decl, patterns))
844+
return match;
845+
846+
if (const auto *recordDecl = llvm::dyn_cast<clang::CXXRecordDecl>(decl)) {
847+
std::optional<T> result;
848+
recordDecl->forallBases([&](const clang::CXXRecordDecl *base) -> bool {
849+
if (auto baseMatch = matchSwiftAttr<T>(base, patterns)) {
850+
result = baseMatch;
851+
return false;
852+
}
853+
854+
return true;
855+
});
856+
857+
return result;
858+
}
859+
860+
return std::nullopt;
861+
}
862+
863+
/// Matches a `swift_attr("...")` on the record type pointed to by the given
864+
/// Clang type, searching base classes if it's a C++ class.
865+
///
866+
/// \param type A Clang pointer type.
867+
/// \param patterns List of attribute name-value pairs to match.
868+
/// \returns Matched value or std::nullopt.
869+
template <typename T>
870+
std::optional<T> matchSwiftAttrOnRecordPtr(
871+
const clang::QualType &type,
872+
llvm::ArrayRef<std::pair<llvm::StringRef, T>> patterns) {
873+
if (const auto *ptrType = type->getAs<clang::PointerType>()) {
874+
if (const auto *recordDecl = ptrType->getPointeeType()->getAsRecordDecl()) {
875+
return matchSwiftAttrConsideringInheritance<T>(recordDecl, patterns);
876+
}
877+
}
878+
return std::nullopt;
879+
}
880+
881+
/// Determines the C++ reference ownership convention for the return value
882+
/// using `SWIFT_RETURNS_(UN)RETAINED` on the API; falls back to
883+
/// `SWIFT_RETURNED_AS_(UN)RETAINED_BY_DEFAULT` on the pointee record type.
884+
///
885+
/// \param decl The Clang function or method declaration to inspect.
886+
/// \returns Matched `ResultConvention`, or `std::nullopt` if none applies.
887+
std::optional<ResultConvention>
888+
getCxxRefConventionWithAttrs(const clang::Decl *decl);
805889
} // namespace importer
806890

807891
struct ClangInvocationFileMapping {

lib/ClangImporter/ClangImporter.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8794,3 +8794,37 @@ bool importer::isSymbolicCircularBase(const clang::CXXRecordDecl *symbolicClass,
87948794
return classTemplate->getCanonicalDecl() ==
87958795
specializedBase->getSpecializedTemplate()->getCanonicalDecl();
87968796
}
8797+
8798+
std::optional<ResultConvention>
8799+
swift::importer::getCxxRefConventionWithAttrs(const clang::Decl *decl) {
8800+
using RC = ResultConvention;
8801+
8802+
if (auto result =
8803+
matchSwiftAttr<RC>(decl, {{"returns_unretained", RC::Unowned},
8804+
{"returns_retained", RC::Owned}}))
8805+
return result;
8806+
8807+
const clang::Type *returnTy = nullptr;
8808+
if (const auto *func = llvm::dyn_cast<clang::FunctionDecl>(decl))
8809+
returnTy = func->getReturnType().getTypePtrOrNull();
8810+
else if (const auto *method = llvm::dyn_cast<clang::ObjCMethodDecl>(decl))
8811+
returnTy = method->getReturnType().getTypePtrOrNull();
8812+
8813+
if (!returnTy)
8814+
return std::nullopt;
8815+
8816+
const clang::Type *desugaredReturnTy =
8817+
returnTy->getUnqualifiedDesugaredType();
8818+
8819+
if (const auto *ptrType =
8820+
llvm::dyn_cast<clang::PointerType>(desugaredReturnTy)) {
8821+
if (const clang::RecordDecl *record =
8822+
ptrType->getPointeeType()->getAsRecordDecl()) {
8823+
return matchSwiftAttrConsideringInheritance<RC>(
8824+
record, {{"returned_as_unretained_by_default", RC::Unowned},
8825+
{"returned_as_retained_by_default", RC::Owned}});
8826+
}
8827+
}
8828+
8829+
return std::nullopt;
8830+
}

lib/ClangImporter/ImportDecl.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3665,6 +3665,12 @@ namespace {
36653665
unannotatedAPIWarningNeeded = true;
36663666
}
36673667

3668+
if (importer::matchSwiftAttrOnRecordPtr<bool>(
3669+
retType, {{"returned_as_retained_by_default", true},
3670+
{"returned_as_unretained_by_default", true}})) {
3671+
unannotatedAPIWarningNeeded = false;
3672+
}
3673+
36683674
if (unannotatedAPIWarningNeeded) {
36693675
HeaderLoc loc(decl->getLocation());
36703676
Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,

lib/ClangImporter/SwiftBridging/swift/bridging

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,40 @@
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+
211+
/// Applied to a C++ foreign reference type annotated with
212+
/// SWIFT_SHARED_REFERENCE. Indicates that C++ APIs returning this type are
213+
/// assumed to return an unowned (+0) value by default, unless explicitly annotated
214+
/// with SWIFT_RETURNS_RETAINED.
215+
///
216+
/// For example:
217+
/// ```c++
218+
/// struct SWIFT_SHARED_REFERENCE(retainBar, releaseBar)
219+
/// SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT
220+
/// Bar { ... };
221+
/// ```
222+
///
223+
/// In Swift, C++ APIs returning `Bar*` will be assumed to return an unowned
224+
/// value.
225+
#define SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT \
226+
__attribute__((swift_attr("returned_as_unretained_by_default")))
227+
194228
/// Specifies that the non-public members of a C++ class, struct, or union can
195229
/// be accessed from extensions of that type, in the given file ID.
196230
///

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,23 +1345,18 @@ class Conventions {
13451345
llvm_unreachable("unhandled ownership");
13461346
}
13471347

1348-
// Determines owned/unowned ResultConvention of the returned value based on
1349-
// returns_retained/returns_unretained attribute.
1348+
// Determines the ownership ResultConvention (owned/unowned) of the return
1349+
// value using the SWIFT_RETURNS_(UN)RETAINED annotation on the C++ API; if
1350+
// not explicitly annotated, falls back to the
1351+
// SWIFT_RETURNED_AS_(UN)RETAINED_BY_DEFAULT annotation on the C++
1352+
// SWIFT_SHARED_REFERENCE type.
13501353
std::optional<ResultConvention>
13511354
getCxxRefConventionWithAttrs(const TypeLowering &tl,
13521355
const clang::Decl *decl) const {
1353-
if (tl.getLoweredType().isForeignReferenceType() && decl->hasAttrs()) {
1354-
for (const auto *attr : decl->getAttrs()) {
1355-
if (const auto *swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
1356-
if (swiftAttr->getAttribute() == "returns_unretained") {
1357-
return ResultConvention::Unowned;
1358-
} else if (swiftAttr->getAttribute() == "returns_retained") {
1359-
return ResultConvention::Owned;
1360-
}
1361-
}
1362-
}
1363-
}
1364-
return std::nullopt;
1356+
if (!tl.getLoweredType().isForeignReferenceType())
1357+
return std::nullopt;
1358+
1359+
return importer::getCxxRefConventionWithAttrs(decl);
13651360
}
13661361
};
13671362

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

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

45
// FRT or SWIFT_SHARED_REFERENCE type
@@ -326,3 +327,157 @@ struct Derived : Base {
326327
FRTStruct *_Nonnull VirtualMethodReturningFRTUnowned() override
327328
__attribute__((swift_attr("returns_unretained")));
328329
};
330+
331+
SWIFT_BEGIN_NULLABILITY_ANNOTATIONS
332+
333+
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+
341+
struct __attribute__((swift_attr("import_reference")))
342+
__attribute__((swift_attr("retain:defRetain2")))
343+
__attribute__((swift_attr("release:defRelease2")))
344+
__attribute__((swift_attr("returned_as_unretained_by_default"))) RefTyDefUnretained {};
345+
346+
RefTyDefUnretained *returnRefTyDefUnretained() {
347+
return new RefTyDefUnretained();
348+
}
349+
} // namespace DefaultOwnershipConventionOnCXXForeignRefType
350+
351+
void defRetain1(
352+
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefRetained *v) {};
353+
void defRelease1(
354+
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefRetained *v) {};
355+
356+
void defRetain2(
357+
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefUnretained *v) {};
358+
void defRelease2(
359+
DefaultOwnershipConventionOnCXXForeignRefType::RefTyDefUnretained *v) {};
360+
361+
namespace FunctionAnnotationHasPrecedence {
362+
struct __attribute__((swift_attr("import_reference")))
363+
__attribute__((swift_attr("retain:defaultRetain1")))
364+
__attribute__((swift_attr("release:defaultRelease1")))
365+
__attribute__((swift_attr("returned_as_unretained_by_default"))) RefTyDefUnretained {};
366+
367+
RefTyDefUnretained *returnRefTyDefUnretained() {
368+
return new RefTyDefUnretained();
369+
}
370+
RefTyDefUnretained *returnRefTyDefUnretainedAnnotatedRetained()
371+
__attribute__((swift_attr("returns_retained"))) {
372+
return new RefTyDefUnretained();
373+
}
374+
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+
386+
} // namespace FunctionAnnotationHasPrecedence
387+
388+
void defaultRetain1(FunctionAnnotationHasPrecedence::RefTyDefUnretained *v) {};
389+
void defaultRelease1(FunctionAnnotationHasPrecedence::RefTyDefUnretained *v) {};
390+
391+
void defaultRetain2(FunctionAnnotationHasPrecedence::RefTyDefRetained *v) {};
392+
void defaultRelease2(FunctionAnnotationHasPrecedence::RefTyDefRetained *v) {};
393+
394+
namespace DefaultOwnershipSuppressUnannotatedAPIWarning {
395+
396+
struct __attribute__((swift_attr("import_reference")))
397+
__attribute__((swift_attr("retain:rretain")))
398+
__attribute__((swift_attr("release:rrelease"))) RefType {};
399+
400+
RefType *returnRefType() { return new RefType(); } // expected-warning {{'returnRefType' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
401+
402+
struct __attribute__((swift_attr("import_reference")))
403+
__attribute__((swift_attr("retain:dretain")))
404+
__attribute__((swift_attr("release:drelease")))
405+
__attribute__((swift_attr("returned_as_retained_by_default"))) RefTyDefRetained {};
406+
407+
RefTyDefRetained *returnRefTyDefRetained() { return new RefTyDefRetained(); }
408+
409+
} // namespace DefaultOwnershipSuppressUnannotatedAPIWarning
410+
411+
void rretain(DefaultOwnershipSuppressUnannotatedAPIWarning::RefType *v) {};
412+
void rrelease(DefaultOwnershipSuppressUnannotatedAPIWarning::RefType *v) {};
413+
414+
void dretain(
415+
DefaultOwnershipSuppressUnannotatedAPIWarning::RefTyDefRetained *v) {};
416+
void drelease(
417+
DefaultOwnershipSuppressUnannotatedAPIWarning::RefTyDefRetained *v) {};
418+
419+
namespace DefaultOwnershipInheritance {
420+
421+
struct __attribute__((swift_attr("import_reference")))
422+
__attribute__((swift_attr("retain:baseRetain")))
423+
__attribute__((swift_attr("release:baseRelease")))
424+
__attribute__((swift_attr("returned_as_retained_by_default"))) BaseType {};
425+
426+
struct __attribute__((swift_attr("import_reference")))
427+
__attribute__((swift_attr("retain:derivedRetain")))
428+
__attribute__((swift_attr("release:derivedRelease"))) DerivedType
429+
: public BaseType {};
430+
431+
struct __attribute__((swift_attr("import_reference")))
432+
__attribute__((swift_attr("retain:derivedRetain2")))
433+
__attribute__((swift_attr("release:derivedRelease2"))) DerivedType2
434+
: public DerivedType {};
435+
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(); }
446+
447+
struct __attribute__((swift_attr("import_reference")))
448+
__attribute__((swift_attr("retain:bRetain")))
449+
__attribute__((swift_attr("release:bRelease"))) BaseTypeNonDefault {};
450+
451+
struct __attribute__((swift_attr("import_reference")))
452+
__attribute__((swift_attr("retain:dRetain")))
453+
__attribute__((swift_attr("release:dRelease"))) DerivedTypeNonDefault
454+
: public BaseTypeNonDefault {};
455+
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}}
457+
return new BaseTypeNonDefault();
458+
}
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}}
460+
return new DerivedTypeNonDefault();
461+
}
462+
463+
} // namespace DefaultOwnershipInheritance
464+
465+
void baseRetain(DefaultOwnershipInheritance::BaseType *v) {};
466+
void baseRelease(DefaultOwnershipInheritance::BaseType *v) {};
467+
468+
void derivedRetain(DefaultOwnershipInheritance::DerivedType *v) {};
469+
void derivedRelease(DefaultOwnershipInheritance::DerivedType *v) {};
470+
471+
void derivedRetain2(DefaultOwnershipInheritance::DerivedType2 *v) {};
472+
void derivedRelease2(DefaultOwnershipInheritance::DerivedType2 *v) {};
473+
474+
void derivedRetain3(DefaultOwnershipInheritance::DerivedOverride *v) {};
475+
void derivedRelease3(DefaultOwnershipInheritance::DerivedOverride *v) {};
476+
477+
void bRetain(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {};
478+
void bRelease(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {};
479+
480+
void dRetain(DefaultOwnershipInheritance::DerivedTypeNonDefault *v) {};
481+
void dRelease(DefaultOwnershipInheritance::DerivedTypeNonDefault *v) {};
482+
483+
SWIFT_END_NULLABILITY_ANNOTATIONS

0 commit comments

Comments
 (0)