[cxx-interop] Enabling WarnUnannotatedReturnOfCxxFrt on by default and add it to a diagnostic group

fahadnayyar · j-hui · commit facef0e034db · 2025-10-17T17:06:44.000-07:00
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
diff --git 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
@@ -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
@@ -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")
@@ -498,10 +499,9 @@ EXPERIMENTAL_FEATURE(AssumeResilientCxxTypes, true)
 /// Import inherited non-public members when importing C++ classes.
 EXPERIMENTAL_FEATURE(ImportNonPublicCxxMembers, 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)
+/// Suppress the synthesis of static factory methods for C++ foreign reference
+/// types and importing them as Swift initializers.
+EXPERIMENTAL_FEATURE(SuppressCXXForeignReferenceTypeInitializers, true)
 
 /// modify/read single-yield coroutines
 SUPPRESSIBLE_EXPERIMENTAL_FEATURE(CoroutineAccessors, true)
diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp
@@ -332,7 +332,7 @@ UNINTERESTING_FEATURE(LibraryEvolution)
 UNINTERESTING_FEATURE(SafeInteropWrappers)
 UNINTERESTING_FEATURE(AssumeResilientCxxTypes)
 UNINTERESTING_FEATURE(ImportNonPublicCxxMembers)
-UNINTERESTING_FEATURE(WarnUnannotatedReturnOfCxxFrt)
+UNINTERESTING_FEATURE(SuppressCXXForeignReferenceTypeInitializers)
 UNINTERESTING_FEATURE(CoroutineAccessorsUnwindOnCallerError)
 UNINTERESTING_FEATURE(AllowRuntimeSymbolDeclarations)
 
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -6375,8 +6375,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
+```
