chore: Enable most cppcoreguidelines checks (#7660)

This commit is contained in:
Ayaz Salikhov
2026-07-07 14:12:52 +01:00
committed by GitHub
parent f5e63f8a91
commit c5a6de6ef7
29 changed files with 86 additions and 74 deletions

View File

@@ -9,37 +9,24 @@ Checks: "-*,
cppcoreguidelines-*,
-cppcoreguidelines-avoid-c-arrays,
-cppcoreguidelines-avoid-capturing-lambda-coroutines,
-cppcoreguidelines-avoid-const-or-ref-data-members,
-cppcoreguidelines-avoid-do-while,
-cppcoreguidelines-avoid-goto,
-cppcoreguidelines-avoid-magic-numbers,
-cppcoreguidelines-avoid-non-const-global-variables,
-cppcoreguidelines-avoid-reference-coroutine-parameters,
-cppcoreguidelines-c-copy-assignment-signature,
-cppcoreguidelines-explicit-virtual-functions,
-cppcoreguidelines-interfaces-global-init,
-cppcoreguidelines-macro-to-enum,
-cppcoreguidelines-macro-usage,
-cppcoreguidelines-missing-std-forward,
-cppcoreguidelines-narrowing-conversions,
-cppcoreguidelines-no-malloc,
-cppcoreguidelines-noexcept-destructor,
-cppcoreguidelines-noexcept-move-operations,
-cppcoreguidelines-noexcept-swap,
-cppcoreguidelines-non-private-member-variables-in-classes,
-cppcoreguidelines-owning-memory,
-cppcoreguidelines-prefer-member-initializer,
-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
-cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,
-cppcoreguidelines-pro-bounds-constant-array-index,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-cppcoreguidelines-pro-type-const-cast,
-cppcoreguidelines-pro-type-cstyle-cast,
-cppcoreguidelines-pro-type-reinterpret-cast,
-cppcoreguidelines-pro-type-union-access,
-cppcoreguidelines-pro-type-vararg,
-cppcoreguidelines-slicing,
-cppcoreguidelines-special-member-functions,
llvm-namespace-comment,

View File

@@ -138,11 +138,8 @@ public:
{
}
ConstIterator(Iterator const& orig)
ConstIterator(Iterator const& orig) : map(orig.map), ait(orig.ait), mit(orig.mit)
{
map = orig.map;
ait = orig.ait;
mit = orig.mit;
}
const_reference
@@ -231,11 +228,11 @@ private:
public:
PartitionedUnorderedMap(std::optional<std::size_t> partitions = std::nullopt)
{
// Set partitions to the number of hardware threads if the parameter
// is either empty or set to 0.
partitions_ =
partitions && (*partitions != 0u) ? *partitions : std::thread::hardware_concurrency();
: partitions_(
partitions && (*partitions != 0u) ? *partitions : std::thread::hardware_concurrency())
{
map_.resize(partitions_);
XRPL_ASSERT(
partitions_,

View File

@@ -359,6 +359,7 @@ private:
deleteElement(Element const* p)
{
ElementAllocatorTraits::destroy(config_.alloc(), p);
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
ElementAllocatorTraits::deallocate(config_.alloc(), const_cast<Element*>(p), 1);
}

View File

@@ -528,6 +528,7 @@ private:
deleteElement(Element const* p)
{
ElementAllocatorTraits::destroy(config_.alloc(), p);
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
ElementAllocatorTraits::deallocate(config_.alloc(), const_cast<Element*>(p), 1);
}

View File

@@ -23,6 +23,7 @@ typeName()
if (auto s = abi::__cxa_demangle(name.c_str(), nullptr, nullptr, nullptr))
{
name = s;
// NOLINTNEXTLINE(cppcoreguidelines-no-malloc)
std::free(s);
}
#endif

View File

@@ -34,11 +34,11 @@ public:
createObject();
private:
bool success_;
bool success_{false};
void const* key_;
NodeObjectType objectType_;
unsigned char const* objectData_;
NodeObjectType objectType_{NodeObjectType::Unknown};
unsigned char const* objectData_{nullptr};
int dataBytes_;
};

View File

@@ -240,6 +240,9 @@ private:
inline STPathElement::STPathElement() : type_(TypeNone), isOffer_(true)
{
// hashValue_ is derived from the whole object, so it is computed in the body
// once every other member is initialized (as in the other constructors).
// NOLINTNEXTLINE(cppcoreguidelines-prefer-member-initializer)
hashValue_ = getHash(*this);
}
@@ -315,6 +318,9 @@ inline STPathElement::STPathElement(
assetID_.visit(
[&](Currency const&) { type_ = type_ & (~Type::TypeMpt); },
[&](MPTID const&) { type_ = type_ & (~Type::TypeCurrency); });
// hashValue_ must be computed after type_ is adjusted above, so this cannot
// be a member initializer.
// NOLINTNEXTLINE(cppcoreguidelines-prefer-member-initializer)
hashValue_ = getHash(*this);
}

View File

@@ -50,14 +50,13 @@ public:
STVar&
operator=(STVar&& rhs);
STVar(STBase&& t) // NOLINT(cppcoreguidelines-rvalue-reference-param-not-moved)
// NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved)
STVar(STBase&& t) : p_(t.move(kMaxSize, &d_))
{
p_ = t.move(kMaxSize, &d_);
}
STVar(STBase const& t)
STVar(STBase const& t) : p_(t.copy(kMaxSize, &d_))
{
p_ = t.copy(kMaxSize, &d_);
}
STVar(DefaultObjectT, SField const& name);

View File

@@ -138,6 +138,7 @@ intrusive_ptr_release(SHAMapItem const* x)
// If the slabber doesn't claim this pointer, it was allocated
// manually, so we free it manually.
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
if (!detail::gSlabber.deallocate(const_cast<std::uint8_t*>(p)))
delete[] p;
}

View File

@@ -92,6 +92,7 @@ public:
[[nodiscard]] TOffer<TIn, TOut>&
tip() const
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
return const_cast<TOfferStreamBase*>(this)->offer_;
}

View File

@@ -230,9 +230,8 @@ Writer::~Writer()
impl_->finishAll();
}
Writer::Writer(Writer&& w) noexcept
Writer::Writer(Writer&& w) noexcept : impl_(std::move(w.impl_))
{
impl_ = std::move(w.impl_);
}
Writer&

View File

@@ -624,11 +624,13 @@ Reader::decodeDouble(Token& token)
Char buffer[bufferSize + 1];
memcpy(buffer, token.start, length);
buffer[length] = 0;
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
count = sscanf(buffer, format, &value);
}
else
{
std::string const buffer(token.start, token.end);
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
count = sscanf(buffer.c_str(), format, &value);
}
if (count != 1)

View File

@@ -48,6 +48,7 @@ public:
if (length == kUnknown)
length = (value != nullptr) ? (unsigned int)strlen(value) : 0;
// NOLINTNEXTLINE(cppcoreguidelines-no-malloc)
char* newString = static_cast<char*>(malloc(length + 1));
if (value != nullptr)
memcpy(newString, value, length);
@@ -59,7 +60,10 @@ public:
releaseStringValue(char* value) override
{
if (value != nullptr)
{
// NOLINTNEXTLINE(cppcoreguidelines-no-malloc)
free(value);
}
}
};
@@ -120,7 +124,10 @@ Value::CZString::CZString(CZString const& other)
Value::CZString::~CZString()
{
if ((cstr_ != nullptr) && index_ == static_cast<int>(DuplicationPolicy::Duplicate))
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
valueAllocator()->releaseMemberName(const_cast<char*>(cstr_));
}
}
bool
@@ -241,6 +248,7 @@ Value::Value(std::string const& value) : type_(ValueType::String), allocated_(tr
Value::Value(StaticString const& value) : type_(ValueType::String)
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
value_.stringVal = const_cast<char*>(value.cStr());
}

View File

@@ -86,6 +86,7 @@ valueToString(double value)
// to avoid warning.
sprintf_s(buffer, sizeof(buffer), "%.16g", value);
#else
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
snprintf(buffer, sizeof(buffer), "%.16g", value);
#endif
return buffer;

View File

@@ -12,7 +12,7 @@
namespace xrpl::NodeStore {
DecodedBlob::DecodedBlob(void const* key, void const* value, int valueBytes)
DecodedBlob::DecodedBlob(void const* key, void const* value, int valueBytes) : key_(key)
{
/* Data format:
@@ -23,10 +23,6 @@ DecodedBlob::DecodedBlob(void const* key, void const* value, int valueBytes)
9...end The body of the object data
*/
success_ = false;
key_ = key;
objectType_ = NodeObjectType::Unknown;
objectData_ = nullptr;
dataBytes_ = std::max(0, valueBytes - 9);
// VFALCO NOTE What about bytes 4 through 7 inclusive?

View File

@@ -152,7 +152,12 @@ std::uint64_t
getQuality(uint256 const& uBase)
{
// VFALCO [base_uint] This assumes a certain storage format
return boost::endian::big_to_native(((std::uint64_t*)uBase.end())[-1]);
//
// Load the final 8 bytes as a big-endian integer. load_big_u64 reads
// through unaligned byte storage (via memcpy) and applies the endian
// conversion, avoiding the alignment/strict-aliasing UB of casting the
// unsigned char* returned by end() to a std::uint64_t*.
return boost::endian::load_big_u64(uBase.end() - 8);
}
uint256
@@ -278,8 +283,11 @@ quality(Keylet const& k, std::uint64_t q) noexcept
// for indexes.
uint256 x = k.key;
// FIXME This is ugly and we can and should do better...
((std::uint64_t*)x.end())[-1] = boost::endian::native_to_big(q);
// Store the quality as a big-endian integer in the final 8 bytes.
// store_big_u64 writes through unaligned byte storage (via memcpy) and
// applies the endian conversion, avoiding the alignment/strict-aliasing UB
// of casting the unsigned char* returned by end() to a std::uint64_t*.
boost::endian::store_big_u64(x.end() - 8, q);
return {ltDIR_NODE, x};
}

View File

@@ -21,11 +21,12 @@ SOTemplate::SOTemplate(
}
SOTemplate::SOTemplate(std::vector<SOElement> uniqueFields, std::vector<SOElement> commonFields)
: indices_(SField::getNumFields() + 1, -1) // Unmapped indices == -1
: elements_(std::move(uniqueFields))
, indices_(SField::getNumFields() + 1, -1) // Unmapped indices == -1
{
// Add all SOElements.
//
elements_ = std::move(uniqueFields);
std::ranges::move(commonFields, std::back_inserter(elements_));
// Validate and index elements_.

View File

@@ -66,9 +66,9 @@ getTxFormat(TxType type)
return format;
}
STTx::STTx(STObject&& object) : STObject(std::move(object))
STTx::STTx(STObject&& object)
: STObject(std::move(object)), txType_(safeCast<TxType>(getFieldU16(sfTransactionType)))
{
txType_ = safeCast<TxType>(getFieldU16(sfTransactionType));
applyTemplate(getTxFormat(txType_)->getSOTemplate()); // may throw
tid_ = getHash(HashPrefix::TransactionId);
buildBatchTxnIds();
@@ -100,6 +100,9 @@ STTx::STTx(TxType type, std::function<void(STObject&)> assembler) : STObject(sfT
assembler(*this);
// txType_ must be read after the object is assembled, so this cannot be a
// member initializer.
// NOLINTNEXTLINE(cppcoreguidelines-prefer-member-initializer)
txType_ = safeCast<TxType>(getFieldU16(sfTransactionType));
if (txType_ != type)

View File

@@ -99,7 +99,7 @@ int
Serializer::addRaw(void const* ptr, int len)
{
int const ret = data_.size();
data_.insert(data_.end(), (char const*)ptr, ((char const*)ptr) + len);
data_.insert(data_.end(), static_cast<char const*>(ptr), static_cast<char const*>(ptr) + len);
return ret;
}

View File

@@ -23,16 +23,16 @@
namespace xrpl {
TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, STObject const& obj)
: transactionID_(txid), ledgerSeq_(ledger), nodes_(obj.getFieldArray(sfAffectedNodes))
: transactionID_(txid)
, ledgerSeq_(ledger)
, index_(obj.getFieldU32(sfTransactionIndex))
, result_(obj.getFieldU8(sfTransactionResult))
, nodes_([&obj] {
auto const affectedNodes = dynamic_cast<STArray const*>(obj.peekAtPField(sfAffectedNodes));
XRPL_ASSERT(affectedNodes, "xrpl::TxMeta::TxMeta(STObject) : type cast succeeded");
return affectedNodes != nullptr ? *affectedNodes : obj.getFieldArray(sfAffectedNodes);
}())
{
result_ = obj.getFieldU8(sfTransactionResult);
index_ = obj.getFieldU32(sfTransactionIndex);
auto affectedNodes = dynamic_cast<STArray const*>(obj.peekAtPField(sfAffectedNodes));
XRPL_ASSERT(affectedNodes, "xrpl::TxMeta::TxMeta(STObject) : type cast succeeded");
if (affectedNodes != nullptr)
nodes_ = *affectedNodes;
setAdditionalFields(obj);
}

View File

@@ -838,6 +838,7 @@ SHAMap::getHash() const
auto hash = root_->getHash();
if (hash.isZero())
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
const_cast<SHAMap&>(*this).unshare();
hash = root_->getHash();
}

View File

@@ -1157,7 +1157,11 @@ toClaim(STTx const& tx)
try
{
STObject o{tx};
// Copy just the field bag out of the transaction (explicitly, via the
// STObject base) so it can be reinterpreted as a cross-chain attestation
// below, with sfAccount replaced by sfOtherChainSource. STTx-specific
// state (txType_, tid_) is intentionally not needed here.
STObject o{static_cast<STObject const&>(tx)};
o.setAccountID(sfAccount, o[sfOtherChainSource]);
return TAttestation(o);
}

View File

@@ -40,7 +40,6 @@
#include <cassert>
#include <cstddef>
#include <cstdint>
#include <cstdio>
#include <initializer_list>
#include <optional>
#include <stdexcept>
@@ -309,31 +308,26 @@ struct ExistingElementPool
currencyNames.clear();
currencyNames.reserve(numCur);
static constexpr size_t kBufSize = 32;
char buf[kBufSize];
for (size_t id = 0; id < numAct; ++id)
{
snprintf(buf, kBufSize, "A%zu", id);
accounts.emplace_back(buf);
}
accounts.emplace_back("A" + std::to_string(id));
for (size_t id = 0; id < numCur; ++id)
{
std::string name;
if (id < 10)
{
snprintf(buf, kBufSize, "CC%zu", id);
name = "CC" + std::to_string(id);
}
else if (id < 100)
{
snprintf(buf, kBufSize, "C%zu", id);
name = "C" + std::to_string(id);
}
else
{
snprintf(buf, kBufSize, "%zu", id);
name = std::to_string(id);
}
currencies.emplace_back(toCurrency(buf));
currencyNames.emplace_back(buf);
currencies.emplace_back(toCurrency(name));
currencyNames.emplace_back(name);
}
for (auto const& a : accounts)

View File

@@ -245,9 +245,9 @@ struct Balance
T& env;
STAmount startAmount;
Balance(T& env, jtx::Account const& account) : account(account), env(env)
Balance(T& env, jtx::Account const& account)
: account(account), env(env), startAmount(env.balance(account))
{
startAmount = env.balance(account);
}
[[nodiscard]] STAmount

View File

@@ -135,9 +135,8 @@ class PowerLawDistribution
public:
using result_type = double;
PowerLawDistribution(double xmin, double a) : xmin_{xmin}, a_{a}
PowerLawDistribution(double xmin, double a) : xmin_{xmin}, a_{a}, inv_(1.0 / (1.0 - a_))
{
inv_ = 1.0 / (1.0 - a_);
}
template <class Generator>

View File

@@ -323,12 +323,11 @@ class Validator
using Links = std::unordered_map<Peer::id_t, LinkSPtr>;
public:
Validator() : pkey_(std::get<0>(randomKeyPair(KeyType::Ed25519)))
Validator() : pkey_(std::get<0>(randomKeyPair(KeyType::Ed25519))), id_(sid++)
{
protocol::TMValidation v;
v.set_validation("validation");
message_ = std::make_shared<Message>(v, protocol::mtVALIDATION, pkey_);
id_ = sid++;
}
Validator(Validator const&) = default;
Validator(Validator&&) = default;
@@ -457,7 +456,6 @@ public:
using id_t = Peer::id_t;
PeerSim(Overlay& overlay, beast::Journal journal) : overlay_(overlay), squelch_(journal)
{
id_ = sid++;
}
~PeerSim() override = default;
@@ -512,7 +510,7 @@ public:
private:
inline static id_t sid = 0;
std::string fingerprint_;
id_t id_;
id_t id_{sid++};
Overlay& overlay_;
reduce_relay::Squelch<ManualClock> squelch_;
};

View File

@@ -1366,6 +1366,7 @@ public:
{
fail("Unable to build object from json");
}
// NOLINTNEXTLINE(cppcoreguidelines-slicing)
else if (STObject(j) != parsed.object)
{
log << "ORIG: " << j.getJson(JsonOptions::Values::None) << '\n'

View File

@@ -80,7 +80,9 @@ TxTest::TxTest(std::optional<FeatureBitset> features)
std::vector<uint256>{featureSet_.begin(), featureSet_.end()},
registry_.getNodeFamily());
// Initialize time from the genesis ledger
// Initialize time from the genesis ledger. closedLedger_ is created above
// in the body, so this cannot be a member initializer.
// NOLINTNEXTLINE(cppcoreguidelines-prefer-member-initializer)
now_ = closedLedger_->header().closeTime;
// Create an open view on top of the genesis ledger

View File

@@ -643,6 +643,7 @@ AmendmentState*
AmendmentTableImpl::get(uint256 const& amendmentHash, std::scoped_lock<std::mutex> const& lock)
{
// Forward to the const version of get.
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
return const_cast<AmendmentState*>(std::as_const(*this).get(amendmentHash, lock));
}