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

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood 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
----------------
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?


================
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.
----------------
This block comment mentions functions, but the documentation only mentions variables.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-use-inline-const-variables-in-headers.rst:6
+
+Finds non-inline const variables definitions and extern const variables
+declarations in header in header files. Non-inline const variables make
----------------
Eugene.Zelenko wrote:
> Please synchronize first statement with Release Notes.
...`const variable definitions`...`const variable declarations`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-use-inline-const-variables-in-headers.rst:10
+includes the header, which can lead to subtle violation of the ODR.
+Extern const variables is a deprecated way to define a constant since C++17.
+
----------------
...`variables are a deprecated way`...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118743/new/

https://reviews.llvm.org/D118743