[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3492
+def warn_ignored_objc_externally_retained : Warning<
+  "'objc_externally_retained' can only be applied to strong retainable "
+  "object pointer types with automatic storage">, InGroup<IgnoredAttributes>;
----------------
erik.pilkington wrote:
> aaron.ballman wrote:
> > This wording isn't quite right -- it doesn't apply to types, it applies to variables of those types. How about: `... to local variables or parameters of strong, retainable object pointer type`? or something along those lines?
> Sure, that is a bit more clear.
I'd split this into two diagnostics:
- "...can only be applied to local variables and parameters of retainable type" (if the type or decl kind is wrong)
- "...can only be applied to variables with strong ownership" (if the qualifier is wrong)


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:806
+    // If D is pseudo-strong, omit the retain.
+    LLVM_FALLTHROUGH;
+  }
----------------
`EmitARCUnsafeUnretainedScalarExpr` doesn't just not retain the value, it actually triggers different handling, e.g. causing non-autoreleased return values to be immediately released.  Now I think we arguably want that behavior here, but you should be explicit about it in the comment.


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:2340
+        }
       }
 
----------------
Most of this code is there to support an assertion that's no longer true.  Just keep the parts about `lt == OCL_Strong` and `qs.hasConst()`.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:1946
+    if (isInit && isPseudoStrong)
+      Lifetime = Qualifiers::OCL_ExplicitNone;
+
----------------
Where are we allowing assignments into pseudo-strong variables?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865