czhang marked 2 inline comments as done. czhang 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. ---------------- 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. 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
