From fa2670bd4b5b199c417011942228ba87d1613030 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 26 Feb 2026 21:13:21 +0100 Subject: [PATCH 1/2] refactor: Enable -Wswitch in exhaustive switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, apply the comment according to the dev notes. Also, modify the dev notes to give a lambda-wrapped example. Can be reviewed via --ignore-all-space Co-authored-by: Lőrinc --- doc/developer-notes.md | 18 ++++---- src/bench/sign_transaction.cpp | 15 +++--- src/bench/verify_script.cpp | 13 +++--- src/common/args.cpp | 4 +- src/common/messages.cpp | 11 ++--- src/common/signmessage.cpp | 3 +- src/psbt.cpp | 3 +- src/qt/notificator.cpp | 3 +- src/qt/sendcoinsdialog.cpp | 5 +- src/qt/signverifymessagedialog.cpp | 3 +- src/qt/transactiontablemodel.cpp | 5 +- src/rpc/util.cpp | 44 +++++++++--------- src/script/script.cpp | 5 +- src/script/script_error.cpp | 4 +- src/test/fuzz/versionbits.cpp | 73 +++++++++++++++--------------- src/wallet/coinselection.cpp | 3 +- src/wallet/rpc/spend.cpp | 20 ++++---- src/wallet/rpc/transactions.cpp | 3 +- src/wallet/wallet.h | 2 +- 19 files changed, 112 insertions(+), 125 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index d8ce01c8118..496fa8ef0bf 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -854,19 +854,19 @@ Foo(vec); enum class Tabs { info, console, - network_graph, - peers }; int GetInt(Tabs tab) { - switch (tab) { - case Tabs::info: return 0; - case Tabs::console: return 1; - case Tabs::network_graph: return 2; - case Tabs::peers: return 3; - } // no default case, so the compiler can warn about missing cases - assert(false); + int ret = [&]() { + switch (tab) { + case Tabs::info: return 0; + case Tabs::console: return 1; + } // no default case, so the compiler can warn about missing cases + assert(false); + }(); + LogInfo("Tab %s", ret); + return ret; } ``` diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp index 96af48c5724..5405265b230 100644 --- a/src/bench/sign_transaction.cpp +++ b/src/bench/sign_transaction.cpp @@ -42,12 +42,15 @@ static void SignTransactionSingleInput(benchmark::Bench& bench, InputType input_ keystore.pubkeys.emplace(key_id, pubkey); // Create specified locking script type - CScript prev_spk; - switch (input_type) { - case InputType::P2WPKH: prev_spk = GetScriptForDestination(WitnessV0KeyHash(pubkey)); break; - case InputType::P2TR: prev_spk = GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey{pubkey})); break; - default: assert(false); - } + CScript prev_spk = [&]() { + switch (input_type) { + case InputType::P2WPKH: + return GetScriptForDestination(WitnessV0KeyHash(pubkey)); + case InputType::P2TR: + return GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey{pubkey})); + } // no default case, so the compiler can warn about missing cases + assert(false); + }(); prev_spks.push_back(prev_spk); } diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp index 1ef6a3613f4..b0ef4988ce1 100644 --- a/src/bench/verify_script.cpp +++ b/src/bench/verify_script.cpp @@ -41,12 +41,13 @@ static void VerifyScriptBench(benchmark::Bench& bench, ScriptType script_type) keystore.pubkeys.emplace(key_id, pubkey); // Create crediting and spending transactions with provided input type - CTxDestination dest; - switch (script_type) { - case ScriptType::P2WPKH: dest = WitnessV0KeyHash(pubkey); break; - case ScriptType::P2TR: dest = WitnessV1Taproot(XOnlyPubKey{pubkey}); break; - default: assert(false); - } + const auto dest{[&]() -> CTxDestination { + switch (script_type) { + case ScriptType::P2WPKH: return WitnessV0KeyHash(pubkey); + case ScriptType::P2TR: return WitnessV1Taproot(XOnlyPubKey{pubkey}); + } // no default case, so the compiler can warn about missing cases + assert(false); + }()}; const CMutableTransaction& txCredit = BuildCreditingTransaction(GetScriptForDestination(dest), 1); CMutableTransaction txSpend = BuildSpendingTransaction(/*scriptSig=*/{}, /*scriptWitness=*/{}, CTransaction(txCredit)); diff --git a/src/common/args.cpp b/src/common/args.cpp index 5c8589cf440..6d723030717 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -692,9 +692,9 @@ std::string ArgsManager::GetHelpMessage() const case OptionsCategory::CLI_COMMANDS: usage += HelpMessageGroup("CLI Commands:"); break; - default: + case OptionsCategory::HIDDEN: break; - } + } // no default case, so the compiler can warn about missing cases // When we get to the hidden options, stop if (arg_map.first == OptionsCategory::HIDDEN) break; diff --git a/src/common/messages.cpp b/src/common/messages.cpp index 637ec62af89..12e32cae806 100644 --- a/src/common/messages.cpp +++ b/src/common/messages.cpp @@ -66,9 +66,8 @@ std::string FeeModeInfo(const std::pair& mode, std return strprintf("%s estimates use a longer time horizon, making them\n" "less responsive to short-term drops in the prevailing fee market. This mode\n" "potentially returns a higher fee rate estimate.\n", mode.first); - default: - assert(false); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } std::string FeeModesDetail(std::string default_info) @@ -119,8 +118,7 @@ bilingual_str PSBTErrorString(PSBTError err) return Untranslated("Input needs additional signatures or other data"); case PSBTError::OK: return Untranslated("No errors"); - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases assert(false); } @@ -143,8 +141,7 @@ bilingual_str TransactionErrorString(const TransactionError err) return Untranslated("Unspendable output exceeds maximum configured by user (maxburnamount)"); case TransactionError::INVALID_PACKAGE: return Untranslated("Transaction rejected due to invalid package"); - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/common/signmessage.cpp b/src/common/signmessage.cpp index a86310e1d49..0f9e1f5e30d 100644 --- a/src/common/signmessage.cpp +++ b/src/common/signmessage.cpp @@ -87,7 +87,6 @@ std::string SigningResultString(const SigningResult res) return "Private key not available"; case SigningResult::SIGNING_FAILED: return "Sign failed"; - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/psbt.cpp b/src/psbt.cpp index 4ee5f5bdd30..58979152e65 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -600,8 +600,7 @@ std::string PSBTRoleName(PSBTRole role) { case PSBTRole::SIGNER: return "signer"; case PSBTRole::FINALIZER: return "finalizer"; case PSBTRole::EXTRACTOR: return "extractor"; - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/qt/notificator.cpp b/src/qt/notificator.cpp index 397d3ad88af..215ffa52ea1 100644 --- a/src/qt/notificator.cpp +++ b/src/qt/notificator.cpp @@ -176,8 +176,7 @@ void Notificator::notifyDBus(Class cls, const QString &title, const QString &tex case Information: sicon = QStyle::SP_MessageBoxInformation; break; case Warning: sicon = QStyle::SP_MessageBoxWarning; break; case Critical: sicon = QStyle::SP_MessageBoxCritical; break; - default: break; - } + } // no default case, so the compiler can warn about missing cases tmpicon = QApplication::style()->standardIcon(sicon); } else diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index d4a63987f47..493b4fb7b3e 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -749,12 +749,9 @@ void SendCoinsDialog::processSendCoinsReturn(const WalletModel::SendCoinsReturn case WalletModel::AbsurdFee: msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->wallet().getDefaultMaxTxFee())); break; - // included to prevent a compiler warning. case WalletModel::OK: - default: return; - } - + } // no default case, so the compiler can warn about missing cases Q_EMIT message(tr("Send Coins"), msgParams.first, msgParams.second); } diff --git a/src/qt/signverifymessagedialog.cpp b/src/qt/signverifymessagedialog.cpp index e87f1b84f67..cb14192f4d4 100644 --- a/src/qt/signverifymessagedialog.cpp +++ b/src/qt/signverifymessagedialog.cpp @@ -153,8 +153,7 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked() case SigningResult::SIGNING_FAILED: error = tr("Message signing failed."); break; - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases if (res != SigningResult::OK) { ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }"); diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 6c09c4d7a89..0eff47ee927 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -477,9 +477,8 @@ QVariant TransactionTableModel::txStatusDecoration(const TransactionRecord *wtx) } case TransactionStatus::NotAccepted: return QIcon(":/icons/transaction_0"); - default: - return COLOR_BLACK; - } + } // no default case, so the compiler can warn about missing cases + assert(false); } QString TransactionTableModel::formatTooltip(const TransactionRecord *rec) const diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 49569c7be54..b4fa28243f1 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -583,29 +583,29 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector(arg.m_fallback).getType()) { - case UniValue::VOBJ: - CHECK_NONFATAL(type == RPCArg::Type::OBJ); - break; - case UniValue::VARR: - CHECK_NONFATAL(type == RPCArg::Type::ARR); - break; - case UniValue::VSTR: - CHECK_NONFATAL(type == RPCArg::Type::STR || type == RPCArg::Type::STR_HEX || type == RPCArg::Type::AMOUNT); - break; - case UniValue::VNUM: - CHECK_NONFATAL(type == RPCArg::Type::NUM || type == RPCArg::Type::AMOUNT || type == RPCArg::Type::RANGE); - break; - case UniValue::VBOOL: - CHECK_NONFATAL(type == RPCArg::Type::BOOL); - break; - case UniValue::VNULL: - // Null values are accepted in all arguments - break; - default: + [&]() { + switch (std::get(arg.m_fallback).getType()) { + case UniValue::VOBJ: + CHECK_NONFATAL(type == RPCArg::Type::OBJ); + return; + case UniValue::VARR: + CHECK_NONFATAL(type == RPCArg::Type::ARR); + return; + case UniValue::VSTR: + CHECK_NONFATAL(type == RPCArg::Type::STR || type == RPCArg::Type::STR_HEX || type == RPCArg::Type::AMOUNT); + return; + case UniValue::VNUM: + CHECK_NONFATAL(type == RPCArg::Type::NUM || type == RPCArg::Type::AMOUNT || type == RPCArg::Type::RANGE); + return; + case UniValue::VBOOL: + CHECK_NONFATAL(type == RPCArg::Type::BOOL); + return; + case UniValue::VNULL: + // Null values are accepted in all arguments + return; + } // no default case, so the compiler can warn about missing cases NONFATAL_UNREACHABLE(); - break; - } + }(); } } } diff --git a/src/script/script.cpp b/src/script/script.cpp index 829e351be69..3f764aaf214 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -151,9 +151,8 @@ std::string GetOpName(opcodetype opcode) case OP_INVALIDOPCODE : return "OP_INVALIDOPCODE"; - default: - return "OP_UNKNOWN"; - } + } // no default case, so the compiler can warn about missing cases + return "OP_UNKNOWN"; } unsigned int CScript::GetSigOpCount(bool fAccurate) const diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp index b0e7a6e10ec..7fa127420e6 100644 --- a/src/script/script_error.cpp +++ b/src/script/script_error.cpp @@ -121,7 +121,7 @@ std::string ScriptErrorString(const ScriptError serror) return "Script number overflowed or is non-minimally encoded"; case SCRIPT_ERR_UNKNOWN_ERROR: case SCRIPT_ERR_ERROR_COUNT: - default: break; - } + break; + } // no default case, so the compiler can warn about missing cases return "unknown error"; } diff --git a/src/test/fuzz/versionbits.cpp b/src/test/fuzz/versionbits.cpp index a2085e6a409..b68ef58a156 100644 --- a/src/test/fuzz/versionbits.cpp +++ b/src/test/fuzz/versionbits.cpp @@ -279,44 +279,45 @@ FUZZ_TARGET(versionbits, .init = initialize) } // state is where everything interesting is - switch (state) { - case ThresholdState::DEFINED: - assert(since == 0); - assert(exp_state == ThresholdState::DEFINED); - assert(current_block->GetMedianTimePast() < dep.nStartTime); - break; - case ThresholdState::STARTED: - assert(current_block->GetMedianTimePast() >= dep.nStartTime); - if (exp_state == ThresholdState::STARTED) { - assert(blocks_sig < dep.threshold); - assert(current_block->GetMedianTimePast() < dep.nTimeout); - } else { + [&]() { + switch (state) { + case ThresholdState::DEFINED: + assert(since == 0); assert(exp_state == ThresholdState::DEFINED); - } - break; - case ThresholdState::LOCKED_IN: - if (exp_state == ThresholdState::LOCKED_IN) { - assert(current_block->nHeight + 1 < dep.min_activation_height); - } else { - assert(exp_state == ThresholdState::STARTED); - assert(blocks_sig >= dep.threshold); - } - break; - case ThresholdState::ACTIVE: - assert(always_active_test || dep.min_activation_height <= current_block->nHeight + 1); - assert(exp_state == ThresholdState::ACTIVE || exp_state == ThresholdState::LOCKED_IN); - break; - case ThresholdState::FAILED: - assert(never_active_test || current_block->GetMedianTimePast() >= dep.nTimeout); - if (exp_state == ThresholdState::STARTED) { - assert(blocks_sig < dep.threshold); - } else { - assert(exp_state == ThresholdState::FAILED); - } - break; - default: + assert(current_block->GetMedianTimePast() < dep.nStartTime); + return; + case ThresholdState::STARTED: + assert(current_block->GetMedianTimePast() >= dep.nStartTime); + if (exp_state == ThresholdState::STARTED) { + assert(blocks_sig < dep.threshold); + assert(current_block->GetMedianTimePast() < dep.nTimeout); + } else { + assert(exp_state == ThresholdState::DEFINED); + } + return; + case ThresholdState::LOCKED_IN: + if (exp_state == ThresholdState::LOCKED_IN) { + assert(current_block->nHeight + 1 < dep.min_activation_height); + } else { + assert(exp_state == ThresholdState::STARTED); + assert(blocks_sig >= dep.threshold); + } + return; + case ThresholdState::ACTIVE: + assert(always_active_test || dep.min_activation_height <= current_block->nHeight + 1); + assert(exp_state == ThresholdState::ACTIVE || exp_state == ThresholdState::LOCKED_IN); + return; + case ThresholdState::FAILED: + assert(never_active_test || current_block->GetMedianTimePast() >= dep.nTimeout); + if (exp_state == ThresholdState::STARTED) { + assert(blocks_sig < dep.threshold); + } else { + assert(exp_state == ThresholdState::FAILED); + } + return; + } // no default case, so the compiler can warn about missing cases assert(false); - } + }(); if (blocks.size() >= period * max_periods) { // we chose the timeout (and block times) so that by the time we have this many blocks it's all over diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index d7fde38991c..d6ea6851e64 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -967,8 +967,7 @@ std::string GetAlgorithmName(const SelectionAlgorithm algo) case SelectionAlgorithm::SRD: return "srd"; case SelectionAlgorithm::CG: return "cg"; case SelectionAlgorithm::MANUAL: return "manual"; - // No default case to allow for compiler to warn - } + } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 2991a27594e..f548dae5efc 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1098,28 +1098,24 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CAmount old_fee; CAmount new_fee; CMutableTransaction mtx; - feebumper::Result res; // Targeting feerate bump. - res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs, original_change_index); - if (res != feebumper::Result::OK) { - switch(res) { + [&](){ + switch (feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs, original_change_index)) { + case feebumper::Result::OK: + return; case feebumper::Result::INVALID_ADDRESS_OR_KEY: throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, errors[0].original); - break; case feebumper::Result::INVALID_REQUEST: throw JSONRPCError(RPC_INVALID_REQUEST, errors[0].original); - break; case feebumper::Result::INVALID_PARAMETER: throw JSONRPCError(RPC_INVALID_PARAMETER, errors[0].original); - break; case feebumper::Result::WALLET_ERROR: throw JSONRPCError(RPC_WALLET_ERROR, errors[0].original); - break; - default: + case feebumper::Result::MISC_ERROR: throw JSONRPCError(RPC_MISC_ERROR, errors[0].original); - break; - } - } + } // no default case, so the compiler can warn about missing cases + NONFATAL_UNREACHABLE(); + }(); UniValue result(UniValue::VOBJ); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 34ebdea79c7..20cd64674d4 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -905,8 +905,7 @@ RPCHelpMan rescanblockchain() throw JSONRPCError(RPC_MISC_ERROR, "Rescan failed. Potentially corrupted data files."); case CWallet::ScanResult::USER_ABORT: throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted."); - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases UniValue response(UniValue::VOBJ); response.pushKV("start_height", start_height); response.pushKV("stop_height", result.last_scanned_height ? *result.last_scanned_height : UniValue()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fd4b368d781..81d40ec8938 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -283,7 +283,7 @@ inline std::string PurposeToString(AddressPurpose p) case AddressPurpose::RECEIVE: return "receive"; case AddressPurpose::SEND: return "send"; case AddressPurpose::REFUND: return "refund"; - } // no default case so the compiler will warn when a new enum as added + } // no default case, so the compiler can warn about missing cases assert(false); } From fa4ec13b44d1d6dc7bfe942e5904548456b49210 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Sat, 28 Feb 2026 15:51:38 +0100 Subject: [PATCH 2/2] build: Enable -Wcovered-switch-default The leveldb exclusion is required to avoid this warning in the subtree: ``` src/leveldb/util/status.cc:63:7: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default] 63 | default: | ^ 1 warning generated. ``` --- CMakeLists.txt | 1 + cmake/leveldb.cmake | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 518e02776d4..e7cb8965665 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -449,6 +449,7 @@ else() try_append_cxx_flags("-Wall" TARGET warn_interface SKIP_LINK) try_append_cxx_flags("-Wextra" TARGET warn_interface SKIP_LINK) try_append_cxx_flags("-Wgnu" TARGET warn_interface SKIP_LINK) + try_append_cxx_flags("-Wcovered-switch-default" TARGET warn_interface SKIP_LINK) # Some compilers will ignore -Wformat-security without -Wformat, so just combine the two here. try_append_cxx_flags("-Wformat -Wformat-security" TARGET warn_interface SKIP_LINK) try_append_cxx_flags("-Wvla" TARGET warn_interface SKIP_LINK) diff --git a/cmake/leveldb.cmake b/cmake/leveldb.cmake index e21190420ec..cff8c61f96c 100644 --- a/cmake/leveldb.cmake +++ b/cmake/leveldb.cmake @@ -87,6 +87,9 @@ else() try_append_cxx_flags("-Wconditional-uninitialized" TARGET nowarn_leveldb_interface SKIP_LINK IF_CHECK_PASSED "-Wno-conditional-uninitialized" ) + try_append_cxx_flags("-Wcovered-switch-default" TARGET nowarn_leveldb_interface SKIP_LINK + IF_CHECK_PASSED "-Wno-covered-switch-default" + ) endif() target_link_libraries(leveldb PRIVATE