[PATCH] D54912: [attributes] Add a family of OS_CONSUMED, OS_RETURNS and OS_RETURNS_RETAINED attributes

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added inline comments.


================
Comment at: cfe/trunk/lib/Sema/SemaDeclAttr.cpp:4862
+    case ParsedAttr::AT_OSReturnsNotRetained:
       ExpectedDeclKind = ExpectedFunctionOrMethod;
       break;
----------------
george.karpenkov wrote:
> aaron.ballman wrote:
> > george.karpenkov wrote:
> > > aaron.ballman wrote:
> > > > I mentioned that I could fix this up in another patch, but there's a different subtlety here -- you can apply NSReturnsRetained and friends to types. See test\SemaObjC\attr-ns_returns_retained.m for an example -- where it's being added to a block type. It also seems to appertain to block literals (test\SemaObjC\block-literal-with-attribute.m).
> > > > 
> > > > Should the OS versions also apply to types and block literals?
> > > OS versions should not apply to block literals, neither they should be applicable to block types.
> > Can you add some tests to ensure it's properly diagnosed?
> @aaron.ballman Do you mean something like this?
> 
> ```
> diff --git a/clang/test/Sema/attr-osobject.mm b/clang/test/Sema/attr-osobject.mm
> index dbd9122a8fa..ffb4394e4bc 100644
> --- a/clang/test/Sema/attr-osobject.mm
> +++ b/clang/test/Sema/attr-osobject.mm
> @@ -1,6 +1,4 @@
> -// RUN: %clang_cc1 -fsyntax-only -verify %s
> -
> -// expected-no-diagnostics
> +// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
> 
>  struct S {};
> 
> @@ -9,3 +7,5 @@ @interface I
>    - (S*) generateS __attribute__((os_returns_retained));
>    - (void) takeS:(S*) __attribute__((os_consumed)) s;
>  @end
> +
> +typedef __attribute__((os_returns_retained)) id (^blockType)(); // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, and Objective-C properties}}
> ```
Yup! Also:
```
__auto_type b = ^ id (id filter)  __attribute__((os_returns_retained))  {
  return filter;
};
```
This is accepted by ns_returns_retained, so it would be good to clearly show it's not supported by os_returns_retained.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54912