> On Jan 29, 2016, at 6:02 AM, Aaron Ballman <aaron.ball...@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
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
