Address AI review feedback

- Mostly just clarifying comments and reorganizing functions.
This commit is contained in:
Ed Hennis
2026-06-25 15:31:51 -04:00
parent 6aa9031848
commit 4ff398ba2d
3 changed files with 41 additions and 29 deletions

View File

@@ -136,6 +136,10 @@ struct MantissaRange final
Large320,
// If Large330 is ever the only remaining "Large*" entry, it can be renamed to just "Large".
Large330,
// Large is a de-facto alias for "the latest", and is only here for backward compatibility
// in the extremely unlikely case that a downstream project made use of it. Note that
// because the behavior changed, this may still be a breaking change.
Large = Large330,
};
// This entire enum can be removed when the last relevant amendment is retired
@@ -146,6 +150,10 @@ struct MantissaRange final
Enabled320 = 1,
// If we ever get to the point that there's only one entry, remove the entire enum
Enabled330 = 2,
// Enabled is a de-facto alias for "the latest", and is only here for backward compatibility
// in the extremely unlikely case that a downstream project made use of it. Note that
// because the behavior changed, this may still be a breaking change.
Enabled = Enabled330,
};
explicit constexpr MantissaRange(MantissaScale sc) : scale(sc)
@@ -158,11 +166,25 @@ struct MantissaRange final
rep const max{(min * 10) - 1};
CuspRoundingFix const cuspRoundingFix{isCuspFixEnabled(scale)};
static constexpr MantissaRange const&
getMantissaRange(MantissaScale scale);
static constexpr std::set<MantissaScale> const&
getAllScales()
{
static std::set<MantissaRange::MantissaScale> const kScales = {
MantissaRange::MantissaScale::Small,
MantissaRange::MantissaScale::LargeLegacy,
MantissaRange::MantissaScale::Large320,
MantissaRange::MantissaScale::Large330,
};
return kScales;
}
static std::set<MantissaScale> const&
getAllScales();
class Get
{
static constexpr MantissaRange const&
mantissaRange(MantissaScale scale);
friend Number;
};
private:
static constexpr int

View File

@@ -30,7 +30,7 @@ namespace xrpl {
thread_local Number::RoundingMode Number::mode = Number::RoundingMode::ToNearest;
thread_local std::reference_wrapper<MantissaRange const> Number::kRange =
MantissaRange::getMantissaRange(MantissaRange::MantissaScale::Large330);
MantissaRange::Get::mantissaRange(MantissaRange::MantissaScale::Large330);
std::string
to_string(MantissaRange::MantissaScale const& scale)
@@ -68,20 +68,8 @@ to_string(Number::RoundingMode const& round)
}
}
std::set<MantissaRange::MantissaScale> const&
MantissaRange::getAllScales()
{
static std::set<MantissaRange::MantissaScale> const kScales = {
MantissaRange::MantissaScale::Small,
MantissaRange::MantissaScale::LargeLegacy,
MantissaRange::MantissaScale::Large320,
MantissaRange::MantissaScale::Large330,
};
return kScales;
}
constexpr MantissaRange const&
MantissaRange::getMantissaRange(MantissaScale scale)
MantissaRange::Get::mantissaRange(MantissaScale scale)
{
static constexpr MantissaRange kSmall{MantissaScale::Small};
static constexpr MantissaRange kLegacy{MantissaScale::LargeLegacy};
@@ -164,7 +152,7 @@ Number::setMantissaScale(MantissaRange::MantissaScale scale)
{
if (!MantissaRange::getAllScales().contains(scale))
logicError("Unknown mantissa scale");
kRange = MantissaRange::getMantissaRange(scale);
kRange = MantissaRange::Get::mantissaRange(scale);
}
// Optimization equivalent to:

View File

@@ -41,23 +41,24 @@ setCurrentTransactionRules(std::optional<Rules> r)
// Declare the range this way to keep clang-tidy from complaining
auto const range = [&r]() {
// If any new conditions with new amendments are added to "enableVaultNumbers", those
// If any new conditions with new amendments are added to "enableLargeNumbers", those
// amendments must also be added to useRulesGuards.
bool const enableVaultNumbers =
bool const enableLargeNumbers =
!r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol));
// If enableLargeNumbers is true, then useRulesGuard must also return true.
// However, the reverse is not true. Other amendments can cause the rules guard to be used,
// even though large numbers are _not_ used.
XRPL_ASSERT(
!r || !enableVaultNumbers || useRulesGuards(*r),
!r || !enableLargeNumbers || useRulesGuards(*r),
"setCurrentTransactionRules : rule decisions match");
bool const enableCuspRounding320 = !r || r->enabled(fixCleanup3_2_0);
bool const enableCuspRounding330 = !r || r->enabled(fixCleanup3_3_0);
if (enableVaultNumbers)
if (enableLargeNumbers)
{
if (enableCuspRounding330)
if (!r || r->enabled(fixCleanup3_3_0))
{
return MantissaRange::MantissaScale::Large330;
}
if (enableCuspRounding320)
if (r->enabled(fixCleanup3_2_0))
{
return MantissaRange::MantissaScale::Large320;
}
@@ -74,7 +75,7 @@ bool
useRulesGuards(Rules const& rules)
{
// The list of amendments used here - to decide whether to create a RulesGuard - must be a
// superset of the list used to determine "enableVaultNumbers" in setCurrentTransactionRules.
// superset of the list used to determine "enableLargeNumbers" in setCurrentTransactionRules.
// Additional amendments can be added if desired.
//
// As soon as any one of these amendments is retired, this whole function can be removed, along
@@ -92,7 +93,8 @@ createGuards(
{
if (useRulesGuards(rules))
{
// raii classes for the current ledger rules.
// raii classes for the current ledger rules. If the rules are set, the MantissaRange will
// be updated, too.
rulesGuard.emplace(rules);
}
else