[cxx-interop] Fix type-level ownership annotations for C++ APIs returning reference to frt (#84480)

Type-level ownership annotations
(`SWIFT_RETURNED_AS_UNRETAINED_BY_DEFAULT`) were not suppressing warning
for C++ functions (which are not annotated with
`SWIFT_RETURNS_(UN)RETAIND`) returning references (`&`) to foreign
reference types. The annotation only worked for pointer returns (`*`),
forcing developers to manually annotate every function returning a
reference.
This PR fixes this problem by changing the logic of
`matchSwiftAttrOnRecordPtr` to consider `ReferenceType` also along with
`PointerType`.

This PR also adds comprehensive lit tests for ownership conventions and
diagnostics related to C++ functions that return references to foreign
reference types. These tests were previously not covered by our lit
tests.

rdar://161226212

Author: fahadnayyar

include/swift/ClangImporter/ClangImporter.h

@@ -832,17 +832,24 @@ std::optional<T> matchSwiftAttrConsideringInheritance(
 /// Matches a `swift_attr("...")` on the record type pointed to by the given
 /// Clang type, searching base classes if it's a C++ class.
 ///
-/// \param type A Clang pointer type.
+/// \param type A Clang pointer or reference type.
 /// \param patterns List of attribute name-value pairs to match.
 /// \returns Matched value or std::nullopt.
 template <typename T>
 std::optional<T> matchSwiftAttrOnRecordPtr(
     const clang::QualType &type,
     llvm::ArrayRef<std::pair<llvm::StringRef, T>> patterns) {
+  clang::QualType pointeeType;
   if (const auto *ptrType = type->getAs<clang::PointerType>()) {
-    if (const auto *recordDecl = ptrType->getPointeeType()->getAsRecordDecl()) {
-      return matchSwiftAttrConsideringInheritance<T>(recordDecl, patterns);
-    }
+    pointeeType = ptrType->getPointeeType();
+  } else if (const auto *refType = type->getAs<clang::ReferenceType>()) {
+    pointeeType = refType->getPointeeType();
+  } else {
+    return std::nullopt;
+  }
+
+  if (const auto *recordDecl = pointeeType->getAsRecordDecl()) {
+    return matchSwiftAttrConsideringInheritance<T>(recordDecl, patterns);
   }
   return std::nullopt;
 }

test/Interop/Cxx/foreign-reference/Inputs/frt-reference-returns.h

@@ -0,0 +1,71 @@
+#pragma once
+
+namespace NoAnnotations {
+
+struct RefCountedType {
+  int value;
+} __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:retainRefCounted")))
+__attribute__((swift_attr("release:releaseRefCounted")));
+
+RefCountedType& getRefCountedByRef(); // expected-note {{'getRefCountedByRef()' is defined here}}
+RefCountedType& createRefCountedByRef(); // expected-note {{'createRefCountedByRef()' is defined here}}
+RefCountedType& copyRefCountedByRef(); // expected-note {{'copyRefCountedByRef()' is defined here}}   
+
+} // namespace NoAnnotations
+
+void retainRefCounted(NoAnnotations::RefCountedType* obj);
+void releaseRefCounted(NoAnnotations::RefCountedType* obj);
+
+namespace APIAnnotations {
+
+struct RefCountedType {
+  int value;
+} __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:retainAPIRefCounted")))
+__attribute__((swift_attr("release:releaseAPIRefCounted")));
+
+RefCountedType& createByRef() __attribute__((swift_attr("returns_retained")));
+RefCountedType& getByRef() __attribute__((swift_attr("returns_unretained")));
+
+
+} // namespace APIAnnotations
+
+void retainAPIRefCounted(APIAnnotations::RefCountedType* obj);
+void releaseAPIRefCounted(APIAnnotations::RefCountedType* obj);
+
+namespace TypeAnnotation {
+
+struct RefCountedType {
+  int value;
+} __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:retainTypeRefCounted")))
+__attribute__((swift_attr("release:releaseTypeRefCounted")))
+__attribute__((swift_attr("returned_as_unretained_by_default")));
+
+RefCountedType& getByRef();
+RefCountedType& createByRef();
+RefCountedType& copyByRef();
+
+} // namespace TypeAnnotation
+
+void retainTypeRefCounted(TypeAnnotation::RefCountedType* obj);
+void releaseTypeRefCounted(TypeAnnotation::RefCountedType* obj);
+
+namespace BothAnnotations {
+
+struct RefCountedType {
+  int value;
+} __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:retainBothRefCounted")))
+__attribute__((swift_attr("release:releaseBothRefCounted")))
+__attribute__((swift_attr("returned_as_unretained_by_default")));
+
+// Note: Type default at type level is unretained, but API annotation overrides
+RefCountedType& createByRef() __attribute__((swift_attr("returns_retained")));
+RefCountedType& getByRef();
+
+} // namespace BothAnnotations
+
+void retainBothRefCounted(BothAnnotations::RefCountedType* obj);
+void releaseBothRefCounted(BothAnnotations::RefCountedType* obj);

test/Interop/Cxx/foreign-reference/Inputs/module.modulemap

@@ -87,3 +87,8 @@ module LoggingFrts {
     header "logging-frts.h"
     requires cplusplus
 }
+
+module FRTReferenceReturns {
+  header "frt-reference-returns.h"
+  requires cplusplus
+}

test/Interop/Cxx/foreign-reference/frt-reference-returns-diagnostics.swift

@@ -0,0 +1,31 @@
+// 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
+
+import FRTReferenceReturns
+
+func testNoAnnotations() {
+  let _ = NoAnnotations.getRefCountedByRef()
+  // expected-warning@-1 {{cannot infer the ownership of the returned value, annotate 'getRefCountedByRef()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}}
+  let _ = NoAnnotations.createRefCountedByRef()
+  // expected-warning@-1 {{cannot infer the ownership of the returned value, annotate 'createRefCountedByRef()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}}
+  let _ = NoAnnotations.copyRefCountedByRef()
+  // expected-warning@-1 {{cannot infer the ownership of the returned value, annotate 'copyRefCountedByRef()' with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED}}
+}
+
+func testAPIAnnotations() {
+  let _ = APIAnnotations.createByRef()
+  let _ = APIAnnotations.getByRef()
+}
+
+func testTypeAnnotation() {
+  let _ = TypeAnnotation.getByRef()
+  let _ = TypeAnnotation.createByRef()
+  let _ = TypeAnnotation.copyByRef()
+}
+
+func testBothAnnotations() {
+  let _ = BothAnnotations.createByRef() 
+  let _ = BothAnnotations.getByRef()
+}

test/Interop/Cxx/foreign-reference/frt-reference-returns-sil.swift

@@ -0,0 +1,35 @@
+// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=default -disable-availability-checking -diagnostic-style llvm %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
+
+import FRTReferenceReturns
+
+// Test that the ownership annotations work correctly for reference returns
+
+func useAll() {
+  let _ = NoAnnotations.getRefCountedByRef()
+  // CHECK: sil [clang NoAnnotations.getRefCountedByRef] @{{.*}} : $@convention(c) () -> NoAnnotations.RefCountedType
+  // Note: create/copy rule (default without ownership annotations) does not apply if return type is & to frt
+  let _ = NoAnnotations.createRefCountedByRef()
+  // CHECK: sil [clang NoAnnotations.createRefCountedByRef] @{{.*}} : $@convention(c) () -> NoAnnotations.RefCountedType
+  let _ = NoAnnotations.copyRefCountedByRef()
+  // CHECK: sil [clang NoAnnotations.copyRefCountedByRef] @{{.*}} : $@convention(c) () -> NoAnnotations.RefCountedType
+
+  // APIAnnotations - explicit ownership
+  let _ = APIAnnotations.createByRef()
+  // CHECK: sil [clang APIAnnotations.createByRef] @{{.*}} : $@convention(c) () -> @owned APIAnnotations.RefCountedType
+  let _ = APIAnnotations.getByRef()
+  // CHECK: sil [clang APIAnnotations.getByRef] @{{.*}} : $@convention(c) () -> APIAnnotations.RefCountedType
+
+  // TypeAnnotation - all unretained due to type default
+  let _ = TypeAnnotation.getByRef()
+  // CHECK: sil [clang TypeAnnotation.getByRef] @{{.*}} : $@convention(c) () -> TypeAnnotation.RefCountedType
+  let _ = TypeAnnotation.createByRef()
+  // CHECK: sil [clang TypeAnnotation.createByRef] @{{.*}} : $@convention(c) () -> TypeAnnotation.RefCountedType
+  let _ = TypeAnnotation.copyByRef()
+  // CHECK: sil [clang TypeAnnotation.copyByRef] @{{.*}} : $@convention(c) () -> TypeAnnotation.RefCountedType
+
+  // BothAnnotations - API overrides type
+  let _ = BothAnnotations.createByRef()
+  // CHECK: sil [clang BothAnnotations.createByRef] @{{.*}} : $@convention(c) () -> @owned BothAnnotations.RefCountedType
+  let _ = BothAnnotations.getByRef()
+  // CHECK: sil [clang BothAnnotations.getByRef] @{{.*}} : $@convention(c) () -> BothAnnotations.RefCountedType
+}