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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 19 12:45:12 PST 2018

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.
+  }
`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?



More information about the cfe-commits mailing list