NoQ added subscribers: george.karpenkov, dcoughlin, NoQ.
NoQ added a comment.

Hmmm, interesting. A checker doesn't usually need to access these specific 
static locals, at least not directly. These are usually accessed through 
functions in .cpp files that are supposed to be compiled with a pointer to the 
correct instance of the static local, and it doesn't seem to be necessary to 
expose them to plugins, simply because i don't immediately see why would a 
plugin want to use them. In this sense, i believe that the entire definition of 
these traits should be moved to .cpp files and be made private, accessed only 
via public methods of respective classes. But i guess it's more difficult and 
it's a separate chunk of work, so i totally approve this patch.

Also, i guess that's what they meant when they were saying that 
REGISTER_TRAIT_WITH_PROGRAMSTATE [and similar] macros shouldn't be used in 
headers (https://reviews.llvm.org/D51388?id=162968#inline-455495).

Did you use any sort of tool to find those? I.e., are there more such bugs, and 
can we prevent this from regressing and breaking your workflow later?

You didn't add reviewers. Does it mean that you are still planning to work on 
this patch further, or do you want this patch to be committed in its current 
shape? Static Analyzer patches are usually prefixed with [analyzer] (a few 
people auto-subscribe to those) and please feel free to add me and 
@george.karpenkov as reviewers, and the code owner is @dcoughlin.


Repository:
  rC Clang

https://reviews.llvm.org/D52905



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to