[PATCH] D58612: Make the static counters in ASTContext non-static

[Alexander Kornienko via Phabricator via llvm-commits]

alexfh added a comment.

In D58612#1409216 <https://reviews.llvm.org/D58612#1409216>, @riccibruno wrote:

> In D58612#1409215 <https://reviews.llvm.org/D58612#1409215>, @alexfh wrote:
>
> > > In D58612#1409024 <https://reviews.llvm.org/D58612#1409024>, @riccibruno wrote:
> > > 
> > >> ...
> > >>  For example in `DeclBase.cpp`
> > >>
> > >>   #define DECL(DERIVED, BASE) static int n##DERIVED##s = 0;
> > >>   #define ABSTRACT_DECL(DECL)
> > >>   #include "clang/AST/DeclNodes.inc"
> > >>
> > >>
> > >> ...
> > > 
> > > 
> > > These are probably easier to convert to std::atomic<int>, but I'd do this separately.
> >
> > Just converting these to `std::atomic<>` will mute TSan, but won't fix the underlying issue: the stats should be collected per-compiler instance. Initialization will require a bit more locking as well. Not sure what would be a good solution here.
>
>
> I agree that making them atomic is not a good solution. I am not sure that the stats should be stored in the compiler instance (is that what you are suggesting ?). The number of declaration nodes of each kind seems to be something that should be stored in the AST context.


Well, I wasn't talking about storing the metrics in the `CompilerInstance` class directly. Maybe in `ASTContext`. But after looking a bit closer I see that the problem is that an instance of `ASTContext` is not readily available where these stats are gathered. I'm not sure I'll get to change this any time soon though.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58612/new/

https://reviews.llvm.org/D58612