[arcmt] More automatic transformations and safety improvements; rdar://9615812 :

- Replace calling -zone with 'nil'. -zone is obsolete in ARC.
- Allow removing retain/release on a static global var.
- Fix assertion hit when scanning for name references outside a NSAutoreleasePool scope.
- Automatically add bridged casts for results of objc method calls and when calling CFRetain, for example:

NSString *s;
CFStringRef ref = [s string];   -> CFStringRef ref = (__bridge CFStringRef)([s string]);
ref = s.string;                 -> ref = (__bridge CFStringRef)(s.string);
ref = [NSString new];           -> ref = (__bridge_retained CFStringRef)([NSString new]);
ref = [s newString];            -> ref = (__bridge_retained CFStringRef)([s newString]);
ref = [[NSString alloc] init];  -> ref = (__bridge_retained CFStringRef)([[NSString alloc] init]);
ref = [[s string] retain];      -> ref = (__bridge_retained CFStringRef)([s string]);
ref = CFRetain(s);              -> ref = (__bridge_retained CFTypeRef)(s);
ref = [s retain];               -> ref = (__bridge_retained CFStringRef)(s);

- Emit migrator error when trying to cast to CF type the result of autorelease/release:
  for

CFStringRef f3() {
  return (CFStringRef)[[[NSString alloc] init] autorelease];
}

emits:

t.m:12:10: error: [rewriter] it is not safe to cast to 'CFStringRef' the result of 'autorelease' message; a __bridge cast may result in a pointer to a destroyed object and a __bridge_retained may leak the object
  return (CFStringRef)[[[NSString alloc] init] autorelease];
         ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
t.m:12:3: note: [rewriter] remove the cast and change return type of function to 'NSString *' to have the object automatically autoreleased
  return (CFStringRef)[[[NSString alloc] init] autorelease];
  ^

- Before changing attributes to weak/unsafe_unretained, check if the backing ivar
  is set to a +1 object, in which case use 'strong' instead.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@136208 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 73 insertions, 10 deletions

diff --git a/lib/ARCMigrate/TransProperties.cpp b/lib/ARCMigrate/TransProperties.cpp
index 365168e933..943eea2646 100644
--- a/lib/ARCMigrate/TransProperties.cpp
+++ b/lib/ARCMigrate/TransProperties.cpp
@@ -45,6 +45,7 @@ namespace {
 
 class PropertiesRewriter {
   MigrationPass &Pass;
+  ObjCImplementationDecl *CurImplD;
 
   struct PropData {
     ObjCPropertyDecl *PropD;
@@ -62,6 +63,7 @@ public:
   PropertiesRewriter(MigrationPass &pass) : Pass(pass) { }
 
   void doTransform(ObjCImplementationDecl *D) {
+    CurImplD = D;
     ObjCInterfaceDecl *iface = D->getClassInterface();
     if (!iface)
       return;
@@ -134,8 +136,16 @@ private:
       return;
     }
 
-    if (propAttrs & ObjCPropertyDecl::OBJC_PR_assign)
+    if (propAttrs & ObjCPropertyDecl::OBJC_PR_assign) {
+      if (hasIvarAssignedAPlusOneObject(props)) {
+        rewriteAttribute("assign", "strong", atLoc);
+        return;
+      }
       return rewriteAssign(props, atLoc);
+    }
+
+    if (hasIvarAssignedAPlusOneObject(props))
+      return maybeAddStrongAttr(props, atLoc);
 
     return maybeAddWeakOrUnsafeUnretainedAttr(props, atLoc);
   }
@@ -162,15 +172,15 @@ private:
   void maybeAddWeakOrUnsafeUnretainedAttr(PropsTy &props,
                                           SourceLocation atLoc) const {
     ObjCPropertyDecl::PropertyAttributeKind propAttrs = getPropertyAttrs(props);
-    if ((propAttrs & ObjCPropertyDecl::OBJC_PR_readonly) &&
-        hasNoBackingIvars(props))
-      return;
 
     bool canUseWeak = canApplyWeak(Pass.Ctx, getPropertyType(props));
-    bool addedAttr = addAttribute(canUseWeak ? "weak" : "unsafe_unretained",
-                                  atLoc);
-    if (!addedAttr)
-      canUseWeak = false;
+    if (!(propAttrs & ObjCPropertyDecl::OBJC_PR_readonly) ||
+        !hasAllIvarsBacked(props)) {
+      bool addedAttr = addAttribute(canUseWeak ? "weak" : "unsafe_unretained",
+                                    atLoc);
+      if (!addedAttr)
+        canUseWeak = false;
+    }
 
     for (PropsTy::iterator I = props.begin(), E = props.end(); I != E; ++I) {
       if (isUserDeclared(I->IvarD))
@@ -186,6 +196,25 @@ private:
     }
   }
 
+  void maybeAddStrongAttr(PropsTy &props, SourceLocation atLoc) const {
+    ObjCPropertyDecl::PropertyAttributeKind propAttrs = getPropertyAttrs(props);
+
+    if (!(propAttrs & ObjCPropertyDecl::OBJC_PR_readonly) ||
+        !hasAllIvarsBacked(props)) {
+      addAttribute("strong", atLoc);
+    }
+
+    for (PropsTy::iterator I = props.begin(), E = props.end(); I != E; ++I) {
+      if (I->ImplD) {
+        Pass.TA.clearDiagnostic(diag::err_arc_assign_property_ownership,
+                                I->ImplD->getLocation());
+        Pass.TA.clearDiagnostic(
+                           diag::err_arc_objc_property_default_assign_on_object,
+                           I->ImplD->getLocation());
+      }
+    }
+  }
+
   bool rewriteAttribute(StringRef fromAttr, StringRef toAttr,
                         SourceLocation atLoc) const {
     if (atLoc.isMacroID())
@@ -290,6 +319,40 @@ private:
     return true;
   }
 
+  class PlusOneAssign : public RecursiveASTVisitor<PlusOneAssign> {
+    ObjCIvarDecl *Ivar;
+  public:
+    PlusOneAssign(ObjCIvarDecl *D) : Ivar(D) {}
+
+    bool VisitBinAssign(BinaryOperator *E) {
+      Expr *lhs = E->getLHS()->IgnoreParenImpCasts();
+      if (ObjCIvarRefExpr *RE = dyn_cast<ObjCIvarRefExpr>(lhs)) {
+        if (RE->getDecl() != Ivar)
+          return true;
+
+      ImplicitCastExpr *implCE = dyn_cast<ImplicitCastExpr>(E->getRHS());
+      while (implCE && implCE->getCastKind() ==  CK_BitCast)
+        implCE = dyn_cast<ImplicitCastExpr>(implCE->getSubExpr());
+
+      if (implCE && implCE->getCastKind() == CK_ObjCConsumeObject)
+        return false;
+      }
+
+      return true;
+    }
+  };
+
+  bool hasIvarAssignedAPlusOneObject(PropsTy &props) const {
+    for (PropsTy::iterator I = props.begin(), E = props.end(); I != E; ++I) {
+      PlusOneAssign oneAssign(I->IvarD);
+      bool notFound = oneAssign.TraverseDecl(CurImplD);
+      if (!notFound)
+        return true;
+    }
+
+    return false;
+  }
+
   bool hasIvarWithExplicitOwnership(PropsTy &props) const {
     for (PropsTy::iterator I = props.begin(), E = props.end(); I != E; ++I) {
       if (isUserDeclared(I->IvarD)) {
@@ -304,9 +367,9 @@ private:
     return false;    
   }
 
-  bool hasNoBackingIvars(PropsTy &props) const {
+  bool hasAllIvarsBacked(PropsTy &props) const {
     for (PropsTy::iterator I = props.begin(), E = props.end(); I != E; ++I)
-      if (I->IvarD)
+      if (!isUserDeclared(I->IvarD))
         return false;
 
     return true;