[PATCH] D16708: Add a new attribute CFNoRelease.

[Michael Gottesman via cfe-commits]

> On Jan 29, 2016, at 6:02 AM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
> 
> aaron.ballman added a subscriber: aaron.ballman.
> aaron.ballman added a reviewer: aaron.ballman.
> aaron.ballman added a comment.
> 
> It's a bit strange to add an attribute that has absolutely no semantic effect whatsoever. Where is this attribute intended to be queried within the compiler? Are there additional functionality patches coming for this?

Yes there is a forthcoming patch for CodeGen which will place an attribute on the relevant functions. The attribute will be queried in the middle end optimizer. The reason why there has been a bit of a delay is I realized I wanted to talk to a few more people about this attribute internally.

We may want to expand its use to essentially mean "no-arc", i.e. this is a c function that uses pure c-code.

There are other possibilities as well.

> 
> 
> ================
> Comment at: include/clang/Basic/Attr.td:540
> @@ +539,3 @@
> +  let Subjects = SubjectList<[Function]>;
> +  let Documentation = [Undocumented];
> +}
> ----------------
> Please, no undocumented new attributes. You should modify AttrDocs.td and include that reference here.

I was just following the model of the code that was already there and documented the attribute in a comment above the attribute.

I am fine with adding specific documentation for the attribute.

> 
> ================
> Comment at: test/SemaObjC/attr-cf_no_release.m:31
> @@ +30,2 @@
> +                  void *d CF_NO_RELEASE, // expected-warning{{'cf_no_release' attribute only applies to functions}}
> +                  CFFooRef e CF_NO_RELEASE); // expected-warning{{'cf_no_release' attribute only applies to functions}}
> ----------------
> Not that lots of test are bad, but a lot of these tests are duplicates and can be removed. For instance, really only need one test for ObjC methods, one for properties, one for params, etc. It would be good to add a test ensuring that the attribute accepts no arguments.

Ok.

Michael

> 
> 
> http://reviews.llvm.org/D16708
> 
> 
>