[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

This revision was automatically updated to reflect the committed changes. Closed by commit rL369568: [clang-tidy] Check for dynamically initialized statics in headers. (authored by yuanfang, committed by ). Herald added a project: LLVM. Changed prior to commit:

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

ychen added a comment. Since Charlie has completed his internship before LGTM, I'll commit this on his behalf. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 ___ cfe-commits mailing list

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:1 +// RUN: %check_clang_tidy %s

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:1 +// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t + aaron.ballman wrote: > I'm a bit confused. > > 1) Why is this file a

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang updated this revision to Diff 214275. czhang added a comment. Fix test and formatting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:47 +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus || getLangOpts().ThreadsafeStatics) +

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang marked an inline comment as done. czhang added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:47 +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus ||

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:47 +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus || getLangOpts().ThreadsafeStatics) +

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang marked an inline comment as done. czhang added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:47 +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus ||

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:47 +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus || getLangOpts().ThreadsafeStatics) +

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

lebedev.ri added a comment. I think this last update clarified the concerns i raised in last comment. But, did you mean to update the tests? They should have broke now that you require `!getLangOpts().ThreadsafeStatics`. Comment at:

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang updated this revision to Diff 213102. czhang marked an inline comment as done. czhang added a comment. Be more explicit about -fno-threadsafe-statics CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 Files:

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

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 + //

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman 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 + //

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman 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 + //

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang updated this revision to Diff 212890. czhang marked an inline comment as done. czhang added a comment. Add example in documentation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 Files:

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang marked 3 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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst:9 + +This can pose problems in certain multithreaded contexts. Eugene.Zelenko wrote: > Will be good idea to provide example.

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang marked 4 inline comments as done. czhang added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:40 +return; + Finder->addMatcher(varDecl().bind("var"), this); +} aaron.ballman wrote: > czhang

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang updated this revision to Diff 209071. czhang added a comment. Hoist constant initializer check and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 Files:

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:39 +return; + Finder->addMatcher(varDecl(hasGlobalStorage()).bind("var"), this); +} czhang wrote: > aaron.ballman wrote: > > Do you

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang marked an inline comment as done. czhang added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:39 +return; + Finder->addMatcher(varDecl(hasGlobalStorage()).bind("var"), this); +} aaron.ballman

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:40 +return; + Finder->addMatcher(varDecl().bind("var"), this); +} czhang wrote: > lebedev.ri wrote: > > Most of the matching should be

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang added a comment. ping 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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang added a comment. In D62829#1533345 , @lebedev.ri wrote: > Hmm, but there already is clang's `-Wglobal-constructors`, that fires on some > of these: > https://godbolt.org/z/rSnNuu > You are sure those extra warning this check produces ontop of >

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

lebedev.ri added a comment. Hmm, but there already is clang's `-Wglobal-constructors`, that fires on some of these: https://godbolt.org/z/rSnNuu You are sure those extra warning this check produces ontop of `-Wglobal-constructors` are correct? If so, maybe `-Wglobal-constructors` should be

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang marked an inline comment as done. czhang added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:40 +return; + Finder->addMatcher(varDecl().bind("var"), this); +} lebedev.ri wrote: > Most of the

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

czhang updated this revision to Diff 203462. czhang marked an inline comment as done. czhang added a comment. Addressed comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 Files:

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:37 +void DynamicStaticInitializersCheck::registerMatchers(MatchFinder *Finder) { + // FIXME: Add matchers. + if (!getLangOpts().CPlusPlus)

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:40 +return; + Finder->addMatcher(varDecl().bind("var"), this); +} Most of the matching should be done here, not in `check()`.