[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

[Evgeny Shulgin via Phabricator via cfe-commits]

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