build: Enable -Wshadow

Empact commented at 9:47 pm on February 26, 2024: member

This is an attempt to demonstrate what is necessary to get -Wshadow building under -Werror on clang, and to see what other existing pull requests that might conflict with.

This was prompted by an example of shadowing I spotted in review. We currently warn against shadowing in the docs but do not detect it, so instead rely on review to identify these issues.

Note:

This was previously enabled in 2016 and reverted six months later.
I proposed a version in 2019 using a different approach in a different context.

The question is: have the circumstances changed in the past 9 years? Is a partial fix beneficial?

One option would be to merge the low-risk changes, re. tests, etc. and exclude sensitive files from this warning to get it applied to regular builds.

#27087

refactor: Avoid shadowing init_* in LoadChainstate

node/chainstate.cpp:234:15: error: declaration shadows a structured binding [-Werror,-Wshadow]
        auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options);
              ^
node/chainstate.cpp:198:11: note: previous declaration is here
    auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options);
          ^
node/chainstate.cpp:234:28: error: declaration shadows a structured binding [-Werror,-Wshadow]
        auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options);
                           ^
node/chainstate.cpp:198:24: note: previous declaration is here
    auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options);
                       ^

bd7e08ad2d

refactor: Avoid shadowing proprietary in rpc/rawtransaction.cpp

rpc/rawtransaction.cpp:1316:22: error: declaration shadows a local variable [-Werror,-Wshadow]
            UniValue proprietary(UniValue::VARR);
                     ^
rpc/rawtransaction.cpp:1095:14: note: previous declaration is here
    UniValue proprietary(UniValue::VARR);
             ^
rpc/rawtransaction.cpp:1411:22: error: declaration shadows a local variable [-Werror,-Wshadow]
            UniValue proprietary(UniValue::VARR);
                     ^
rpc/rawtransaction.cpp:1095:14: note: previous declaration is here
    UniValue proprietary(UniValue::VARR);
             ^

b1a91344b7

refactor: Don't redundantly define error in common/init.cpp

common/init.cpp:73:31: error: declaration shadows a local variable [-Werror,-Wshadow]
            const std::string error = strprintf(
                              ^
common/init.cpp:37:21: note: previous declaration is here
        std::string error;
                    ^

32d29d9f81

refactor: Avoid shadowing it in wallet/rpc/backup.cpp

wallet/rpc/backup.cpp:792:24: error: declaration shadows a local variable [-Werror,-Wshadow]
            const auto it{spk_man.mapKeyMetadata.find(keyid)};
                       ^
wallet/rpc/backup.cpp:784:67: note: previous declaration is here
    for (std::vector<std::pair<int64_t, CKeyID> >::const_iterator it = vKeyBirth.begin(); it != vKeyBirth.end(); it++) {
                                                                  ^

f672f37365

refactor: Avoid shadowing val in UniValue

univalue/lib/univalue.cpp:104:35: error: declaration shadows a field of 'UniValue' [-Werror,-Wshadow]
void UniValue::push_back(UniValue val)
                                  ^
./univalue/include/univalue.h:103:17: note: previous declaration is here
    std::string val;                       // numbers are stored as C++ strings
                ^
univalue/lib/univalue.cpp:118:52: error: declaration shadows a field of 'UniValue' [-Werror,-Wshadow]
void UniValue::pushKVEnd(std::string key, UniValue val)
                                                   ^
./univalue/include/univalue.h:103:17: note: previous declaration is here
    std::string val;                       // numbers are stored as C++ strings
                ^
univalue/lib/univalue.cpp:126:49: error: declaration shadows a field of 'UniValue' [-Werror,-Wshadow]
void UniValue::pushKV(std::string key, UniValue val)
                                                ^
./univalue/include/univalue.h:103:17: note: previous declaration is here
    std::string val;                       // numbers are stored as C++ strings
                ^

dd18e4c271

refactor: Avoid shadowing error in bitcoind ParseArgs

bitcoind.cpp:122:14: error: declaration shadows a local variable [-Werror,-Wshadow]
    if (auto error = common::InitConfig(args)) {
             ^
bitcoind.cpp:117:17: note: previous declaration is here
    std::string error;
                ^

fae61ad7f8

refactor: Avoid shadowing signer in external_signer.cpp

external_signer.cpp:49:36: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (const ExternalSigner& signer : signers) {
                                   ^
external_signer.cpp:32:26: note: previous declaration is here
    for (const UniValue& signer : result.getValues()) {
                         ^

d749cc1e64

refactor: Avoid shadowing cdeq in test/fuzz/bitdeque.cpp

test/fuzz/bitdeque.cpp:353:33: error: declaration shadows a local variable [-Werror,-Wshadow]
                    const auto& cdeq = deq;
                                ^
test/fuzz/bitdeque.cpp:352:22: note: variable 'cdeq' is captured here
                if (!cdeq.empty()) {
                     ^
test/fuzz/bitdeque.cpp:45:17: note: previous declaration is here
    const auto& cdeq = deq;
                ^

348d156195

refactor: Avoid shadowing hdr in V1Transport::SetMessageToSend

net.cpp:817:20: error: declaration shadows a field of 'V1Transport' [-Werror,-Wshadow]
    CMessageHeader hdr(m_magic_bytes, msg.m_type.c_str(), msg.data.size());
                   ^
./net.h:381:20: note: previous declaration is here
    CMessageHeader hdr GUARDED_BY(m_recv_mutex); // complete header
                   ^

1cee14e84f

refactor: Avoid shadowing unknowns in rawtransaction.cpp

rpc/rawtransaction.cpp:1330:22: error: declaration shadows a local variable [-Werror,-Wshadow]
            UniValue unknowns(UniValue::VOBJ);
                     ^
rpc/rawtransaction.cpp:1107:14: note: previous declaration is here
    UniValue unknowns(UniValue::VOBJ);
             ^
rpc/rawtransaction.cpp:1425:22: error: declaration shadows a local variable [-Werror,-Wshadow]
            UniValue unknowns(UniValue::VOBJ);
                     ^
rpc/rawtransaction.cpp:1107:14: note: previous declaration is here
    UniValue unknowns(UniValue::VOBJ);
             ^

44f4a0551a

refactor: Avoid shadowing i in wallet/rpc/wallet.cpp

wallet/rpc/wallet.cpp:737:21: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (size_t i = 0; i < mtx.vout.size(); ++i) {
                    ^
wallet/rpc/wallet.cpp:699:17: note: previous declaration is here
    for (size_t i = 0; i < txs.size(); ++i) {
                ^

a0ab4abb46

refactor: Avoid shadowing conf in wallet/transaction.cpp

wallet/transaction.cpp:48:22: error: declaration shadows a local variable [-Werror,-Wshadow]
    } else if (auto* conf = state<TxStateConflicted>()) {
                     ^
wallet/transaction.cpp:46:15: note: previous declaration is here
    if (auto* conf = state<TxStateConfirmed>()) {
              ^

83a296aaae

refactor: Avoid shadowing result in wallet/spend.cpp

wallet/spend.cpp:1313:14: error: declaration shadows a local variable [-Werror,-Wshadow]
        auto result = wallet.chain().checkChainLimits(tx);
             ^
wallet/spend.cpp:1138:28: note: previous declaration is here
    const SelectionResult& result = *select_coins_res;
                           ^

3ac0a820ce

refactor: Avoid shadowing timeout in netbase.cpp

netbase.cpp:294:28: error: declaration shadows a local variable [-Werror,-Wshadow]
                const auto timeout = std::min(remaining, std::chrono::milliseconds{MAX_WAIT_FOR_IO});
                           ^
netbase.cpp:277:93: note: previous declaration is here
static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, std::chrono::milliseconds timeout, const Sock& sock)
                                                                                            ^

326aee5880

refactor: Avoid shadowing ssValue in CDBBatch::WriteImpl

void CDBBatch::WriteImpl(Span<const std::byte> key, DataStream& ssValue)
                                                                ^
./dbwrapper.h:83:16: note: previous declaration is here
    DataStream ssValue{};
               ^

ba0ebd3a23

Empact marked this as ready for review on Feb 26, 2024

DrahtBot commented at 9:51 pm on February 26, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29432 (Stratum v2 Template Provider (take 3) by Sjors)
#29015 (kernel: Streamline util library by ryanofsky)
#28678 (miniscript: convert non-critical asserts to Assumes by sipa)
#28031 (Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling by glozow)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#27307 (wallet: track mempool conflicts with wallet transactions by ishaanam)
#26022 (Add util::ResultPtr class by ryanofsky)
#25972 (build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set by fanquake)
#25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
#25665 (refactor: Add util::Result failure values, multiple error and warning messages 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.

DrahtBot added the label Build system on Feb 26, 2024

Empact marked this as a draft on Feb 26, 2024

DrahtBot commented at 11:53 pm on February 26, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/22004282753

DrahtBot added the label CI failed on Feb 26, 2024

refactor: Avoid shadowing name in RPCHelpMan

rpc/util.cpp:536:33: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (const std::string& name : names) {
                                ^
rpc/util.cpp:519:36: note: previous declaration is here
RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples, RPCMethodImpl fun)
                                   ^

7b74af6a69

refactor: Avoid shadowing script in descriptor.cpp

script/descriptor.cpp:1933:42: error: declaration shadows a local variable [-Werror,-Wshadow]
                for (const auto& [depth, script, leaf_ver] : *tree) {
                                         ^
script/descriptor.cpp:1847:60: note: previous declaration is here
std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider)
                                                           ^

758fec1d46

refactor: Avoid shadowing it in validation.cpp

validation.cpp:1616:31: error: declaration shadows a local variable [-Werror,-Wshadow]
        } else if (const auto it{individual_results_nonfinal.find(wtxid)}; it != individual_results_nonfinal.end()) {
                              ^
validation.cpp:1602:31: note: previous declaration is here
        } else if (const auto it{results_final.find(wtxid)}; it != results_final.end()) {
                              ^

2b12b7e42f

refactor: Avoid shadowing peer in net_processing.cpp

net_processing.cpp:2658:35: error: declaration shadows a local variable [-Werror,-Wshadow]
                for (const auto& [peer, stat] : m_headers_presync_stats) {
                                  ^
net_processing.cpp:2605:64: note: previous declaration is here
bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfrom, std::vector<CBlockHeader>& headers)
                                                               ^

d8b3bc13ba

refactor: Don't redundantly define porphanTx in net_processing.cpp

net_processing.cpp:3042:28: error: declaration shadows a local variable [-Werror,-Wshadow]
    while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id)) {
                           ^
net_processing.cpp:3040:21: note: previous declaration is here
    CTransactionRef porphanTx = nullptr;
                    ^

5a31bc3085

refactor: Don't redundantly define TxItems in wallet/wallet.cpp

wallet/wallet.cpp:901:48: error: declaration shadows a typedef in 'wallet::CWallet' [-Werror,-Wshadow]
    typedef std::multimap<int64_t, CWalletTx*> TxItems;
                                               ^
./wallet/wallet.h:483:48: note: previous declaration is here
    typedef std::multimap<int64_t, CWalletTx*> TxItems;
                                               ^

5a4534c8b0

refactor: Avoid shadowing conf in wallet/wallet.cpp

wallet/wallet.cpp:2787:22: error: declaration shadows a local variable [-Werror,-Wshadow]
    } else if (auto* conf = wtx.state<TxStateConflicted>()) {
                     ^
wallet/wallet.cpp:2785:15: note: previous declaration is here
    if (auto* conf = wtx.state<TxStateConfirmed>()) {
              ^
wallet/wallet.cpp:3377:22: error: declaration shadows a local variable [-Werror,-Wshadow]
    } else if (auto* conf = wtx.state<TxStateConflicted>()) {
                     ^
wallet/wallet.cpp:3374:15: note: previous declaration is here
    if (auto* conf = wtx.state<TxStateConfirmed>()) {
              ^

26d4ed9822

refactor: Avoid shadowing status, warnings in wallet/wallet.cpp

wallet/wallet.cpp:4441:24: error: declaration shadows a local variable [-Werror,-Wshadow]
        DatabaseStatus status;
                       ^
wallet/wallet.cpp:4305:20: note: previous declaration is here
    DatabaseStatus status;
                   ^
wallet/wallet.cpp:4442:36: error: declaration shadows a local variable [-Werror,-Wshadow]
        std::vector<bilingual_str> warnings;
                                   ^
wallet/wallet.cpp:4287:32: note: previous declaration is here
    std::vector<bilingual_str> warnings;
                               ^

30d8e1c5ca

refactor: Avoid shadowing subs in miniscript.h

./script/miniscript.h:649:64: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
            [&upfn](DummyState, const Node& node, Span<Result> subs) {
                                                               ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:663:67: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
            [&upfn](State&& state, const Node& node, Span<Result> subs) {
                                                                  ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:678:64: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
            [&upfn](DummyState, const Node& node, Span<Result> subs) {
                                                               ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:741:87: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        auto upfn = [&ctx, is_tapscript](bool verify, const Node& node, Span<CScript> subs) -> CScript {
                                                                                      ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:819:92: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        auto upfn = [&ctx, is_tapscript](bool wrapped, const Node& node, Span<std::string> subs) -> std::optional<std::string> {
                                                                                           ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:1430:58: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        auto upfn = [&ctx](const Node& node, Span<state> subs) -> state {
                                                         ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:1538:77: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        return TreeEval<const Node*>([](const Node& node, Span<const Node*> subs) -> const Node* {
                                                                            ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^
./script/miniscript.h:1551:64: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        return TreeEval<int>([&fn](const Node& node, Span<int> subs) -> bool {
                                                               ^
./script/miniscript.h:509:39: note: previous declaration is here
    mutable std::vector<NodeRef<Key>> subs;
                                      ^

997e91b8c3

refactor: Avoid shadowing ops in miniscript.h

./script/miniscript.h:1487:24: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        if (const auto ops = GetOps()) return *ops <= MAX_OPS_PER_SCRIPT;
                       ^
./script/miniscript.h:528:25: note: previous declaration is here
    const internal::Ops ops;
                        ^

41a211625f

refactor: Avoid shadowing ss in miniscript.h

./script/miniscript.h:1516:24: error: declaration shadows a field of 'Node<Key>' [-Werror,-Wshadow]
        if (const auto ss = GetStackSize()) return *ss <= MAX_STANDARD_P2WSH_STACK_ITEMS;
                       ^
./script/miniscript.h:530:31: note: previous declaration is here
    const internal::StackSize ss;
                              ^

806758c506

refactor: Don't shadow n in miniscript.h

./script/miniscript.h:2308:28: error: declaration shadows a structured binding [-Werror,-Wshadow]
                const auto n = ParseScriptNumber(in[1]);
                           ^
./script/miniscript.h:2237:28: note: previous declaration is here
        auto [cur_context, n, k] = to_parse.back();
                           ^

ec5d7a0c18

refactor: Don't shadow k in miniscript.h

./script/miniscript.h:2317:28: error: declaration shadows a structured binding [-Werror,-Wshadow]
                const auto k = ParseScriptNumber(in[2 + *n]);
                           ^
./script/miniscript.h:2237:31: note: previous declaration is here
        auto [cur_context, n, k] = to_parse.back();
                              ^
./script/miniscript.h:2328:28: error: declaration shadows a structured binding [-Werror,-Wshadow]
                const auto k = ParseScriptNumber(in[1]);
                           ^
./script/miniscript.h:2237:31: note: previous declaration is here
        auto [cur_context, n, k] = to_parse.back();
                              ^

fbb846dea7

refactor: Avoid shadowing error in qt/bitcoin.cpp

qt/bitcoin.cpp:617:14: error: declaration shadows a local variable [-Werror,-Wshadow]
    if (auto error = common::InitConfig(gArgs, ErrorSettingsRead)) {
             ^
qt/bitcoin.cpp:540:17: note: previous declaration is here
    std::string error;
                ^

d92bda0199

refactor: Avoid shadowing mode in qt/askpassphrasedialog.cpp

qt/askpassphrasedialog.cpp:248:16: error: declaration shadows a field of 'AskPassphraseDialog' [-Werror,-Wshadow]
    const auto mode = show ? QLineEdit::Normal : QLineEdit::Password;
               ^
./qt/askpassphrasedialog.h:40:10: note: previous declaration is here
    Mode mode;
         ^

5aa88f644b

Empact force-pushed on Feb 27, 2024

refactor: Avoid shadowing ERR in util/sock.cpp

util/sock.cpp:75:27: error: declaration shadows a static data member of 'Sock' [-Werror,-Wshadow]
     static constexpr auto ERR = SOCKET_ERROR;
                           ^
./util/sock.h:154:28: note: previous declaration is here
     static constexpr Event ERR = 0b100;
                            ^

80a54c20ed

test: Avoid shadowing in sha256 SelfTest

crypto/sha256.cpp:554:23: error: declaration shadows a local variable [-Werror,-Wshadow]
        unsigned char out[64];
                      ^
crypto/sha256.cpp:548:19: note: previous declaration is here
    unsigned char out[32];
                  ^
crypto/sha256.cpp:561:23: error: declaration shadows a local variable [-Werror,-Wshadow]
        unsigned char out[128];
                      ^
crypto/sha256.cpp:548:19: note: previous declaration is here
    unsigned char out[32];
                  ^
crypto/sha256.cpp:568:23: error: declaration shadows a local variable [-Werror,-Wshadow]
        unsigned char out[256];
                      ^
crypto/sha256.cpp:548:19: note: previous declaration is here
    unsigned char out[32];
                  ^

4913dd51d9

test: Don't redundantly define msg in key_tests

test/key_tests.cpp:184:21: error: declaration shadows a local variable [-Werror,-Wshadow]
        std::string msg = "A message to be signed" + ToString(i);
                    ^
test/key_tests.cpp:161:17: note: previous declaration is here
    std::string msg = "A message to be signed";
                ^

37f14893af

test: Avoid shadowing ok in key_tests.cpp

test/key_tests.cpp:340:18: error: declaration shadows a local variable [-Werror,-Wshadow]
            bool ok = key.SignSchnorr(msg256, sig64, &merkle_root, aux256);
                 ^
test/key_tests.cpp:324:14: note: previous declaration is here
        bool ok = key.SignSchnorr(msg256, sig64, nullptr, aux256);
             ^

08221dffbf

test: Avoid shadowing entry in miniminer_tests.cpp

test/miniminer_tests.cpp:182:21: error: declaration shadows a local variable [-Werror,-Wshadow]
        const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))};
                    ^
test/miniminer_tests.cpp:113:28: note: previous declaration is here
    TestMemPoolEntryHelper entry;
                           ^

45a2956325

test: Avoid shadowing addr in net_tests.cpp

test/net_tests.cpp:888:30: error: declaration shadows a local variable [-Werror,-Wshadow]
            for (const auto& addr : addresses) {
                             ^
test/net_tests.cpp:878:57: note: previous declaration is here
    CaptureMessage = [&sent, &expected](const CAddress& addr,
                                                        ^

93db03948a

test: Avoid shadowing i in test/fuzz/utxo_total_supply.cpp

test/fuzz/utxo_total_supply.cpp:71:28: error: declaration shadows a local variable [-Werror,-Wshadow]
            const uint32_t i = tx.vout.size() - 2;
                           ^
test/fuzz/utxo_total_supply.cpp:66:24: note: previous declaration is here
        const uint32_t i = tx.vout.size() - 1;
                       ^

a16266833c

test: Avoid shadowing i in minisketch_tests.cpp

test/minisketch_tests.cpp:47:23: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (uint32_t i = 0; i < a_not_b; ++i) BOOST_CHECK_EQUAL(sols[i], start_a + i);
                      ^
test/minisketch_tests.cpp:21:14: note: previous declaration is here
    for (int i = 0; i < 100; ++i) {
             ^
test/minisketch_tests.cpp:48:23: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (uint32_t i = 0; i < b_not_a; ++i) BOOST_CHECK_EQUAL(sols[i + a_not_b], start_b + both + i);
                      ^
test/minisketch_tests.cpp:21:14: note: previous declaration is here
    for (int i = 0; i < 100; ++i) {
             ^

580b7c7ee3

test: Avoid shadowing variables in nested LIMITED_WHILE macros

By including the line number in the counter variable name

Here I use __LINE__ rather than UNIQUE_NAME's __COUNTER__ to generate a stable name
for the repeated references.

test/fuzz/txorphan.cpp:82:9: error: declaration shadows a local variable [-Werror,-Wshadow]
        LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)
        ^
./test/fuzz/fuzz.h:24:19: note: expanded from macro 'LIMITED_WHILE'
    for (unsigned _count{limit}; (condition) && _count; --_count)
                  ^
test/fuzz/txorphan.cpp:48:5: note: previous declaration is here
    LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)
    ^
./test/fuzz/fuzz.h:24:19: note: expanded from macro 'LIMITED_WHILE'
    for (unsigned _count{limit}; (condition) && _count; --_count)
                  ^

aa97b6a59d

build: Enable -Wshadow d0933c5229

Empact force-pushed on Feb 27, 2024

DrahtBot removed the label CI failed on Feb 27, 2024

Empact marked this as ready for review on Feb 27, 2024

maflcko commented at 7:36 am on February 27, 2024: member

The question is: have the circumstances changed in the past 9 years? Is a partial fix beneficial?

I don’t think they have. The problem is still that system-dependent stuff (system headers, compilers, compiler versions, lib versions) will mutate the result, so I am sure the frustration will come back.

Just looking at the diff:

sometimes std::string error; is allowed, other times it is not, depending on the location. In any case this is fixed by #29236?
There aren’t any bugs found by this (except maybe for the one you mentioned in a draft pull request?), so the benefit seems questionable.
There are a bunch of conflicts, and the diff isn’t trivial to review.

fanquake commented at 9:52 am on February 27, 2024: member

I think the commits removing what should be dead code should be split out, but I’m not quite convinced otherwise.

Empact commented at 4:54 pm on February 27, 2024: member

Ok sounds reasonable to me, particularly on the conflicts point, I’ll split out that subset for now.

FYI @maflcko AFAIK this is unrelated to #29236, the error in question are local string variables, rather than the log function.

Empact closed this on Feb 27, 2024

Empact deleted the branch on Feb 27, 2024

build: Enable -Wshadow #29485

Code Coverage

Reviews

Conflicts