refactor: Remove redundant definitions #29492

1. 
   Empact commented at 5:08 pm on February 27, 2024: contributor

   These were identified via -Wshadow in #29485, see the commit messages for the associated compiler message.

2. 
   DrahtBot commented at 5:08 pm on February 27, 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:

   • #30110 (refactor: TxDownloadManager + fuzzing by glozow)

   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.

3. DrahtBot added the label Refactoring on Feb 27, 2024
4. 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;
                       ^

   3894374ce8
5. 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;
                       ^

   ad15919548
6. 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;
                                                  ^

   f22114bdf4
7. 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";
                   ^

   da188bf523
8. in src/common/init.cpp:73 in b1fe46a4cd outdated

   69@@ -70,7 +70,7 @@ std::optional<ConfigError> InitConfig(ArgsManager& args, SettingsAbortFn setting
   70             const std::string config_source = cli_config_path.empty()
   71                 ? strprintf("data directory %s", fs::quoted(fs::PathToString(orig_datadir_path)))
   72                 : strprintf("command line argument %s", fs::quoted("-conf=" + cli_config_path));
   73-            const std::string error = strprintf(
   74+            error = strprintf(
   

   ---

   ---

   

   maflcko commented at 5:28 pm on February 27, 2024:

   not sure. Shadowing is a side effect of the effort to limit the scope of a symbol to a smaller scope. I don’t think re-using a symbol of the outer scope is the correct solution, merely because it happens to have the same name and type.
9. Empact force-pushed on Feb 27, 2024
10. 
    Empact commented at 11:11 pm on February 27, 2024: contributor

    My inclination would be to fix the -Wshadow warnings over the course of several PRs (to ease review/minimize conflicts), then add the warning to one of the CI runs to guard against introduction, while leaving it disabled by default to avoid other incidental disruption.
11. DrahtBot added the label CI failed on Apr 19, 2024
12. DrahtBot removed the label CI failed on Apr 23, 2024
13. 
    achow101 commented at 2:37 pm on October 15, 2024: member

    Are you still working on this?
14. in src/net_processing.cpp:3040 in da188bf523

    3036@@ -3037,8 +3037,6 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
    3037     AssertLockHeld(g_msgproc_mutex);
    3038     LOCK(cs_main);
    3039 
    3040-    CTransactionRef porphanTx = nullptr;
    

    ---

    ---

    

    maflcko commented at 2:39 pm on October 15, 2024:

    I wonder why clang-tidy can’t pick this one up. Ref: https://github.com/bitcoin/bitcoin/pull/31051/files
15. DrahtBot added the label Needs rebase on Oct 29, 2024
16. 
    DrahtBot commented at 8:06 pm on October 29, 2024: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

Contributors
Empact DrahtBot maflcko achow101

Labels
Refactoring Needs rebase