Izaron added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp:64-68 + unless(hasType(isVolatileQualified())), // non-volatile + unless(isTemplateVariable()), // non-template + unless(isVarInline()), // non-inline + unless(isExternC()), // not "extern C" variable + unless(isInAnonymousNamespace())); // not within an anonymous namespace ---------------- LegalizeAdulthood wrote: > Comment: Why don't we have a variadic `unless(...)` matcher? > > Is there any utility to reordering these `unless()` clauses in > order of "most likely encountered first" to get an early out > when matching? Thanks, it is more convenient! > reordering these unless() clauses in order of "most likely encountered first" I reordered them based on my own approximal estimation. We could make a benchmark to observe the impact of different orders, but it goes beyond this patch. ================ Comment at: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h:19 + +/// Finds non-extern non-inline function and variable definitions in header +/// files, which can lead to potential ODR violations. ---------------- LegalizeAdulthood wrote: > This block comment mentions functions, but the documentation only mentions > variables. Oops, that's a typo: of course there are nothing abouth functions anywhere in the checker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118743/new/ https://reviews.llvm.org/D118743 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
