From 31d7816f925c01ca5e96d2d4eb19053debed2b29 Mon Sep 17 00:00:00 2001 From: Fahad Nayyar Date: Tue, 7 Oct 2025 22:37:57 -0700 Subject: [PATCH] [cxx-interop] Enabling WarnUnannotatedReturnOfCxxFrt on by default and add it to a diagnostic group This change makes the warning for unannotated C++ functions returning foreign reference types (FRT) enabled by default, improving memory safety for Swift/C++ interop users. Also added CxxForeignReferenceType diagnostic group for better control --- include/swift/AST/DiagnosticGroups.def | 1 + include/swift/AST/DiagnosticsSema.def | 2 +- include/swift/Basic/Features.def | 5 +-- lib/AST/FeatureSet.cpp | 1 - lib/Sema/MiscDiagnostics.cpp | 2 - .../frt-reference-returns-diagnostics.swift | 4 +- ...retained-unretained-attributes-error.swift | 3 +- .../inheritance-diagnostics.swift | 3 +- .../objc-returning-cxx-frt-diagnostics.swift | 3 +- ...struct-parameter-back-to-cxx-execution.cpp | 14 +++--- .../diagnostics/cxx-foreign-reference-type.md | 44 +++++++++++++++++++ 11 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 userdocs/diagnostics/cxx-foreign-reference-type.md diff --git a/include/swift/AST/DiagnosticGroups.def b/include/swift/AST/DiagnosticGroups.def index 4f55336d1ed97..600ab59241f91 100644 --- a/include/swift/AST/DiagnosticGroups.def +++ b/include/swift/AST/DiagnosticGroups.def @@ -45,6 +45,7 @@ GROUP(AlwaysAvailableDomain, "always-available-domain") GROUP(AvailabilityUnrecognizedName, "availability-unrecognized-name") GROUP(ClangDeclarationImport, "clang-declaration-import") GROUP(ConformanceIsolation, "conformance-isolation") +GROUP(CxxForeignReferenceType, "cxx-foreign-reference-type") GROUP(DeprecatedDeclaration, "deprecated-declaration") GROUP(DynamicCallable, "dynamic-callable-requirements") GROUP(EmbeddedRestrictions, "embedded-restrictions") diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 1e67a2b7a491d..83a636f5f9beb 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2211,7 +2211,7 @@ ERROR(expose_nested_type_to_cxx,none, "nested %kind0 can not yet be represented in C++", (ValueDecl *)) ERROR(expose_macro_to_cxx,none, "Swift macro can not yet be represented in C++", (ValueDecl *)) -WARNING(warn_unannotated_cxx_func_returning_frt, none, +GROUPED_WARNING(warn_unannotated_cxx_func_returning_frt, CxxForeignReferenceType, none, "cannot infer the ownership of the returned value, annotate %0 with " "either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED", (const ValueDecl *)) diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index cc66d0f279c4a..20187cd156c92 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -213,6 +213,7 @@ BASELINE_LANGUAGE_FEATURE(BuiltinUnprotectedStackAlloc, 0, "Builtin.unprotectedS BASELINE_LANGUAGE_FEATURE(BuiltinTaskRunInline, 0, "Builtin.taskRunInline") BASELINE_LANGUAGE_FEATURE(BuiltinUnprotectedAddressOf, 0, "Builtin.unprotectedAddressOf") BASELINE_LANGUAGE_FEATURE(NewCxxMethodSafetyHeuristics, 0, "Only import C++ methods that return pointers (projections) on owned types as unsafe") +BASELINE_LANGUAGE_FEATURE(WarnUnannotatedReturnOfCxxFrt, 0, "Warn about unannotated C++ functions returning foreign reference types") BASELINE_LANGUAGE_FEATURE(SpecializeAttributeWithAvailability, 0, "@_specialize attribute with availability") BASELINE_LANGUAGE_FEATURE(BuiltinAssumeAlignment, 0, "Builtin.assumeAlignment") BASELINE_LANGUAGE_FEATURE(BuiltinCreateTaskGroupWithFlags, 0, "Builtin.createTaskGroupWithFlags") @@ -502,10 +503,6 @@ EXPERIMENTAL_FEATURE(ImportNonPublicCxxMembers, true) /// types and importing them as Swift initializers. EXPERIMENTAL_FEATURE(SuppressCXXForeignReferenceTypeInitializers, true) -/// Emit a warning when a C++ API returns a SWIFT_SHARED_REFERENCE type -/// without being explicitly annotated with either SWIFT_RETURNS_RETAINED -/// or SWIFT_RETURNS_UNRETAINED. -EXPERIMENTAL_FEATURE(WarnUnannotatedReturnOfCxxFrt, true) /// modify/read single-yield coroutines SUPPRESSIBLE_EXPERIMENTAL_FEATURE(CoroutineAccessors, true) diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index 8ff646b1922a5..3c78cc9065962 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -333,7 +333,6 @@ UNINTERESTING_FEATURE(SafeInteropWrappers) UNINTERESTING_FEATURE(AssumeResilientCxxTypes) UNINTERESTING_FEATURE(ImportNonPublicCxxMembers) UNINTERESTING_FEATURE(SuppressCXXForeignReferenceTypeInitializers) -UNINTERESTING_FEATURE(WarnUnannotatedReturnOfCxxFrt) UNINTERESTING_FEATURE(CoroutineAccessorsUnwindOnCallerError) UNINTERESTING_FEATURE(AllowRuntimeSymbolDeclarations) diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index f9bfc444c8965..f7ad35ff8f106 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -6373,8 +6373,6 @@ static bool isReturningFRT(const clang::NamedDecl *ND, static bool shouldDiagnoseMissingReturnsRetained(const clang::NamedDecl *ND, clang::QualType retType, ASTContext &Ctx) { - if (!Ctx.LangOpts.hasFeature(Feature::WarnUnannotatedReturnOfCxxFrt)) - return false; auto attrInfo = importer::ReturnOwnershipInfo(ND); if (attrInfo.hasRetainAttr()) diff --git a/test/Interop/Cxx/foreign-reference/frt-reference-returns-diagnostics.swift b/test/Interop/Cxx/foreign-reference/frt-reference-returns-diagnostics.swift index 7f974ab2c1a23..553892e05f792 100644 --- a/test/Interop/Cxx/foreign-reference/frt-reference-returns-diagnostics.swift +++ b/test/Interop/Cxx/foreign-reference/frt-reference-returns-diagnostics.swift @@ -1,7 +1,5 @@ // RUN: rm -rf %t -// RUN: %target-typecheck-verify-swift -I %S%{fs-sep}Inputs -cxx-interoperability-mode=default -enable-experimental-feature WarnUnannotatedReturnOfCxxFrt -verify-additional-file %S%{fs-sep}Inputs%{fs-sep}frt-reference-returns.h - -// REQUIRES: swift_feature_WarnUnannotatedReturnOfCxxFrt +// RUN: %target-typecheck-verify-swift -I %S%{fs-sep}Inputs -cxx-interoperability-mode=default -verify-additional-file %S%{fs-sep}Inputs%{fs-sep}frt-reference-returns.h import FRTReferenceReturns diff --git a/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift b/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift index 4bc5eaf434cc8..9f86e1b9c729d 100644 --- a/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift +++ b/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift @@ -1,10 +1,9 @@ // RUN: rm -rf %t -// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs %s -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature WarnUnannotatedReturnOfCxxFrt -verify-additional-file %S/Inputs/cxx-functions-and-methods-returning-frt.h -Xcc -Wno-return-type -Xcc -Wno-nullability-completeness +// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs %s -cxx-interoperability-mode=upcoming-swift -verify-additional-file %S/Inputs/cxx-functions-and-methods-returning-frt.h -Xcc -Wno-return-type -Xcc -Wno-nullability-completeness // XFAIL: OS=windows-msvc // TODO: Enable this on windows when -verify-additional-file issue on Windows Swift CI is resolved -// REQUIRES: swift_feature_WarnUnannotatedReturnOfCxxFrt import FunctionsAndMethodsReturningFRT import CxxStdlib diff --git a/test/Interop/Cxx/foreign-reference/inheritance-diagnostics.swift b/test/Interop/Cxx/foreign-reference/inheritance-diagnostics.swift index 563b377b2c474..c080097ff400a 100644 --- a/test/Interop/Cxx/foreign-reference/inheritance-diagnostics.swift +++ b/test/Interop/Cxx/foreign-reference/inheritance-diagnostics.swift @@ -1,7 +1,6 @@ // RUN: rm -rf %t -// RUN: %target-swift-frontend -typecheck -verify -I %S%{fs-sep}Inputs %s -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature WarnUnannotatedReturnOfCxxFrt -verify-additional-file %S%{fs-sep}Inputs%{fs-sep}inheritance.h -Xcc -Wno-return-type -Xcc -Wno-inaccessible-base +// RUN: %target-swift-frontend -typecheck -verify -I %S%{fs-sep}Inputs %s -cxx-interoperability-mode=upcoming-swift -verify-additional-file %S%{fs-sep}Inputs%{fs-sep}inheritance.h -Xcc -Wno-return-type -Xcc -Wno-inaccessible-base -// REQUIRES: swift_feature_WarnUnannotatedReturnOfCxxFrt import Inheritance diff --git a/test/Interop/Cxx/objc-correctness/objc-returning-cxx-frt-diagnostics.swift b/test/Interop/Cxx/objc-correctness/objc-returning-cxx-frt-diagnostics.swift index 40adaff181db4..bf47ee5a82157 100644 --- a/test/Interop/Cxx/objc-correctness/objc-returning-cxx-frt-diagnostics.swift +++ b/test/Interop/Cxx/objc-correctness/objc-returning-cxx-frt-diagnostics.swift @@ -1,7 +1,6 @@ // RUN: rm -rf %t -// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs %s -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature WarnUnannotatedReturnOfCxxFrt -verify-additional-file %S/Inputs/cxx-frt.h -Xcc -Wno-return-type +// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs %s -cxx-interoperability-mode=upcoming-swift -verify-additional-file %S/Inputs/cxx-frt.h -Xcc -Wno-return-type -// REQUIRES: swift_feature_WarnUnannotatedReturnOfCxxFrt import CxxForeignRef diff --git a/test/Interop/CxxToSwiftToCxx/consuming-cxx-struct-parameter-back-to-cxx-execution.cpp b/test/Interop/CxxToSwiftToCxx/consuming-cxx-struct-parameter-back-to-cxx-execution.cpp index c0b4698b375b0..e780f5155a8c2 100644 --- a/test/Interop/CxxToSwiftToCxx/consuming-cxx-struct-parameter-back-to-cxx-execution.cpp +++ b/test/Interop/CxxToSwiftToCxx/consuming-cxx-struct-parameter-back-to-cxx-execution.cpp @@ -1,13 +1,13 @@ // RUN: %empty-directory(%t) // RUN: split-file %s %t -// RUN: %target-swift-frontend %t/use-cxx-types.swift -module-name UseCxx -typecheck -verify -emit-clang-header-path %t/UseCxx.h -I %t -enable-experimental-cxx-interop -clang-header-expose-decls=all-public -disable-availability-checking +// RUN: %target-swift-frontend %t%{fs-sep}use-cxx-types.swift -module-name UseCxx -typecheck -verify -verify-additional-file %t%{fs-sep}header.h -emit-clang-header-path %t%{fs-sep}UseCxx.h -I %t -enable-experimental-cxx-interop -clang-header-expose-decls=all-public -disable-availability-checking -// RUN: %target-interop-build-clangxx -std=c++20 -c %t/use-swift-cxx-types.cpp -I %t -o %t/swift-cxx-execution.o -// RUN: %target-interop-build-swift %t/use-cxx-types.swift -o %t/swift-cxx-execution -Xlinker %t/swift-cxx-execution.o -module-name UseCxx -Xfrontend -entry-point-function-name -Xfrontend swiftMain -I %t -O -Xfrontend -disable-availability-checking +// RUN: %target-interop-build-clangxx -std=c++20 -c %t%{fs-sep}use-swift-cxx-types.cpp -I %t -o %t%{fs-sep}swift-cxx-execution.o +// RUN: %target-interop-build-swift %t%{fs-sep}use-cxx-types.swift -o %t%{fs-sep}swift-cxx-execution -Xlinker %t%{fs-sep}swift-cxx-execution.o -module-name UseCxx -Xfrontend -entry-point-function-name -Xfrontend swiftMain -I %t -O -Xfrontend -disable-availability-checking -// RUN: %target-codesign %t/swift-cxx-execution -// RUN: %target-run %t/swift-cxx-execution | %FileCheck %s +// RUN: %target-codesign %t%{fs-sep}swift-cxx-execution +// RUN: %target-run %t%{fs-sep}swift-cxx-execution | %FileCheck %s // REQUIRES: executable_test @@ -63,7 +63,7 @@ __attribute__((swift_attr("release:releaseShared"))); inline void retainShared(SharedFRT *r) { puts("retainShared"); } inline void releaseShared(SharedFRT *r) { puts("releaseShared"); } -inline SharedFRT* createSharedFRT() { return new SharedFRT(); } +inline SharedFRT* createSharedFRT() { return new SharedFRT(); } // expected-note {{'createSharedFRT()' is defined here}} //--- module.modulemap module CxxTest { @@ -109,7 +109,7 @@ public func returnSharedFRT(_ x : SharedFRT) -> SharedFRT { } public func returnSharedFRT2() -> SharedFRT { - return createSharedFRT() + return createSharedFRT() // expected-warning {{cannot infer the ownership of the returned value, annotate 'createSharedFRT()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}} } public struct ValueWrapper { diff --git a/userdocs/diagnostics/cxx-foreign-reference-type.md b/userdocs/diagnostics/cxx-foreign-reference-type.md new file mode 100644 index 0000000000000..fa559cb02cc5e --- /dev/null +++ b/userdocs/diagnostics/cxx-foreign-reference-type.md @@ -0,0 +1,44 @@ +# C++ Foreign Reference Type warnings (CxxForeignReferenceType) + +Warnings related to C++ APIs returning foreign reference types that lack proper ownership annotations. + +## Overview + +When importing C++ types marked with `SWIFT_SHARED_REFERENCE` into Swift, the compiler needs to understand the ownership semantics of functions returning these types. Without explicit annotations, the compiler cannot determine whether the returned object should be retained or not, which can lead to memory management issues. + +## The Warning + +The compiler emits a warning when it encounters a C++ function or Objective-C method that: +1. Returns a type marked with `SWIFT_SHARED_REFERENCE` (foreign reference type) +2. Lacks either `SWIFT_RETURNS_RETAINED` or `SWIFT_RETURNS_UNRETAINED` annotation + +Example warning: +``` +warning: cannot infer the ownership of the returned value, annotate 'functionName()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED +``` + +## How to Fix + +### For C++ Functions + +Add the appropriate annotation to your C++ function declaration: + +```cpp +// If the function returns a retained value +SWIFT_RETURNS_RETAINED MyFRT* createObject(); + +// If the function returns an unretained value +SWIFT_RETURNS_UNRETAINED MyFRT* getExistingObject(); +``` + +### For Objective-C Methods + +Similarly, annotate your Objective-C methods: + +```objc +// Retained value +- (MyFRT *)createObject SWIFT_RETURNS_RETAINED; + +// Unretained value +- (MyFRT *)getExistingObject SWIFT_RETURNS_UNRETAINED; +```