build: Enable -Wshadow #29485
Closed
build: Enable -Wshadow #29485
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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);
^
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);
^
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;
^
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++) {
^
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
^
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;
^
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()) {
^
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;
^
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
^
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);
^
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) {
^
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>()) {
^
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;
^
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)
^
void CDBBatch::WriteImpl(Span<const std::byte> key, DataStream& ssValue)
^
./dbwrapper.h:83:16: note: previous declaration is here
DataStream ssValue{};
^
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
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)
^
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)
^
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()) {
^
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)
^
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;
^
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;
^
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>()) {
^
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;
^
./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;
^
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;
^
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];
^
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";
^
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);
^
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;
^
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,
^
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;
^
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) {
^
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)
^
27 tasks
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:
|
This was referenced Feb 27, 2024
|
I think the commits removing what should be dead code should be split out, but I'm not quite convinced otherwise. |
This was referenced Feb 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an attempt to demonstrate what is necessary to get
-Wshadowbuilding under-Werroron 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:
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.
Related: