refactor: Make all `const static` class members `constexpr` #35792

pull rustaceanrob wants to merge 1 commits into bitcoin:master from rustaceanrob:static-class-mems changing 24 files +38 −40
  1. rustaceanrob commented at 10:27 AM on July 24, 2026: member

    Found in #35713. If a static class member is not inlined or constexpr, the linker will fail when attempting to ODR-use the constant (passing as const T&). These can be fixed by finding all member variables that are const qualified and inlining them with constexpr. There is a clang-tidy pull request that would lint these callsites: https://github.com/llvm/llvm-project/pull/162741

    A script was used to modify these sites, however it cannot run as a scripted-diff because it uses clang-query and a build folder.

    The script only queries for integer and enumeration types, as other data members would have to be marked constexpr or inline from what I understand: https://en.cppreference.com/cpp/language/static#Constant_static_members

    Removing the ZMQ forward declaration was a clang-tidy lint.

    <details> <summary>The script used to find these sites, LLM assisted:</summary>

    set -uxo pipefail
    
    cd "$(git rev-parse --show-toplevel)"
    
    BUILD=${BUILD:-build}
    if [ ! -f "${BUILD}/compile_commands.json" ]; then
        echo "error: ${BUILD}/compile_commands.json not found. Run cmake -B ${BUILD} first." >&2
        exit 1
    fi
    if ! command -v clang-query >/dev/null; then
        echo "error: clang-query not on PATH. Install clang-tools." >&2
        exit 1
    fi
    if ! git diff --quiet || ! git diff --cached --quiet; then
        echo "error: working tree has uncommitted changes. Commit or stash first." >&2
        exit 1
    fi
    
    MATCHER='match varDecl(hasParent(cxxRecordDecl()),
                          hasType(qualType(isConstQualified(),
                                           anyOf(hasCanonicalType(isInteger()),
                                                 hasDeclaration(enumDecl())))),
                          hasInitializer(expr()),
                          unless(isConstexpr()),
                          isExpansionInFileMatching("/src/"))'
    
    RAW=$(mktemp)
    trap 'rm -f "$RAW"' EXIT
    
    echo "Sweeping TUs (batched, may take a few minutes)..." >&2
    find src -type d \( -name secp256k1 -o -name leveldb -o -name crc32c \
                        -o -name minisketch -o -name libmultiprocess -o -name ctaes \) -prune -o \
            -name '*.cpp' -print0 \
        | xargs -0 -n 50 clang-query -p "${BUILD}" \
              -c 'set output diag' \
              -c "${MATCHER}" \
              >>"$RAW" || true
    
    ROOT=$(pwd)
    LOCS=$(grep -oE "${ROOT}/src/[^:]+:[0-9]+:[0-9]+:" "$RAW" \
        | sed -E "s|^${ROOT}/||; s|:[0-9]+:$||" \
        | sort -u)
    
    if [ -z "$LOCS" ]; then
        echo "no matches" >&2
        exit 0
    fi
    
    FILTERED=""
    while IFS=: read -r file line; do
        case "$file" in
            src/secp256k1/*|src/leveldb/*|src/crc32c/*|src/minisketch/*|src/ipc/libmultiprocess/*|src/crypto/ctaes/*) continue ;;
            src/tinyformat.h) continue ;;
        esac
        src=$(sed -n "${line}p" "$file")
        case "$src" in *inline*) continue ;; esac
        FILTERED+="${file}:${line}"$'\n'
    done <<<"$LOCS"
    FILTERED=$(printf '%s' "$FILTERED" | sed '/^$/d')
    
    if [ -z "$FILTERED" ]; then
        echo "no matches after filtering" >&2
        exit 0
    fi
    
    echo "Sites to rewrite ($(echo "$FILTERED" | wc -l)):" >&2
    echo "$FILTERED" >&2
    
    declare -A LINES
    while IFS=: read -r file line; do
        LINES[$file]+="${line} "
    done <<<"$FILTERED"
    
    for file in "${!LINES[@]}"; do
        args=()
        for line in ${LINES[$file]}; do
            args+=(-e "${line}s/static const /static constexpr /")
        done
        sed -i "${args[@]}" "$file"
    done
    
    echo >&2
    echo "===== proposed diff =====" >&2
    git --no-pager diff
    

    </details>

  2. DrahtBot added the label Refactoring on Jul 24, 2026
  3. DrahtBot commented at 10:27 AM on July 24, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    ACK fanquake, sedited
    Concept ACK hebasto

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. fanquake commented at 10:39 AM on July 24, 2026: member

    Concept ACK - thanks. Did you mean to link to https://github.com/llvm/llvm-project/pull/162741 instead, in the PR description?

  5. rustaceanrob commented at 10:40 AM on July 24, 2026: member

    Yeah, thanks. I see in your commit you brace initialize all of the sites, I'll do that here as well.

  6. DrahtBot added the label CI failed on Jul 24, 2026
  7. hebasto commented at 11:47 AM on July 24, 2026: member

    Concept ACK.

  8. sedited commented at 12:17 PM on July 24, 2026: contributor

    What about tinyformat.h:235? Are we purposefully skipping over variables that cannot be converted to a constexpr, like the std::string in bitcoingui.h:72 ?

    Since we are doing this, I would also suggest converting the one inlined variable to constexpr in utxo_snapshot.h:39

  9. rustaceanrob force-pushed on Jul 24, 2026
  10. rustaceanrob commented at 12:59 PM on July 24, 2026: member

    What about tinyformat.h:235?

    Was a miss on my end

    converting the one inlined variable to constexpr in utxo_snapshot.h:39

    Done

    Are we purposefully skipping over variables that cannot be converted to a constexpr, like the std::string in bitcoingui.h:72 ?

    I think since the definition is not in the class this ODR-use bug is not possible in that case, so yes those were skipped over.

  11. refactor: Make all `const static` class members `constexpr`
    If a `static class` member is not inlined or `constexpr`, the linker
    will fail when attempting to ODR-use the constant (passing as `const
    T&`). These can be fixed by finding all member variables that are
    `const` qualified and inlining them with `constexpr`. There is a
    clang-tidy pull request that would lint these callsites: https://github.com/llvm/llvm-project/pull/162741
    
    A script was used to modify these sites, however it cannot run as a
    scripted-diff because it uses clang-query and a build folder.
    
    The script only queries for integer and enumeration types, as other data
    members would have to be marked `constexpr` or `inline` from what I
    understand: https://en.cppreference.com/cpp/language/static#Constant_static_members
    
    Removing the ZMQ forward declaration was a clang-tidy lint.
    
    The script used to find these sites, LLM assisted:
    ```
    set -uxo pipefail
    
    cd "$(git rev-parse --show-toplevel)"
    
    BUILD=${BUILD:-build}
    if [ ! -f "${BUILD}/compile_commands.json" ]; then
        echo "error: ${BUILD}/compile_commands.json not found. Run cmake -B ${BUILD} first." >&2
        exit 1
    fi
    if ! command -v clang-query >/dev/null; then
        echo "error: clang-query not on PATH. Install clang-tools." >&2
        exit 1
    fi
    if ! git diff --quiet || ! git diff --cached --quiet; then
        echo "error: working tree has uncommitted changes. Commit or stash first." >&2
        exit 1
    fi
    
    MATCHER='match varDecl(hasParent(cxxRecordDecl()),
                          hasType(qualType(isConstQualified(),
                                           anyOf(hasCanonicalType(isInteger()),
                                                 hasDeclaration(enumDecl())))),
                          hasInitializer(expr()),
                          unless(isConstexpr()),
                          isExpansionInFileMatching("/src/"))'
    
    RAW=$(mktemp)
    trap 'rm -f "$RAW"' EXIT
    
    echo "Sweeping TUs (batched, may take a few minutes)..." >&2
    find src -type d \( -name secp256k1 -o -name leveldb -o -name crc32c \
                        -o -name minisketch -o -name libmultiprocess -o -name ctaes \) -prune -o \
            -name '*.cpp' -print0 \
        | xargs -0 -n 50 clang-query -p "${BUILD}" \
              -c 'set output diag' \
              -c "${MATCHER}" \
              >>"$RAW" || true
    
    ROOT=$(pwd)
    LOCS=$(grep -oE "${ROOT}/src/[^:]+:[0-9]+:[0-9]+:" "$RAW" \
        | sed -E "s|^${ROOT}/||; s|:[0-9]+:$||" \
        | sort -u)
    
    if [ -z "$LOCS" ]; then
        echo "no matches" >&2
        exit 0
    fi
    
    FILTERED=""
    while IFS=: read -r file line; do
        case "$file" in
            src/secp256k1/*|src/leveldb/*|src/crc32c/*|src/minisketch/*|src/ipc/libmultiprocess/*|src/crypto/ctaes/*) continue ;;
            src/tinyformat.h) continue ;;
        esac
        src=$(sed -n "${line}p" "$file")
        case "$src" in *inline*) continue ;; esac
        FILTERED+="${file}:${line}"$'\n'
    done <<<"$LOCS"
    FILTERED=$(printf '%s' "$FILTERED" | sed '/^$/d')
    
    if [ -z "$FILTERED" ]; then
        echo "no matches after filtering" >&2
        exit 0
    fi
    
    echo "Sites to rewrite ($(echo "$FILTERED" | wc -l)):" >&2
    echo "$FILTERED" >&2
    
    declare -A LINES
    while IFS=: read -r file line; do
        LINES[$file]+="${line} "
    done <<<"$FILTERED"
    
    for file in "${!LINES[@]}"; do
        args=()
        for line in ${LINES[$file]}; do
            args+=(-e "${line}s/static const /static constexpr /")
        done
        sed -i "${args[@]}" "$file"
    done
    
    echo >&2
    echo "===== proposed diff =====" >&2
    git --no-pager diff
    ```
    05c35c402c
  12. rustaceanrob force-pushed on Jul 24, 2026
  13. fanquake commented at 1:54 PM on July 24, 2026: member

    ACK 05c35c402cc56e7082a48cc073434a40b0bebd8f

  14. DrahtBot requested review from hebasto on Jul 24, 2026
  15. DrahtBot removed the label CI failed on Jul 24, 2026
  16. sedited approved
  17. sedited commented at 4:29 PM on July 24, 2026: contributor

    ACK 05c35c402cc56e7082a48cc073434a40b0bebd8f

  18. fanquake merged this on Jul 24, 2026
  19. fanquake closed this on Jul 24, 2026

  20. rustaceanrob deleted the branch on Jul 24, 2026
  21. l0rinc commented at 6:46 PM on July 24, 2026: contributor

    Post-merge ACK. I was afraid this would cause a lot of needless rebases in other PRs, but it seems the values were selected carefully to minimize that.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-31 20:50 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me