From 74c66d0944f9da3dd94ac670e7b6c5524f15af65 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 5 Jun 2026 12:06:41 -0400 Subject: [PATCH] refactor: Construct Number::Guard from MantissaRange or relevant fields - Simplifies the function signatures in Guard, because it doesn't need to have those values passed in constantly. - Also simplifies some of the functions because they don't need to store values just to pass them to Guard functions. --- include/xrpl/basics/Number.h | 14 ++- src/libxrpl/basics/Number.cpp | 194 ++++++++++++++-------------------- 2 files changed, 86 insertions(+), 122 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 8043a850a8..dd74208485 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -147,7 +147,7 @@ struct MantissaRange final int const log{getExponent(scale)}; rep const min{getMin(scale, log)}; rep const max{(min * 10) - 1}; - CuspRoundingFix const cuspRoundingFixEnabled{isCuspFixEnabled(scale)}; + CuspRoundingFix const cuspRoundingFix{isCuspFixEnabled(scale)}; static MantissaRange const& getMantissaRange(MantissaScale scale); @@ -549,9 +549,15 @@ private: // changing the values inside the range. static thread_local std::reference_wrapper kRange; + class Guard; + void normalize(MantissaRange const& range); + // Guard has the fields that we need, as well as MantissaRange, so if we have a guard, use that + void + normalize(Guard const& guard); + /** Normalize Number components to an arbitrary range. * * min/maxMantissa are parameters because this function is used by both @@ -566,7 +572,7 @@ private: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); + MantissaRange::CuspRoundingFix cuspRoundingFix); template friend void @@ -576,7 +582,7 @@ private: int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped); [[nodiscard]] bool @@ -594,8 +600,6 @@ private: // UB, and can vary across compilers. static internalrep externalToInternal(rep mantissa); - - class Guard; }; constexpr Number::Number(bool negative, internalrep mantissa, int exponent, Unchecked) noexcept diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 8e3887e475..7717495cbe 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -65,7 +65,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 15); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max < Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -76,7 +76,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -87,7 +87,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Enabled); } return map; }(); @@ -171,7 +171,21 @@ class Number::Guard std::uint8_t sbit_ : 1 {0}; // the sign of the guard digits public: - explicit Guard() = default; + internalrep const minMantissa_; + internalrep const maxMantissa_; + MantissaRange::CuspRoundingFix const cuspRoundingFix_; + + explicit Guard( + internalrep const& minMantissa, + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFix) + : minMantissa_(minMantissa), maxMantissa_(maxMantissa), cuspRoundingFix_(cuspRoundingFix) + { + } + + explicit Guard(MantissaRange const& range) : Guard(range.min, range.max, range.cuspRoundingFix) + { + } // set & test the sign bit void @@ -221,19 +235,12 @@ public: // Modify the result to the correctly rounded value template void - doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location); + doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); // Modify the result to the correctly rounded value template void - doRoundDown(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + doRoundDown(bool& negative, T& mantissa, int& exponent); // Modify the result to the correctly rounded value void @@ -245,7 +252,7 @@ private: template void - bringIntoRange(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + bringIntoRange(bool& negative, T& mantissa, int& exponent); }; inline void @@ -359,15 +366,11 @@ Number::Guard::round() const noexcept template void -Number::Guard::bringIntoRange( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) { // Bring mantissa back into the minMantissa / maxMantissa range AFTER // rounding - if (mantissa < minMantissa) + if (mantissa < minMantissa_) { mantissa *= 10; --exponent; @@ -384,22 +387,15 @@ Number::Guard::bringIntoRange( template void -Number::Guard::doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location) +Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location) { auto r = round(); if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { - auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { - return mantissa < maxMantissa && mantissa < kMaxRep; + auto const safeToIncrement = [this](auto const& mantissa) { + return mantissa < maxMantissa_ && mantissa < kMaxRep; }; - if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) { // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". @@ -420,14 +416,7 @@ Number::Guard::doRoundUp( safeToIncrement(mantissa), "xrpl::Number::Guard::doRoundUp", "can't recurse more than once"); - doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - location); + doRoundUp(negative, mantissa, exponent, location); return; } } @@ -438,7 +427,7 @@ Number::Guard::doRoundUp( ++mantissa; // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa || mantissa > kMaxRep) + if (mantissa > maxMantissa_ || mantissa > kMaxRep) { // Don't use doDropDigit here mantissa /= 10; @@ -446,30 +435,26 @@ Number::Guard::doRoundUp( } } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + bringIntoRange(negative, mantissa, exponent); if (exponent > kMaxExponent) Throw(std::string(location)); } template void -Number::Guard::doRoundDown( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) { auto r = round(); if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { --mantissa; - if (mantissa < minMantissa) + if (mantissa < minMantissa_) { mantissa *= 10; --exponent; } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + bringIntoRange(negative, mantissa, exponent); } // Modify the result to the correctly rounded value @@ -536,7 +521,7 @@ doNormalize( int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped) { static constexpr auto kMinExponent = Number::kMinExponent; @@ -559,7 +544,7 @@ doNormalize( m *= 10; --exponent; } - Guard g; + Guard g(minMantissa, maxMantissa, cuspRoundingFix); if (negative) g.setNegative(); if (dropped) @@ -604,14 +589,7 @@ doNormalize( XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::normalize 2"); + g.doRoundUp(negative, mantissa, exponent, "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -626,13 +604,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -644,13 +621,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -662,16 +638,27 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); } void Number::normalize(MantissaRange const& range) { - normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFix); +} + +void +Number::normalize(Guard const& guard) +{ + normalize( + negative_, + mantissa_, + exponent_, + guard.minMantissa_, + guard.maxMantissa_, + guard.cuspRoundingFix_); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -725,19 +712,20 @@ Number::operator+=(Number const& y) bool const yn = y.negative_; uint128_t ym = y.mantissa_; auto ye = y.exponent_; - Guard g; + Guard g(kRange); - auto const& range = kRange.get(); + auto const& minMantissa = g.minMantissa_; + auto const& maxMantissa = g.maxMantissa_; + auto const cuspRoundingFix = g.cuspRoundingFix_; // Bring the exponents of both values into agreement, so the mantissas are on the same scale // and can be added directly together // expandM / expandE: First try to expand the mantissa and bring the exponent down // shringM / shrinkE: Then shrink the mantissa and bring the exponent up, if necessary - auto const adjust = [&g, &range]( - uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { + auto const adjust = [&g](uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { constexpr uint128_t kSafeLimit = kPowerOfTenImpl[37]; - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (g.cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) { while (shrinkE < expandE && shrinkM % 10 == 0) { @@ -774,14 +762,10 @@ Number::operator+=(Number const& y) adjust(xm, xe, ym, ye); } - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; - if (xn == yn) { xm += ym; - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) { while (xm > maxMantissa || xm > kMaxRep) { @@ -795,14 +779,7 @@ Number::operator+=(Number const& y) g.doDropDigit(xm, xe); } } - g.doRoundUp( - xn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::addition overflow"); + g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } else { @@ -822,32 +799,25 @@ Number::operator+=(Number const& y) xm -= g.pop(); --xe; } - g.doRoundDown(xn, xm, xe, minMantissa); - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled && xm != 0) + g.doRoundDown(xn, xm, xe); + if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled && xm != 0) { - // make a new guard - Guard g; + // this will be going away + Guard g(kRange); if (xn) g.setNegative(); while (xm > maxMantissa || xm > kMaxRep) { g.doDropDigit(xm, xe); } - g.doRoundUp( - xn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::addition overflow"); + g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } } negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(range); + normalize(g); return *this; } @@ -881,14 +851,11 @@ Number::operator*=(Number const& y) auto ze = xe + ye; auto zs = xs * ys; bool zn = (zs == -1); - Guard g; + Guard g(kRange); if (zn) g.setNegative(); - auto const& range = kRange.get(); - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const& maxMantissa = g.maxMantissa_; while (zm > maxMantissa || zm > kMaxRep) { @@ -897,19 +864,12 @@ Number::operator*=(Number const& y) xm = static_cast(zm); xe = ze; - g.doRoundUp( - zn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::multiplication overflow : exponent is " + std::to_string(xe)); + g.doRoundUp(zn, xm, xe, "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(range); + normalize(g); return *this; } @@ -945,7 +905,7 @@ Number::operator/=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const cuspRoundingFix = range.cuspRoundingFix; // Division operates on two large integers (16-digit for small // mantissas, 19-digit for large) using integer math. If the values @@ -1077,14 +1037,14 @@ Number::operator/=(Number const& y) // rounding fix is enabled, flag if there is still // a remainder from stage 2. bool const useTrailingRemainder = - cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled; + cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled; if (useTrailingRemainder) { dropped = partialNumerator % dm != 0; } } } - doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled, dropped); + doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFix, dropped); negative_ = zp; mantissa_ = static_cast(zm); exponent_ = ze; @@ -1098,7 +1058,7 @@ operator rep() const { rep drops = mantissa(); int offset = exponent(); - Guard g; + Guard g(kRange); if (drops != 0) { if (negative_)