scripted-diff: Use inline const(expr) over static constexpr in headers #35852

maflcko commented at 5:54 PM on July 30, 2026: member

Both are fine and this refactor doesn't change any behavior.

However, inline constexpr from C++17 will ensure each symbol has a single address across all TU, making the release binary minimally smaller. (For me it is smaller by about 1kB)

DrahtBot renamed this:
~~scripted-diff: Use inline constexpr over static constexpr in headers~~
scripted-diff: Use inline constexpr over static constexpr in headers
on Jul 30, 2026

DrahtBot added the label Refactoring on Jul 30, 2026

DrahtBot commented at 5:55 PM on July 30, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35852.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	fanquake

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35820 (refactor: keep duration calculations typed by l0rinc)
#35793 (Implement BIP 54 (Consensus Cleanup) without mainnet activation by darosior)
#35569 (Encapsulation for CTransaction by purpleKarrot)
#35433 (wallet: deprecate replaceable argument from transaction (and psbt) creation (and modification) RPCs by rkrux)
#35205 (kernel,node: add dbcache setter and clarify defaults by l0rinc)
#34075 (fees: Introduce Mempool Based Fee Estimation to reduce overestimation by ismaelsadeeq)
#33112 (wallet: relax external_signer flag constraints, add musig2 test (partial) by Sjors)
#32729 (test, refactor: extract script template helpers and expand sigop coverage by l0rinc)
#24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake commented at 8:24 PM on July 30, 2026: member

Concept ACK - Could change static const here too? Not sure it's worth adding to the dev notes that inline constexpr is generally preferred (over static constexpr, static const etc)?

in src/util/string.h:26 in fac31e6885 outdated

  22 | @@ -23,7 +23,7 @@
  23 |  namespace util {
  24 |  namespace detail {
  25 |  template <unsigned num_params>
  26 | -constexpr static void CheckNumFormatSpecifiers(const char* str)

l0rinc commented at 8:27 PM on July 30, 2026:

fac31e6 refactor: Manually use inline constexpr over constexpr static:

This is already implicitly inline, if we're changing it in a separate commit, we might as well remove the modifier, preferably as a first commit, which would allow us to fix the other case here in the scripted diff.

And maybe cover https://github.com/bitcoin/bitcoin/blob/fa5f29774872d18febc0df38831a6e45f3de69cc/src/net_permissions.h#L49 as well, similarly removing both qualifiers.

maflcko commented at 8:26 AM on July 31, 2026:

Sure, done both.

in src/qt/guiconstants.h:20 in 6666d4c486

  17 |  /* A delay between shutdown pollings */
  18 | -static constexpr auto SHUTDOWN_POLLING_DELAY{200ms};
  19 | +inline constexpr auto SHUTDOWN_POLLING_DELAY{200ms};
  20 |  
  21 |  /* AskPassphraseDialog -- Maximum passphrase length */
  22 |  static const int MAX_PASSPHRASE_SIZE = 1024;

l0rinc commented at 8:59 PM on July 30, 2026:

fac31e6 refactor: Manually use inline constexpr over constexpr static:

Maybe we could extend the PR to cover these as well (edit: #35852 (comment))

maflcko commented at 8:26 AM on July 31, 2026:

Sure, seems few enough to just include here as well.

in src/key.h:28 in fac31e6885

  24 | @@ -25,7 +25,7 @@ typedef struct secp256k1_context_struct secp256k1_context;
  25 |  typedef std::vector<unsigned char, secure_allocator<unsigned char> > CPrivKey;
  26 |  
  27 |  /** Size of ECDH shared secrets. */
  28 | -constexpr static size_t ECDH_SECRET_SIZE = CSHA256::OUTPUT_SIZE;
  29 | +inline constexpr size_t ECDH_SECRET_SIZE{CSHA256::OUTPUT_SIZE};

l0rinc commented at 9:20 PM on July 30, 2026:

fac31e6 refactor: Manually use inline constexpr over constexpr static:

Could we rather extend the scripted diff to cover this?

<details><summary>extended scripted diff</summary>

sed -E -i 's/^(static constexpr|constexpr static)\>/inline constexpr/g' $( \
  git grep -El '^(static constexpr|constexpr static)' -- \
    '*.h' \
    ':(exclude)src/crc32c' \
    ':(exclude)src/ipc/libmultiprocess' \
    ':(exclude)src/minisketch' \
)

</details>

maflcko commented at 8:26 AM on July 31, 2026:

thx, done

in src/common/bloom.h:1 in 6666d4c486 outdated

l0rinc commented at 9:47 PM on July 30, 2026:

6666d4c scripted-diff: Use inline constexpr over static constexpr:

Verified that all 3 excludes are indeed needed 👍

l0rinc approved

refactor: Remove static from constexpr functions in headers

Those functions have no need to be static in every TU. Also, static
somewhat contradicts and overwrites the inline keyword.

So remove the static, and just keep constexpr (which implies inline).

11111bfca5

scripted-diff: Use inline constexpr over static constexpr

Both are fine and this refactor shouldn't change any behavior.

However, inline constexpr will ensure each symbol has a single address
across all TU, making the release binary smaller.

Review note: In theory the script may also cover functions, but they
were handled in the prior commit, to remove the redundant inline for
them.

-BEGIN VERIFY SCRIPT-
 sed --regexp-extended -i 's/^(static constexpr|constexpr static)\>/inline constexpr/g' $( \
   git grep --extended-regexp -l '^(static constexpr|constexpr static)' -- \
     '*.h' \
     ':(exclude)src/crc32c' \
     ':(exclude)src/ipc/libmultiprocess' \
     ':(exclude)src/minisketch' \
 )
-END VERIFY SCRIPT-

fa8f820a20

refactor: Make CFeeRate(integral) ctor constexpr

This is required for the next commit.

faa600b4c3

scripted-diff: Use inline constexpr over static const

Both are fine and this refactor shouldn't change any behavior.

However, inline constexpr will ensure each symbol has a single address
across all TU, making the release binary smaller.

Note, a follow-up commit will deal with string literals (const char*)
and other static const, which can not be constexpr (e.g. std::vector).

-BEGIN VERIFY SCRIPT-
 type='bool|CAmount|size_t|((signed|unsigned) )?int|u?int[0-9]+_t|std::array|DatabaseFormat|CFeeRate'
 sed -i --regexp-extended "s/^static const (${type})\>/inline constexpr \1/" $( \
   git grep -l "^static const " -- \
     '*.h' \
     ':(exclude)src/leveldb' \
     ':(exclude)src/secp256k1' \
 )
-END VERIFY SCRIPT-

fa41c7da8d

maflcko force-pushed on Jul 31, 2026

maflcko renamed this:
~~scripted-diff: Use inline constexpr over static constexpr in headers~~
scripted-diff: Use inline const(expr) over static constexpr in headers
on Jul 31, 2026

refactor: Use inline constexpr for string literals in headers

This is needed for the next scripted-diff commit, which would otherwise
turn the const from `static const char*` into `inline const char*`.

fad0090339

scripted-diff: Use inline const over static const

Both are fine and this refactor shouldn't change any behavior.

However, inline const will ensure each symbol has a single address
across all TU, making the release binary smaller.

-BEGIN VERIFY SCRIPT-
 sed -i "s/^static const /inline const /" $( \
   git grep -l "^static const " -- \
     '*.h' \
     ':(exclude)src/leveldb' \
     ':(exclude)src/secp256k1' \
 )
-END VERIFY SCRIPT-

fac8a3a4a6

maflcko force-pushed on Jul 31, 2026

DrahtBot added the label CI failed on Jul 31, 2026

DrahtBot removed the label CI failed on Jul 31, 2026

fanquake commented at 11:50 AM on July 31, 2026: member

ACK fac8a3a4a68bc7edabaaab6bfb113199792babb4