[cxx-interop] Warning unannotated C++ APIs returning SWIFT_SHARED_REFERENCE types

Author: fahadnayyar

include/swift/AST/DiagnosticsClangImporter.def

@@ -258,8 +258,16 @@ ERROR(failed_base_method_call_synthesis,none,
       "failed to synthesize call to the base method %0 of type %0",
       (ValueDecl *, ValueDecl *))
 
-ERROR(both_returns_retained_returns_unretained,none,
-      "%0 cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes", (const clang::NamedDecl*))
+ERROR(both_returns_retained_returns_unretained, none,
+      "%0 cannot be annotated with both swift_attr('returns_retained') and "
+      "swift_attr('returns_unretained') attributes",
+      (const clang::NamedDecl *))
+
+WARNING(no_returns_retained_returns_unretained, none,
+        "%0 is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated "
+        "with either swift_attr('returns_retained') or "
+        "swift_attr('returns_unretained') attributes",
+        (const clang::NamedDecl *))
 
 NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
 NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (const clang::NamedDecl*))

lib/ClangImporter/ImportDecl.cpp

@@ -2685,7 +2685,7 @@ namespace {
           Impl.diagnoseTopLevelValue(
               DeclName(Impl.SwiftContext.getIdentifier(releaseOperation.name)));
         }
-      }else if (releaseOperation.kind ==
+      } else if (releaseOperation.kind ==
                  CustomRefCountingOperationResult::tooManyFound) {
         HeaderLoc loc(decl->getLocation());
         Impl.diagnose(loc,
@@ -3357,9 +3357,9 @@ namespace {
       // swift_attr("returns_unretained") then emit an error in the swift
       // compiler. Note: this error is not emitted in the clang compiler because
       // these attributes are used only for swift interop.
+      bool returnsRetainedAttrIsPresent = false;
+      bool returnsUnretainedAttrIsPresent = false;
       if (decl->hasAttrs()) {
-        bool returnsRetainedAttrIsPresent = false;
-        bool returnsUnretainedAttrIsPresent = false;
         for (const auto *attr : decl->getAttrs()) {
           if (const auto *swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
             if (swiftAttr->getAttribute() == "returns_unretained") {
@@ -3369,18 +3369,48 @@ namespace {
             }
           }
         }
+      }
+
+      HeaderLoc loc(decl->getLocation());
+      if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
+        Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
+                      decl);
+      }
 
-        if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
-          HeaderLoc loc(decl->getLocation());
-          Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
-                        decl);
+      if ((!returnsRetainedAttrIsPresent) && (!returnsUnretainedAttrIsPresent)) {
+        if (isForeignReferenceType(decl->getReturnType())) {
+          Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
+                      decl);
         }
       }
 
       return importFunctionDecl(decl, importedName, correctSwiftName,
                                 std::nullopt);
     }
 
+    static bool isForeignReferenceType(const clang::QualType type) {
+      if (!type->isPointerType())
+        return false;
+
+      auto pointeeType =
+          dyn_cast<clang::RecordType>(type->getPointeeType().getCanonicalType());
+      if (pointeeType == nullptr)
+        return false;
+
+      return hasImportAsRefAttr(pointeeType->getDecl());
+    }
+
+    static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
+      return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
+              if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
+                return swiftAttr->getAttribute() == "import_reference" ||
+                        // TODO: Remove this once libSwift hosttools no longer
+                        // requires it.
+                        swiftAttr->getAttribute() == "import_as_ref";
+              return false;
+            });
+    }
+
     /// Handles special functions such as subscripts and dereference operators.
     bool
     processSpecialImportedFunc(FuncDecl *func, ImportedName importedName,

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

@@ -158,6 +158,14 @@ struct
       __attribute__((swift_attr("returns_unretained")));
 };
 
+// C++ APIs returning FRTs but not having annotations
+struct
+    StructWithoutReturnsAnnotations {
+  static FRTStruct *_Nonnull StaticMethodReturningFRT();
+};
+
+FRTStruct *_Nonnull global_function_returning_FRT_WithoutReturnsAnnotations();
+
 struct NonFRTStruct {};
 
 // Global/free C++ functions returning non-FRT

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

@@ -8,3 +8,9 @@ let frtLocalVar1 = global_function_returning_FRT_with_both_attrs_returns_retaine
 
 let frtLocalVar2 = StructWithStaticMethodsReturningFRTWithBothAttributesReturnsRetainedAndReturnsUnretained.StaticMethodReturningFRT()
 // CHECK: error: 'StaticMethodReturningFRT' cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes
+
+let frtLocalVar3 = global_function_returning_FRT_WithoutReturnsAnnotations()
+// CHECK: warning: 'global_function_returning_FRT_WithoutReturnsAnnotations' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes
+
+let frtLocalVar4 = StructWithoutReturnsAnnotations.StaticMethodReturningFRT()
+// CHECK: warning: 'StaticMethodReturningFRT' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes