[PATCH] D16708: Add a new attribute CFNoRelease.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 29 06:02:15 PST 2016


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?


================
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.

================
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.


http://reviews.llvm.org/D16708





More information about the cfe-commits mailing list