[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*))

include/swift/ClangImporter/ClangImporter.h

@@ -705,6 +705,9 @@ bool isCxxConstReferenceType(const clang::Type *type);
 /// Determine whether this typedef is a CF type.
 bool isCFTypeDecl(const clang::TypedefNameDecl *Decl);
 
+// Determine whether type is a c++ foreign reference type.
+bool isForeignReferenceType(const clang::QualType type);
+
 /// Determine the imported CF type for the given typedef-name, or the empty
 /// string if this is not an imported CF type name.
 llvm::StringRef getCFTypeName(const clang::TypedefNameDecl *decl);

lib/ClangImporter/ClangImporter.cpp

@@ -7440,17 +7440,21 @@ static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
          });
 }
 
-// Is this a pointer to a foreign reference type.
-static bool isForeignReferenceType(const clang::QualType type) {
-  if (!type->isPointerType())
-    return false;
+namespace swift {
+  namespace importer {
+    // Is this a pointer to a foreign reference type.
+    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;
+      auto pointeeType =
+          dyn_cast<clang::RecordType>(type->getPointeeType().getCanonicalType());
+      if (pointeeType == nullptr)
+        return false;
 
-  return hasImportAsRefAttr(pointeeType->getDecl());
+      return hasImportAsRefAttr(pointeeType->getDecl());
+    }
+  }
 }
 
 static bool hasSwiftAttribute(const clang::Decl *decl, StringRef attr) {

lib/ClangImporter/ImportDecl.cpp

@@ -25,6 +25,7 @@
 #include "swift/AST/ConformanceLookup.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/DiagnosticsClangImporter.h"
+#include "swift/ClangImporter/ClangImporter.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/GenericEnvironment.h"
@@ -96,6 +97,8 @@ struct AccessorInfo {
 
 } // end anonymous namespace
 
+// extern bool isForeignReferenceType(const clang::QualType type);
+
 static bool isInSystemModule(const DeclContext *D) {
   return cast<ClangModuleUnit>(D->getModuleScopeContext())->isSystemModule();
 }
@@ -2685,7 +2688,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 +3360,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 +3372,26 @@ namespace {
             }
           }
         }
+      }
 
-        if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
-          HeaderLoc loc(decl->getLocation());
-          Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
-                        decl);
+      HeaderLoc loc(decl->getLocation());
+      if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
+        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);
     }
 
+
     /// 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