swiftlang · fahadnayyar · Nov 1, 2024 · Oct 1, 2024
@@ -258,8 +258,24 @@ 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 *))
+
+ERROR(returns_retained_or_returns_unretained_for_non_cxx_frt_values, none,
+      "%0 cannot be annotated with either swift_attr('returns_retained') or "
+      "swift_attr('returns_unretained') attribute because it is not returning "
+      "a 'SWIFT_SHARED_REFERENCE' type",
+      (const clang::NamedDecl *))
+
+// TODO: make this case an error in next cxx-interop versions rdar://138806722
+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*))

@@ -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 isForeignReferenceTypeWithoutImmortalAttrs(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);

@@ -7446,6 +7446,31 @@ static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
          });
 }
 
+// TODO: Move all these utility functions in a new file ClangImporterUtils.h
+// rdar://138803759
+static bool hasImmortalAtts(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() == "retain:immortal" ||
+                    swiftAttr->getAttribute() == "release:immortal";
+           return false;
+         });
+}
+
+// Is this a pointer to a foreign reference type.
+bool importer::isForeignReferenceTypeWithoutImmortalAttrs(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()) &&
+         !hasImmortalAtts(pointeeType->getDecl());
+}
+
 // Is this a pointer to a foreign reference type.
 static bool isForeignReferenceType(const clang::QualType type) {
   if (!type->isPointerType())

@@ -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"
@@ -2666,7 +2667,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,
@@ -3335,13 +3336,11 @@ namespace {
         return property->getParsedAccessor(AccessorKind::Set);
       }
 
-      // If a C++ decl is annotated with both swift_attr("returns_retained") and
-      // 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.
+      // Emit diagnostics for incorrect usage of "returns_unretained" and
+      // "returns_unretained" attributes
+      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") {
@@ -3351,11 +3350,25 @@ namespace {
             }
           }
         }
+      }
 
+      HeaderLoc loc(decl->getLocation());
+      if (isForeignReferenceTypeWithoutImmortalAttrs(decl->getReturnType())) {
         if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) {
-          HeaderLoc loc(decl->getLocation());
           Impl.diagnose(loc, diag::both_returns_retained_returns_unretained,
                         decl);
+        } else if (!returnsRetainedAttrIsPresent &&
+                   !returnsUnretainedAttrIsPresent) {
+          Impl.diagnose(loc, diag::no_returns_retained_returns_unretained,
+                        decl);
+        }
+      } else {
+        if (returnsRetainedAttrIsPresent || returnsUnretainedAttrIsPresent) {
+          Impl.diagnose(
+              loc,
+              diag::
+                  returns_retained_or_returns_unretained_for_non_cxx_frt_values,
+              decl);
         }
       }
 

@@ -158,28 +158,79 @@ struct
       __attribute__((swift_attr("returns_unretained")));
 };
 
+// A c++ struct not annotated with SWIFT_SHARED_REFERENCE,
+// SWIFT_IMMORTAL_REFERENCE or SWIFT_UNSAFE_REFERENCE
 struct NonFRTStruct {};
 
+// A c++ struct annotated with SWIFT_IMMORTAL_REFERENCE
+struct ImmortalRefStruct {
+} __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:immortal")))
+__attribute__((swift_attr("release:immortal")));
+
+// A c++ struct annotated with SWIFT_UNSAFE_REFERENCE
+struct UnsafeRefStruct {
+} __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:immortal")))
+__attribute__((swift_attr("release:immortal")))
+__attribute__((swift_attr("unsafe")));
+
+// C++ APIs returning cxx frts (for testing diagnostics)
+struct StructWithAPIsReturningCxxFrt {
+  static FRTStruct *_Nonnull StaticMethodReturningCxxFrt();
+  static FRTStruct *_Nonnull StaticMethodReturningCxxFrtWithAnnotation()
+      __attribute__((swift_attr("returns_retained")));
+};
+
+FRTStruct *_Nonnull global_function_returning_cxx_frt();
+FRTStruct *_Nonnull global_function_returning_cxx_frt_with_annotations()
+    __attribute__((swift_attr("returns_retained")));
+
+// C++ APIs returning non-cxx-frts (for testing diagnostics)
+struct StructWithAPIsReturningNonCxxFrt {
+  static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrt();
+  static NonFRTStruct *_Nonnull StaticMethodReturningNonCxxFrtWithAnnotation()
+      __attribute__((swift_attr("returns_retained")));
+};
+
+NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt();
+NonFRTStruct *_Nonnull global_function_returning_non_cxx_frt_with_annotations()
+    __attribute__((swift_attr("returns_retained")));
+
+// C++ APIs returning SWIFT_IMMORTAL_REFERENCE types (for testing diagnostics)
+struct StructWithAPIsReturningImmortalReference {
+  static ImmortalRefStruct *_Nonnull StaticMethodReturningImmortalReference();
+  static ImmortalRefStruct
+      *_Nonnull StaticMethodReturningImmortalReferenceWithAnnotation()
+          __attribute__((swift_attr("returns_retained")));
+};
+
+ImmortalRefStruct *_Nonnull global_function_returning_immortal_reference();
+ImmortalRefStruct
+    *_Nonnull global_function_returning_immortal_reference_with_annotations()
+        __attribute__((swift_attr("returns_retained")));
+
+// C++ APIs returning SWIFT_UNSAFE_REFERENCE types (for testing diagnostics)
+struct StructWithAPIsReturningUnsafeReference {
+  static UnsafeRefStruct *_Nonnull StaticMethodReturningUnsafeReference();
+  static UnsafeRefStruct
+      *_Nonnull StaticMethodReturningUnsafeReferenceWithAnnotation()
+          __attribute__((swift_attr("returns_retained")));
+};
+
+UnsafeRefStruct *_Nonnull global_function_returning_unsafe_reference();
+UnsafeRefStruct
+    *_Nonnull global_function_returning_unsafe_reference_with_annotations()
+        __attribute__((swift_attr("returns_retained")));
+
 // Global/free C++ functions returning non-FRT
 NonFRTStruct *_Nonnull global_function_returning_non_FRT();
-NonFRTStruct
-    *_Nonnull global_function_returning_non_FRT_with_attr_returns_retained()
-        __attribute__((swift_attr("returns_retained")));
-NonFRTStruct
-    *_Nonnull global_function_returning_non_FRT_with_attr_returns_unretained()
-        __attribute__((swift_attr("returns_unretained")));
 NonFRTStruct *_Nonnull global_function_returning_non_FRT_create();
 NonFRTStruct *_Nonnull global_function_returning_non_FRT_copy();
 
 // Struct having static method returning non-FRT
 struct StructWithStaticMethodsReturningNonFRT {
   static NonFRTStruct *_Nonnull StaticMethodReturningNonFRT();
-  static NonFRTStruct
-      *_Nonnull StaticMethodReturningNonFRTWithAttrReturnsRetained()
-          __attribute__((swift_attr("returns_retained")));
-  static NonFRTStruct
-      *_Nonnull StaticMethodReturningNonFRTWithAttrReturnsUnretained()
-          __attribute__((swift_attr("returns_unretained")));
   static NonFRTStruct *_Nonnull StaticMethodReturningNonFRT_create();
   static NonFRTStruct *_Nonnull StaticMethodReturningNonFRT_copy();
 };

@@ -8,3 +8,36 @@ 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 = StructWithAPIsReturningCxxFrt.StaticMethodReturningCxxFrt()
+// CHECK: warning: 'StaticMethodReturningCxxFrt' 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 = StructWithAPIsReturningCxxFrt.StaticMethodReturningCxxFrtWithAnnotation()
+
+let frtLocalVar5 = global_function_returning_cxx_frt()
+// CHECK: warning: 'global_function_returning_cxx_frt' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes
+let frtLocalVar6 = global_function_returning_cxx_frt_with_annotations()
+
+let frtLocalVar7 = StructWithAPIsReturningNonCxxFrt.StaticMethodReturningNonCxxFrt()
+let frtLocalVar8 = StructWithAPIsReturningNonCxxFrt.StaticMethodReturningNonCxxFrtWithAnnotation()
+// CHECK: error: 'StaticMethodReturningNonCxxFrtWithAnnotation' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
+
+let frtLocalVar9 = global_function_returning_non_cxx_frt()
+let frtLocalVar10 = global_function_returning_non_cxx_frt_with_annotations()
+// CHECK: error: 'global_function_returning_non_cxx_frt_with_annotations' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
+
+let frtLocalVar11 = StructWithAPIsReturningImmortalReference.StaticMethodReturningImmortalReference()
+let frtLocalVar12 = StructWithAPIsReturningImmortalReference.StaticMethodReturningImmortalReferenceWithAnnotation()
+// CHECK: error: 'StaticMethodReturningImmortalReferenceWithAnnotation' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
+
+let frtLocalVar13 = global_function_returning_immortal_reference()
+let frtLocalVar14 = global_function_returning_immortal_reference_with_annotations()
+// CHECK: error: 'global_function_returning_immortal_reference_with_annotations' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
+
+let frtLocalVar15 = StructWithAPIsReturningUnsafeReference.StaticMethodReturningUnsafeReference()
+let frtLocalVar16 = StructWithAPIsReturningUnsafeReference.StaticMethodReturningUnsafeReferenceWithAnnotation()
+// CHECK: error: 'StaticMethodReturningUnsafeReferenceWithAnnotation' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
+
+let frtLocalVar17 = global_function_returning_unsafe_reference()
+let frtLocalVar18 = global_function_returning_unsafe_reference_with_annotations()
+// CHECK: error: 'global_function_returning_unsafe_reference_with_annotations' cannot be annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attribute because it is not returning a 'SWIFT_SHARED_REFERENCE' type
+
@@ -167,12 +167,6 @@ func testFreeFunctionsReturningNonFRT() {
     let frtLocalVar1 = global_function_returning_non_FRT()
     // CHECK: function_ref @{{.*}}global_function_returning_non_FRT{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
 
-    let frtLocalVar2 = global_function_returning_non_FRT_with_attr_returns_retained()
-    // CHECK: function_ref @{{.*}}global_function_returning_non_FRT_with_attr_returns_retained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
-
-    let frtLocalVar3 = global_function_returning_non_FRT_with_attr_returns_unretained()
-    // CHECK: function_ref @{{.*}}global_function_returning_non_FRT_with_attr_returns_unretained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
-
     let frtLocalVar4 = global_function_returning_non_FRT_create()
     // CHECK: function_ref @{{.*}}global_function_returning_non_FRT_create{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
 
@@ -184,12 +178,6 @@ func testStaticMethodsReturningNonFRT() {
     let frtLocalVar1 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRT()
     // CHECK: function_ref @{{.*}}StaticMethodReturningNonFRT{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
 
-    let frtLocalVar2 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRTWithAttrReturnsRetained()
-    // CHECK: function_ref @{{.*}}StaticMethodReturningNonFRTWithAttrReturnsRetained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
-
-    let frtLocalVar3 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRTWithAttrReturnsUnretained()
-    // CHECK: function_ref @{{.*}}StaticMethodReturningNonFRTWithAttrReturnsUnretained{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct> 
-
     let frtLocalVar4 = StructWithStaticMethodsReturningNonFRT.StaticMethodReturningNonFRT_create()
     // CHECK: function_ref @{{.*}}StaticMethodReturningNonFRT_create{{.*}} : $@convention(c) () -> UnsafeMutablePointer<NonFRTStruct>