[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. In D58612#1409215 , @alexfh wrote: > > In D58612#1409024 , @riccibruno > > wrote: > > > >> ... > >> For example in `DeclBase.cpp` > >> > >> #define DECL(DERIVED, BASE) static int n##

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. > In 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" >> >>

[PATCH] D58612: Use std::atomic<> for static counters.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354795: Make static counters in ASTContext non-static. (authored by alexfh, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. (With a commit message which actually reflect the change of course) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 ___ cfe-commits

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 ___ cfe-commits mailing list cfe-comm

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov added a comment. Awesome, I was also surprised it's so easy to convert them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 ___ cfe-commits maili

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno accepted this revision. riccibruno added a comment. This revision is now accepted and ready to land. Sounds good. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 _

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. In D58612#1409024 , @riccibruno wrote: > In D58612#1408991 , @alexfh wrote: > > > In D58612#1408942 , @riccibruno > > wrote: > > > > > Okay, but what

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. In D58612#1408991 , @alexfh wrote: > In D58612#1408942 , @riccibruno > wrote: > > > Okay, but what about the other similar uses of static members which have > > the same problem ? > > >

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. In D58612#1408942 , @riccibruno wrote: > Okay, but what about the other similar uses of static members which have the > same problem ? Do you have any example in mind? I've only seen TSan warnings for these counters, nothing els

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. Okay, but what about the other similar uses of static members which have the same problem ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 ___

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. In D58612#1408845 , @riccibruno wrote: > In D58612#1408843 , @ilya-biryukov > wrote: > > > In D58612#1408839 , @riccibruno > > wrote: > > > > > I do

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh updated this revision to Diff 188163. alexfh added a comment. Make the counters non-static members of ASTContext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 Files: clang/include/clang/AST/ASTC

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. In D58612#1408843 , @ilya-biryukov wrote: > In D58612#1408839 , @riccibruno > wrote: > > > I don't know how hard it would be to do this, but I would like to argue > > that this should

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov added a comment. In D58612#1408839 , @riccibruno wrote: > I don't know how hard it would be to do this, but I would like to argue that > this should be done even if it require some refactoring. These static > variables used for stats are ki

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. In D58612#1408836 , @ilya-biryukov wrote: > In D58612#1408831 , @riccibruno > wrote: > > > Hmm. These are not the only static variables which are used for statistics > > (eg: in `DeclB

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov added a comment. In D58612#1408831 , @riccibruno wrote: > Hmm. These are not the only static variables which are used for statistics > (eg: in `DeclBase.cpp`). Would it make sense instead to keep all of the > statistics in the AST context (

[PATCH] D58612: Use std::atomic<> for static counters.

riccibruno added a comment. Hmm. These are not the only static variables which are used for statistics (eg: in `DeclBase.cpp`). Would it make sense instead to keep all of the statistics in the AST context (without making them atomic) ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh created this revision. alexfh added a reviewer: ilya-biryukov. Herald added a subscriber: jfb. Herald added a project: clang. Fixes a data race and makes it possible to run clang-based tools in multithreaded environment with TSan. Repository: rG LLVM Github Monorepo https://reviews.llv