aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/Attr.td:431
@@ +430,3 @@
+ CXX11<"clang", "xray_always_instrument">];
+ let Subjects = SubjectList<[CXXMethod, Function], WarnDiag,
+ "ExpectedFunctionOrMethod">;
----------------
Would this also make sense for ObjC methods?
================
Comment at: include/clang/Basic/Attr.td:432
@@ +431,3 @@
+ let Subjects = SubjectList<[CXXMethod, Function], WarnDiag,
+ "ExpectedFunctionOrMethod">;
+ let Documentation = [Undocumented]; // TODO(dberris): Document this!
----------------
This gives a diagnostic suggesting ObjC methods, so this should either be
removed (it will diagnose properly without it) or the Subjects list should be
altered to include ObjC methods.
================
Comment at: include/clang/Basic/Attr.td:433
@@ +432,3 @@
+ "ExpectedFunctionOrMethod">;
+ let Documentation = [Undocumented]; // TODO(dberris): Document this!
+}
----------------
No new undocumented attributes. Also, TODO usually don't contain user names
when we add them.
================
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+
+def XRayNeverInstrument : InheritableAttr {
+ let Spellings = [GNU<"xray_never_instrument">,
----------------
Same comments here as above.
I kind of think these are a good candidates to combine into a single attribute
with multiple spellings.
```
def XRayInstrument : InheritableAttr {
let Spellings = [GNU<"xray_always_instrument">, ...,
GNU<"xray_never_instrument">, ...];
}
```
I think this because I am guessing they will always appertain to the same
subjects and take the same (lack of) arguments. You can tell which attribute
you have at runtime by looking at the semantic spelling of the attribute. You
could add an accessor to get that in a more friendly fashion if desired.
If my guess is wrong, feel free to ignore this suggestion. :-)
================
Comment at: test/Sema/xray-always-instrument-attr.c:4
@@ +3,2 @@
+
+struct __attribute__((xray_always_instrument)) a { int x; }; //
expected-warning {{'xray_always_instrument' attribute only applies to functions
and methods}}
----------------
Should also be a test showing that it doesn't accept args. Also, CodeGen tests.
http://reviews.llvm.org/D20352
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
