[PATCH] D16708: Add a new attribute CFNoRelease.

Michael Gottesman via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 31 19:12:54 PST 2016


gottesmm added inline comments.

================
Comment at: include/clang/Basic/Attr.td:540
@@ +539,3 @@
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
----------------
aaron.ballman wrote:
> Please, no undocumented new attributes. You should modify AttrDocs.td and include that reference here.
Ok. I was just following what was done in the surrounding code. I am fine with adding something to AttrDocs.td once we pin down exactly what we want.


================
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}}
----------------
aaron.ballman wrote:
> 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.


http://reviews.llvm.org/D16708





More information about the cfe-commits mailing list