[PATCH] D20352: Add XRay flags to Clang

[Aaron Ballman via cfe-commits]

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