refactor: Remove redundant definitions #29492

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

   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: member

    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

Contributors
Empact DrahtBot maflcko

Labels
Refactoring