lebedev.ri added inline comments.
================ Comment at: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:33-35 + // This may work fine when optimization is enabled because bar() can + // be turned into a constant 7. But without optimization, it can + // cause problems. Therefore, we must err on the side of conservatism. ---------------- czhang wrote: > aaron.ballman wrote: > > czhang wrote: > > > aaron.ballman wrote: > > > > czhang wrote: > > > > > aaron.ballman wrote: > > > > > > What problems can be caused here? Typically, dynamic init is only > > > > > > problematic when it happens before main() is executed (because of > > > > > > initialization order dependencies), but that doesn't apply to local > > > > > > statics. > > > > > Consider the case when synchronization is disabled for static > > > > > initialization, and two threads call `foo2` for the first time. It > > > > > may be the case that they both try and initialize the static variable > > > > > at the same time with different values (since the dynamic initializer > > > > > may not be pure), creating a race condition. > > > > > Consider the case when synchronization is disabled for static > > > > > initialization > > > > > > > > This is a compiler bug, though: > > > > http://eel.is/c++draft/stmt.dcl#4.sentence-3 > > > Sorry, I guess I didn't make it clear enough in the rst documentation > > > file, but this check is for those who explicitly enable the > > > -fno-threadsafe-statics flag because they provide their own > > > synchronization. Then they would like to check if the headers they didn't > > > write may possibly run into this issue when compiling with this flag. > > Ah! Thank you for the explanation. In that case, this behavior makes more > > sense, but I think you should only warn if the user has enabled that > > feature flag rather than always warning. > I haven't been able to find much documentation on how to actually make a tidy > check run against a feature flag. Is it possible to do this? I would think > that said users would manually run this check on their header files. > Sorry, I guess I didn't make it clear enough in the rst documentation file, > but this check is for those who explicitly enable the -fno-threadsafe-statics > flag because they provide their own synchronization. I too want to see this explicitly spelled out in documentation. > Then they would like to check if the headers they didn't write may possibly > run into this issue when compiling with this flag. I'm very much not a fan of this solution. Are you sure that is not exposed in `LangOptions`, e.g. as `ThreadsafeStatics`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
