[clang-tidy] Check request: performance-use-inline-constexpr-in-headers

[denzor200]

C++17 suggests us the possibility to use inline on constexpr variables, but pre-C++17 codebases (or code unaware of this rule) often omit inline in headers. This can silently bloat binaries and harm performance due to multiple symbol definitions.
Need a check to enforce inline on constexpr variables in header files.

BEFORE:

// header.h
constexpr int MAX_SIZE = 1024; 

AFTER:

// header.h
inline constexpr int MAX_SIZE = 1024; 

Scope

• Applies only to headers (.h, .hpp, etc.), ignoring .cpp files.
• Targets constexpr variables in global/namespace scope (not function locals).

Why performance-?

• Binary size (reduces duplicate symbols).
• Cache behavior (fewer redundant constants in memory).

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Denis Mikhailov (denzor200)

C++17 suggests us the possibility to use `inline` on `constexpr` variables, but pre-C++17 codebases (or code unaware of this rule) often omit inline in headers. This can silently bloat binaries and harm performance due to multiple symbol definitions. Need a check to enforce `inline` on `constexpr` variables in header files.

BEFORE:

// header.h
constexpr int MAX_SIZE = 1024; 

AFTER:

// header.h
inline constexpr int MAX_SIZE = 1024; 

Scope

• Applies only to headers (.h, .hpp, etc.), ignoring .cpp files.
• Targets constexpr variables in global/namespace scope (not function locals).

Why performance-?

• Binary size (reduces duplicate symbols).
• Cache behavior (fewer redundant constants in memory).

[denzor200]

Also this check should support template variables.

BEFORE:

// header.h
template<typename T>
constexpr int MAX_SIZE = calculate_max_size<T>(); 

AFTER:

// header.h
template<typename T>
inline constexpr int MAX_SIZE = calculate_max_size<T>(); 

Look at interesting cases in Boost Libraries:
boostorg/pfr@7ba81ad
https://github.com/boostorg/asio/pull/429/files

[carlosgalvezp]

This check can only be enabled in C++17, since there's no inline variables until then.

Also, this check belongs in "misc", not "performance", since it fixes more things than that (e.g. UB due to ODR violations).

[denzor200]

This check can only be enabled in C++17, since there's no inline variables until then.

Let it be enabled before C++17 because that code with ODR violations can be written before C++17. But the fixit with inline insertion will be provided only for C++17 and newer.

Also, this check belongs in "misc", not "performance", since it fixes more things than that (e.g. UB due to ODR violations).

Isn't UB not reason to put it into "bugprone"?

[denzor200]

Wait, we already have "misc-definitions-in-headers", then do we really need to create separate check?
Looks like extending of the existing checker would be more appropriate.

Moreover, "misc-definitions-in-headers" has this statement in it's doc:

Please note that the addition of the inline keyword to variables is not currently supported by this check.

[denzor200]

But also I found some incorrect samples in the doc :)

// OK: internal linkage variable definitions are ignored for now.
// Although these might also cause ODR violations, we can be less certain and
// should try to keep the false-positive rate down.
...
constexpr int k = 1;

// OK: C++14 variable templates are inline.
template <class T>
constexpr T pi = T(3.1415926L);

https://clang.llvm.org/extra/clang-tidy/checks/misc/definitions-in-headers.html

[carlosgalvezp]

can be written before C++17

How so?

I agree with should extend misc-definitions-in-headers to warn and fix about this use case from C++17 onwards.

[denzor200]

How so?

This code successfuly compiles with pre C++17 compilers

constexpr int MAX_SIZE = 1024;

As you mentioned before, in a header file this code might lead to ODR violation.

I agree with should extend misc-definitions-in-headers to warn and fix about this use case from C++17 onwards.

Oke, I will change the description later.

[carlosgalvezp]

This code successfuly compiles with pre C++17 compilers

Yes, but my question was how it should be fixed before C++17.

IMO the check should not warn before C++17, because the warning is not (easily) actionable. This will create a lot of noise for low benefit. Code works well in practice unless strange things are done like storing the address of these constants.

Alternatively, in C++14, it should warn on very limited use cases where we know for a fact that an ODR violation is taking place. It should also tell the user what to do instead.

But like I said, I think we should prioritize implementing C++17 first, which is easily actionable by users. We can iterate later and add more checks for earlier standards if deemed worth the effort.

[denzor200]

But like I said, I think we should prioritize implementing C++17 first, which is easily actionable by users. We can iterate later and add more checks for earlier standards if deemed worth the effort.

Oke, let's extend firstly "misc-definitions-in-headers" to warn and fix C++17 code and newer.
After that we will think what should we do with C++14 code and later.