refactor: Enable -Wswitch for FeeEstimateHorizon #20771

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2012-WswitchFeeEstimateHorizon changing 1 files +14 −21

MarcoFalke commented at 11:43 AM on December 26, 2020: member

This enables the -Wswitch compiler warning for FeeEstimateHorizon by removing the default case in switch statements.
fanquake added the label Refactoring on Dec 26, 2020
MarcoFalke force-pushed on Dec 26, 2020
MarcoFalke force-pushed on Dec 26, 2020
jonatack commented at 12:33 PM on December 26, 2020: member

Concept ACK. I was making a similar refactoring here when adding the estimatefeerate rpc.
mjdietzx commented at 3:07 PM on December 26, 2020: contributor

concept ack fa88a3aeb42192133336233774ee12a1a22dd151
refactor: Enable -Wswitch for FeeEstimateHorizon faccf8b1e1
MarcoFalke force-pushed on Dec 26, 2020

in src/policy/fees.cpp:661 in faccf8b1e1

 656 | @@ -660,10 +657,8 @@ CFeeRate CBlockPolicyEstimator::estimateRawFee(int confTarget, double successThr
 657 |          stats = longStats.get();
 658 |          break;
 659 |      }
 660 | -    default: {
 661 | -        throw std::out_of_range("CBlockPolicyEstimator::estimateRawFee unknown FeeEstimateHorizon");
 662 | -    }
 663 | -    }
 664 | +    } // no default case, so the compiler can warn about missing cases
 665 | +    assert(stats);

jonatack commented at 9:30 PM on December 26, 2020:

nit, assert(stats) seems to be checking for more than missing cases (like assert(false)), maybe update the comment

hebasto commented at 9:45 PM on December 26, 2020:

I think the comment and assert(stats); are independent here, as std::unique_ptr::get could return nullptr.

jonatack commented at 11:22 PM on December 27, 2020:

You're right, nvm

jonatack commented at 9:30 PM on December 26, 2020: member

ACK faccf8b1e1af293dfe9158d732718e7798a2fd89
hebasto approved
hebasto commented at 9:43 PM on December 26, 2020: member

ACK faccf8b1e1af293dfe9158d732718e7798a2fd89, I have reviewed the code and it looks OK, I agree it can be merged.
hebasto commented at 9:47 PM on December 26, 2020: member

I was making a similar refactoring here when adding the estimatefeerate rpc.

Me too: https://github.com/bitcoin-core/gui/pull/166 :)
practicalswift commented at 10:55 PM on December 27, 2020: contributor

cr ACK faccf8b1e1af293dfe9158d732718e7798a2fd89
fanquake merged this on Dec 28, 2020
fanquake closed this on Dec 28, 2020
MarcoFalke deleted the branch on Dec 28, 2020
sidhujag referenced this in commit 878d0220c8 on Dec 28, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels