[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked an inline comment as done.
aaronpuchert added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3016-3017
+def warn_thread_attribute_not_on_capability_member : Warning<
+  "%0 attribute without capability arguments can only be applied in a class "
+  "annotated with 'capability' or 'scoped_lockable' attribute, but %1 isn't">,
+  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
----------------
jmgao wrote:
> aaronpuchert wrote:
> > Alternative wording: "%0 attribute without capability arguments refers to 'this', but %1 isn't annotated with 'capability' or 'scoped_lockable' attribute".
> Maybe something like "implicit 'this' argument for %0 attribute isn't annotated with 'capability' or 'scoped_lockable" attribute"?
I like that it's short, but technically we want the argument's type—not the argument itself—to be annotated and I worry that this might not be clear. In the following warning message we talk about the "argument whose type is annotated" for example.


================
Comment at: test/SemaCXX/warn-thread-safety-parsing.cpp:1286-1287
+
+// FIXME: warn on template instantiation.
+template struct SLTemplateDerived<int>;
+
----------------
The explicit instantiation appears in the AST in its full glory, but I'm not sure how make the analysis run over it. I saw that `Sema::ProcessDeclAttributeList` is called from `Sema::ActOnExplicitInstantiation`, but that seems to check the attributes on the class/function itself, not on members.


Repository:
  rC Clang

https://reviews.llvm.org/D51901