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

Erik Pilkington via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 2 16:36:56 PST 2019


erik.pilkington updated this revision to Diff 179964.
erik.pilkington marked 10 inline comments as done.
erik.pilkington added a comment.

Address review comments. Also, make the attribute apply to parameters via the function declaration instead to parameters directly. i.e.:

  __attribute__((objc_externally_retained(1)))
  void foo(Widget *w) { ... }

The rule being that when this is applied to a non-param variable, it behaves as before, but when applied to a function it takes zero or more indices that correspond to which parameters are pseudo-strong. If no indices are specified, then it applies to every parameter that isn't explicitly __strong. This has a couple of advantages:

1. It fixes a more "theoretical" ABI break when used with objective-c++ decltype:

  void foo(Widget *w, decltype(w) *) {}

Previously, adding an attribute to `w` would have caused the mangling of `foo` to change because `decltype(w)` was `Widget * __strong const`. Now, we wait until the `FunctionType` is constructed and the `decltype(w)` is resolved before tampering with the parameter type, so this gets the same mangling, but we still get the extra safety.

2. It makes it more obvious what this attribute can/can't be applied to. i.e. it now is impossible to annotate parameters of function types, which doesn't make any sense.

3. We don't have to add special-case hacks to `#pragma clang attribute` to make this work. i.e.:

  #pragma clang attribute push (__attribute__((objc_externally_retained)), apply_to=any(function,objc_method,block))
  void f(Widget *w, __strong Widget *other) {} // w is externally-retained, other is not
  #pragma clang attribute pop

Whereas previously we would have had to add something like `apply_to=is_objc_retainable_pointer_parameter_of_function_definition_that_is_implicitly_strong` in order to get these semantics.

Thanks for taking a look!


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

https://reviews.llvm.org/D55865

Files:
  clang/docs/AutomaticReferenceCounting.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/externally-retained-no-arc.m
  clang/test/SemaObjC/externally-retained.m

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55865.179964.patch
Type: text/x-patch
Size: 34544 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190103/5f5ea0a8/attachment-0001.bin>


More information about the cfe-commits mailing list