Merge #20530: lint, refactor: Update cppcheck linter to c++17 and improve explicit usage

1e62350ca20898189904a88dfef9ea11ddcd8626 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb854ca827a5a3925394f9e09d29b898 lint: Use c++17 std in cppcheck linter (Fabian Jahr)

Pull request description:

  I found the `extended-lint-cppcheck` linter still uses `std=c++11` when reviewing #20471. The only difference in the output after this change is one line is missing:

  ```
  src/script/descriptor.cpp:159:5: warning: Struct 'PubkeyProvider' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
  ```

  After some digging, I am still not sure why this one is ignored with c++17 when 40 other`noExplicitConstructor` warnings were still appearing.

  In the second commit, I fix these warnings, adding `explicit` where appropriate and adding fixes to ignore otherwise.

ACKs for top commit:
  practicalswift:
    cr ACK 1e62350ca20898189904a88dfef9ea11ddcd8626: patch looks correct!
  MarcoFalke:
    review ACK 1e62350ca20898189904a88dfef9ea11ddcd8626

Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
This commit is contained in:
fanquake 2020-12-02 20:22:42 +08:00 committed by PastaPastaPasta
parent fa40fc676d
commit de28a0e10c
16 changed files with 27 additions and 18 deletions

View File

@ -194,7 +194,7 @@ public:
MasternodeSyncImpl m_masternodeSync; MasternodeSyncImpl m_masternodeSync;
CoinJoinOptionsImpl m_coinjoin; CoinJoinOptionsImpl m_coinjoin;
NodeImpl(NodeContext* context) { setContext(context); } explicit NodeImpl(NodeContext* context) { setContext(context); }
void initError(const bilingual_str& message) override { InitError(message); } void initError(const bilingual_str& message) override { InitError(message); }
bool parseParameters(int argc, const char* const argv[], std::string& error) override bool parseParameters(int argc, const char* const argv[], std::string& error) override
{ {

View File

@ -15,7 +15,7 @@ class Node;
class AddressBookTests : public QObject class AddressBookTests : public QObject
{ {
public: public:
AddressBookTests(interfaces::Node& node) : m_node(node) {} explicit AddressBookTests(interfaces::Node& node) : m_node(node) {}
interfaces::Node& m_node; interfaces::Node& m_node;
Q_OBJECT Q_OBJECT

View File

@ -15,7 +15,7 @@ class Node;
class RPCNestedTests : public QObject class RPCNestedTests : public QObject
{ {
public: public:
RPCNestedTests(interfaces::Node& node) : m_node(node) {} explicit RPCNestedTests(interfaces::Node& node) : m_node(node) {}
interfaces::Node& m_node; interfaces::Node& m_node;
Q_OBJECT Q_OBJECT

View File

@ -15,7 +15,7 @@ class Node;
class WalletTests : public QObject class WalletTests : public QObject
{ {
public: public:
WalletTests(interfaces::Node& node) : m_node(node) {} explicit WalletTests(interfaces::Node& node) : m_node(node) {}
interfaces::Node& m_node; interfaces::Node& m_node;
Q_OBJECT Q_OBJECT

View File

@ -38,7 +38,7 @@ public:
std::string peerAddr; std::string peerAddr;
const CoreContext& context; const CoreContext& context;
JSONRPCRequest(const CoreContext& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {} explicit JSONRPCRequest(const CoreContext& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {}
//! Initializes request information from another request object and the //! Initializes request information from another request object and the
//! given context. The implementation should be updated if any members are //! given context. The implementation should be updated if any members are

View File

@ -157,7 +157,7 @@ protected:
uint32_t m_expr_index; uint32_t m_expr_index;
public: public:
PubkeyProvider(uint32_t exp_index) : m_expr_index(exp_index) {} explicit PubkeyProvider(uint32_t exp_index) : m_expr_index(exp_index) {}
virtual ~PubkeyProvider() = default; virtual ~PubkeyProvider() = default;

View File

@ -23,7 +23,7 @@ class FuzzedSignatureChecker : public BaseSignatureChecker
FuzzedDataProvider& m_fuzzed_data_provider; FuzzedDataProvider& m_fuzzed_data_provider;
public: public:
FuzzedSignatureChecker(FuzzedDataProvider& fuzzed_data_provider) : m_fuzzed_data_provider(fuzzed_data_provider) explicit FuzzedSignatureChecker(FuzzedDataProvider& fuzzed_data_provider) : m_fuzzed_data_provider(fuzzed_data_provider)
{ {
} }

View File

@ -32,7 +32,7 @@ class DebugLogHelper
void check_found(); void check_found();
public: public:
DebugLogHelper(std::string message, MatchFn match = [](const std::string*){ return true; }); explicit DebugLogHelper(std::string message, MatchFn match = [](const std::string*){ return true; });
~DebugLogHelper() { check_found(); } ~DebugLogHelper() { check_found(); }
}; };

View File

@ -239,7 +239,7 @@ public:
std::optional<std::vector<std::string>> list_value; std::optional<std::vector<std::string>> list_value;
const char* error = nullptr; const char* error = nullptr;
Expect(util::SettingsValue s) : setting(std::move(s)) {} explicit Expect(util::SettingsValue s) : setting(std::move(s)) {}
Expect& DefaultString() { default_string = true; return *this; } Expect& DefaultString() { default_string = true; return *this; }
Expect& DefaultInt() { default_int = true; return *this; } Expect& DefaultInt() { default_int = true; return *this; }
Expect& DefaultBool() { default_bool = true; return *this; } Expect& DefaultBool() { default_bool = true; return *this; }

View File

@ -514,7 +514,7 @@ class FormatArg
{ } { }
template<typename T> template<typename T>
FormatArg(const T& value) explicit FormatArg(const T& value)
: m_value(static_cast<const void*>(&value)), : m_value(static_cast<const void*>(&value)),
m_formatImpl(&formatImpl<T>), m_formatImpl(&formatImpl<T>),
m_toIntImpl(&toIntImpl<T>) m_toIntImpl(&toIntImpl<T>)
@ -970,7 +970,7 @@ class FormatListN : public FormatList
public: public:
#ifdef TINYFORMAT_USE_VARIADIC_TEMPLATES #ifdef TINYFORMAT_USE_VARIADIC_TEMPLATES
template<typename... Args> template<typename... Args>
FormatListN(const Args&... args) explicit FormatListN(const Args&... args)
: FormatList(&m_formatterStore[0], N), : FormatList(&m_formatterStore[0], N),
m_formatterStore { FormatArg(args)... } m_formatterStore { FormatArg(args)... }
{ static_assert(sizeof...(args) == N, "Number of args must be N"); } { static_assert(sizeof...(args) == N, "Number of args must be N"); }

View File

@ -840,7 +840,7 @@ public:
class EpochGuard { class EpochGuard {
const CTxMemPool& pool; const CTxMemPool& pool;
public: public:
EpochGuard(const CTxMemPool& in); explicit EpochGuard(const CTxMemPool& in);
~EpochGuard(); ~EpochGuard();
}; };
// N.B. GetFreshEpoch modifies mutable state via the EpochGuard construction // N.B. GetFreshEpoch modifies mutable state via the EpochGuard construction

View File

@ -13,7 +13,7 @@ protected:
public: public:
BaseHash() : m_hash() {} BaseHash() : m_hash() {}
BaseHash(const HashType& in) : m_hash(in) {} explicit BaseHash(const HashType& in) : m_hash(in) {}
unsigned char* begin() unsigned char* begin()
{ {

View File

@ -566,7 +566,7 @@ private:
// All the intermediate state that gets passed between the various levels // All the intermediate state that gets passed between the various levels
// of checking a given transaction. // of checking a given transaction.
struct Workspace { struct Workspace {
Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {} explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {}
CTxMemPool::setEntries m_ancestors; CTxMemPool::setEntries m_ancestors;
std::unique_ptr<CTxMemPoolEntry> m_entry; std::unique_ptr<CTxMemPoolEntry> m_entry;

View File

@ -56,7 +56,7 @@ public:
std::unordered_map<std::string, WalletDatabaseFileId> m_fileids; std::unordered_map<std::string, WalletDatabaseFileId> m_fileids;
std::condition_variable_any m_db_in_use; std::condition_variable_any m_db_in_use;
BerkeleyEnvironment(const fs::path& env_directory); explicit BerkeleyEnvironment(const fs::path& env_directory);
BerkeleyEnvironment(); BerkeleyEnvironment();
~BerkeleyEnvironment(); ~BerkeleyEnvironment();
void Reset(); void Reset();

View File

@ -148,7 +148,7 @@ protected:
WalletStorage& m_storage; WalletStorage& m_storage;
public: public:
ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {} explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {}
virtual ~ScriptPubKeyMan() {}; virtual ~ScriptPubKeyMan() {};
virtual bool GetNewDestination(CTxDestination& dest, std::string& error) { return false; } virtual bool GetNewDestination(CTxDestination& dest, std::string& error) { return false; }
@ -486,7 +486,7 @@ class LegacySigningProvider : public SigningProvider
private: private:
const LegacyScriptPubKeyMan& m_spk_man; const LegacyScriptPubKeyMan& m_spk_man;
public: public:
LegacySigningProvider(const LegacyScriptPubKeyMan& spk_man) : m_spk_man(spk_man) {} explicit LegacySigningProvider(const LegacyScriptPubKeyMan& spk_man) : m_spk_man(spk_man) {}
bool GetCScript(const CScriptID &scriptid, CScript& script) const override { return m_spk_man.GetCScript(scriptid, script); } bool GetCScript(const CScriptID &scriptid, CScript& script) const override { return m_spk_man.GetCScript(scriptid, script); }
bool HaveCScript(const CScriptID &scriptid) const override { return m_spk_man.HaveCScript(scriptid); } bool HaveCScript(const CScriptID &scriptid) const override { return m_spk_man.HaveCScript(scriptid); }

View File

@ -30,6 +30,7 @@ IGNORED_WARNINGS=(
"src/protocol.h:.* Class 'CMessageHeader' has a constructor with 1 argument that is not explicit." "src/protocol.h:.* Class 'CMessageHeader' has a constructor with 1 argument that is not explicit."
"src/qt/guiutil.h:.* Class 'ItemDelegate' has a constructor with 1 argument that is not explicit." "src/qt/guiutil.h:.* Class 'ItemDelegate' has a constructor with 1 argument that is not explicit."
"src/rpc/util.h:.* Struct 'RPCResults' has a constructor with 1 argument that is not explicit." "src/rpc/util.h:.* Struct 'RPCResults' has a constructor with 1 argument that is not explicit."
"src/rpc/util.h:.* Struct 'UniValueType' has a constructor with 1 argument that is not explicit."
"src/rpc/util.h:.* style: Struct 'UniValueType' has a constructor with 1 argument that is not explicit." "src/rpc/util.h:.* style: Struct 'UniValueType' has a constructor with 1 argument that is not explicit."
"src/script/descriptor.cpp:.* Class 'AddressDescriptor' has a constructor with 1 argument that is not explicit." "src/script/descriptor.cpp:.* Class 'AddressDescriptor' has a constructor with 1 argument that is not explicit."
"src/script/descriptor.cpp:.* Class 'ComboDescriptor' has a constructor with 1 argument that is not explicit." "src/script/descriptor.cpp:.* Class 'ComboDescriptor' has a constructor with 1 argument that is not explicit."
@ -42,6 +43,11 @@ IGNORED_WARNINGS=(
"src/script/descriptor.cpp:.* Class 'WSHDescriptor' has a constructor with 1 argument that is not explicit." "src/script/descriptor.cpp:.* Class 'WSHDescriptor' has a constructor with 1 argument that is not explicit."
"src/script/script.h:.* Class 'CScript' has a constructor with 1 argument that is not explicit." "src/script/script.h:.* Class 'CScript' has a constructor with 1 argument that is not explicit."
"src/script/standard.h:.* Class 'CScriptID' has a constructor with 1 argument that is not explicit." "src/script/standard.h:.* Class 'CScriptID' has a constructor with 1 argument that is not explicit."
"src/span.h:.* Class 'Span < const CRPCCommand >' has a constructor with 1 argument that is not explicit."
"src/span.h:.* Class 'Span < const char >' has a constructor with 1 argument that is not explicit."
"src/span.h:.* Class 'Span < const std :: vector <unsigned char > >' has a constructor with 1 argument that is not explicit."
"src/span.h:.* Class 'Span < const uint8_t >' has a constructor with 1 argument that is not explicit."
"src/span.h:.* Class 'Span' has a constructor with 1 argument that is not explicit."
"src/support/allocators/secure.h:.* Struct 'secure_allocator < char >' has a constructor with 1 argument that is not explicit." "src/support/allocators/secure.h:.* Struct 'secure_allocator < char >' has a constructor with 1 argument that is not explicit."
"src/support/allocators/secure.h:.* Struct 'secure_allocator < RNGState >' has a constructor with 1 argument that is not explicit." "src/support/allocators/secure.h:.* Struct 'secure_allocator < RNGState >' has a constructor with 1 argument that is not explicit."
"src/support/allocators/secure.h:.* Struct 'secure_allocator < unsigned char >' has a constructor with 1 argument that is not explicit." "src/support/allocators/secure.h:.* Struct 'secure_allocator < unsigned char >' has a constructor with 1 argument that is not explicit."
@ -49,6 +55,9 @@ IGNORED_WARNINGS=(
"src/test/checkqueue_tests.cpp:.* Struct 'FailingCheck' has a constructor with 1 argument that is not explicit." "src/test/checkqueue_tests.cpp:.* Struct 'FailingCheck' has a constructor with 1 argument that is not explicit."
"src/test/checkqueue_tests.cpp:.* Struct 'MemoryCheck' has a constructor with 1 argument that is not explicit." "src/test/checkqueue_tests.cpp:.* Struct 'MemoryCheck' has a constructor with 1 argument that is not explicit."
"src/test/checkqueue_tests.cpp:.* Struct 'UniqueCheck' has a constructor with 1 argument that is not explicit." "src/test/checkqueue_tests.cpp:.* Struct 'UniqueCheck' has a constructor with 1 argument that is not explicit."
"src/test/fuzz/util.h:.* Class 'FuzzedFileProvider' has a constructor with 1 argument that is not explicit."
"src/test/fuzz/util.h:.* Class 'FuzzedAutoFileProvider' has a constructor with 1 argument that is not explicit."
"src/util/ref.h:.* Class 'Ref' has a constructor with 1 argument that is not explicit."
"src/wallet/db.h:.* Class 'BerkeleyEnvironment' has a constructor with 1 argument that is not explicit." "src/wallet/db.h:.* Class 'BerkeleyEnvironment' has a constructor with 1 argument that is not explicit."
) )
@ -66,7 +75,7 @@ function join_array {
ENABLED_CHECKS_REGEXP=$(join_array "|" "${ENABLED_CHECKS[@]}") ENABLED_CHECKS_REGEXP=$(join_array "|" "${ENABLED_CHECKS[@]}")
IGNORED_WARNINGS_REGEXP=$(join_array "|" "${IGNORED_WARNINGS[@]}") IGNORED_WARNINGS_REGEXP=$(join_array "|" "${IGNORED_WARNINGS[@]}")
WARNINGS=$(git ls-files -- "*.cpp" "*.h" ":(exclude)src/dashbls/" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | \ WARNINGS=$(git ls-files -- "*.cpp" "*.h" ":(exclude)src/dashbls/" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | \
xargs cppcheck --enable=all -j "$(getconf _NPROCESSORS_ONLN)" --language=c++ --std=c++11 --template=gcc -D__cplusplus -DCLIENT_VERSION_BUILD -DCLIENT_VERSION_IS_RELEASE -DCLIENT_VERSION_MAJOR -DCLIENT_VERSION_MINOR -DCOPYRIGHT_YEAR -DDEBUG -I src/ -q 2>&1 | sort -u | \ xargs cppcheck --enable=all -j "$(getconf _NPROCESSORS_ONLN)" --language=c++ --std=c++17 --template=gcc -D__cplusplus -DCLIENT_VERSION_BUILD -DCLIENT_VERSION_IS_RELEASE -DCLIENT_VERSION_MAJOR -DCLIENT_VERSION_MINOR -DCOPYRIGHT_YEAR -DDEBUG -I src/ -q 2>&1 | sort -u | \
grep -E "${ENABLED_CHECKS_REGEXP}" | \ grep -E "${ENABLED_CHECKS_REGEXP}" | \
grep -vE "${IGNORED_WARNINGS_REGEXP}") grep -vE "${IGNORED_WARNINGS_REGEXP}")
if [[ ${WARNINGS} != "" ]]; then if [[ ${WARNINGS} != "" ]]; then