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 done here, not in `check()`. > I believe that there is no way to use the AST matching language to express > hasConstantDeclaration. You can write a local AST matcher to hoist this functionality into the `check()` call, though. Some of it is already exposed for you, like `hasInitializer()`, `isValueDependent()`, and `isConstexpr()`. ================ Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:39 + return; + Finder->addMatcher(varDecl(hasGlobalStorage()).bind("var"), this); +} ---------------- Do you want to restrict this matcher to only variable declarations that have initializers, or are you also intending for this check to cover cases like: ``` // At file scope. struct S { S(); } s; ``` ================ Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:55 + SourceLocation Loc = Var->getLocation(); + if (!Loc.isValid() || !utils::isPresumedLocInHeaderFile(Loc, *Result.SourceManager, + HeaderFileExtensions)) ---------------- We have an AST matcher for this (`isExpansionInSystemHeader()`). 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