Merge pull request #19167 from jrose-apple/proper-property-properties

[PrintAsObjC] Use 'unsafe_unretained' to print 'unowned', not 'assign'

Author: jrose-apple

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -1027,17 +1027,40 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
     return true;
   }
 
+  /// Returns whether \p ty is the C type \c CFTypeRef, or some typealias
+  /// thereof.
   bool isCFTypeRef(Type ty) {
-    if (ID_CFTypeRef.empty())
-      ID_CFTypeRef = M.getASTContext().getIdentifier("CFTypeRef");
-
     const TypeAliasDecl *TAD = nullptr;
     while (auto aliasTy = dyn_cast<NameAliasType>(ty.getPointer())) {
       TAD = aliasTy->getDecl();
       ty = aliasTy->getSinglyDesugaredType();
     }
 
-    return TAD && TAD->getName() == ID_CFTypeRef && TAD->hasClangNode();
+    if (!TAD || !TAD->hasClangNode())
+      return false;
+
+    if (ID_CFTypeRef.empty())
+      ID_CFTypeRef = M.getASTContext().getIdentifier("CFTypeRef");
+    return TAD->getName() == ID_CFTypeRef;
+  }
+
+  /// Returns true if \p ty can be used with Objective-C reference-counting
+  /// annotations like \c strong and \c weak.
+  bool isObjCReferenceCountableObjectType(Type ty) {
+    if (auto classDecl = ty->getClassOrBoundGenericClass()) {
+      switch (classDecl->getForeignClassKind()) {
+      case ClassDecl::ForeignKind::Normal:
+      case ClassDecl::ForeignKind::RuntimeOnly:
+        return true;
+      case ClassDecl::ForeignKind::CFType:
+        return false;
+      }
+    }
+
+    if (ty->isObjCExistentialType() && !isCFTypeRef(ty))
+      return true;
+
+    return false;
   }
 
   void visitVarDecl(VarDecl *VD) {
@@ -1065,22 +1088,27 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
       os << ", readonly";
 
     // Print the ownership semantics, if relevant.
-    // We treat "unowned" as "assign" (even though it's more like
-    // "safe_unretained") because we want people to think twice about
-    // allowing that object to disappear.
     Type ty = VD->getInterfaceType();
-    if (auto weakTy = ty->getAs<WeakStorageType>()) {
-      auto innerTy = weakTy->getReferentType()->getOptionalObjectType();
-      auto innerClass = innerTy->getClassOrBoundGenericClass();
-      if ((innerClass &&
-           innerClass->getForeignClassKind()!=ClassDecl::ForeignKind::CFType) ||
-          (innerTy->isObjCExistentialType() && !isCFTypeRef(innerTy))) {
-        os << ", weak";
+    if (auto referenceStorageTy = ty->getAs<ReferenceStorageType>()) {
+      switch (referenceStorageTy->getOwnership()) {
+      case ReferenceOwnership::Strong:
+        llvm_unreachable("not represented with a ReferenceStorageType");
+      case ReferenceOwnership::Weak: {
+        auto innerTy =
+            referenceStorageTy->getReferentType()->getOptionalObjectType();
+        if (isObjCReferenceCountableObjectType(innerTy))
+          os << ", weak";
+        break;
+      }
+      case ReferenceOwnership::Unowned:
+      case ReferenceOwnership::Unmanaged:
+        // We treat "unowned" as "unsafe_unretained" (even though it's more
+        // like "safe_unretained") because we want people to think twice about
+        // allowing that object to disappear. "unowned(unsafe)" (and
+        // Swift.Unmanaged, handled below) really are "unsafe_unretained".
+        os << ", unsafe_unretained";
+        break;
       }
-    } else if (ty->is<UnownedStorageType>()) {
-      os << ", assign";
-    } else if (ty->is<UnmanagedStorageType>()) {
-      os << ", unsafe_unretained";
     } else {
       Type copyTy = ty;
       bool isOptional = false;
@@ -1117,10 +1145,7 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
         case FunctionTypeRepresentation::CFunctionPointer:
           break;
         }
-      } else if ((nominal && isa<ClassDecl>(nominal) &&
-                  cast<ClassDecl>(nominal)->getForeignClassKind() !=
-                    ClassDecl::ForeignKind::CFType) ||
-                 (copyTy->isObjCExistentialType() && !isCFTypeRef(copyTy))) {
+      } else if (isObjCReferenceCountableObjectType(copyTy)) {
         os << ", strong";
       }
     }

test/PrintAsObjC/classes.swift

@@ -503,7 +503,7 @@ public class NonObjCClass { }
 // CHECK-NEXT: SWIFT_CLASS_PROPERTY(@property (nonatomic, class, readonly, strong) Properties * _Nonnull sharedRO;)
 // CHECK-NEXT: + (Properties * _Nonnull)sharedRO SWIFT_WARN_UNUSED_RESULT;
 // CHECK-NEXT: @property (nonatomic, weak) Properties * _Nullable weakOther;
-// CHECK-NEXT: @property (nonatomic, assign) Properties * _Nonnull unownedOther;
+// CHECK-NEXT: @property (nonatomic, unsafe_unretained) Properties * _Nonnull unownedOther;
 // CHECK-NEXT: @property (nonatomic, unsafe_unretained) Properties * _Nonnull unmanagedOther;
 // CHECK-NEXT: @property (nonatomic, unsafe_unretained) Properties * _Nullable unmanagedByDecl;
 // CHECK-NEXT: @property (nonatomic, weak) id <MyProtocol> _Nullable weakProto;

test/PrintAsObjC/protocols.swift

@@ -191,7 +191,7 @@ extension NSString : A, ZZZ {}
 @objc class Subclass : RootClass1, ZZZ {}
 
 // CHECK-LABEL: @protocol UnownedProperty
-// CHECK-NEXT: @property (nonatomic, assign) id _Nonnull unownedProp;
+// CHECK-NEXT: @property (nonatomic, unsafe_unretained) id _Nonnull unownedProp;
 @objc protocol UnownedProperty {
   unowned var unownedProp: AnyObject { get set }
 }