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

[Aaron Ballman via llvm-commits]

On Wed, Dec 5, 2018 at 8:25 PM George Karpenkov via Phabricator
<reviews at reviews.llvm.org> wrote:
>
> george.karpenkov marked an inline comment as done.
> george.karpenkov added inline comments.
>
>
> ================
> Comment at: cfe/trunk/lib/Sema/SemaDeclAttr.cpp:4862
> +    case ParsedAttr::AT_OSReturnsNotRetained:
>        ExpectedDeclKind = ExpectedFunctionOrMethod;
>        break;
> ----------------
> aaron.ballman wrote:
> > 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.
> @aaron.ballman committed in rL348443

Thank you!

~Aaron