From fc29fbe946e0c915ed3ba1b6deae860386bfc8e2 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 26 Jan 2026 19:50:38 -0500 Subject: [PATCH] Convert "bool negative_ & uint64_t mantissa_" combo back to "rep mantissa_" --- include/xrpl/basics/Number.h | 54 +++++++--------- src/libxrpl/basics/Number.cpp | 108 +++++++++++++++++++++----------- src/test/basics/Number_test.cpp | 1 - 3 files changed, 91 insertions(+), 72 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 485d5b4908..af53779c6a 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -138,12 +138,7 @@ private: static constexpr rep computeMin(rep max) { - auto min = max + 1; - auto const r = min % 10; - min /= 10; - if (r != 0) - ++min; - return min; + return max / 10 + 1; } static constexpr rep @@ -274,9 +269,7 @@ class Number using rep = std::int64_t; using internalrep = MantissaRange::rep; - // TODO: Get rid of negative_ and convert mantissa back to rep - bool negative_{false}; - internalrep mantissa_{0}; + rep mantissa_{0}; int exponent_{std::numeric_limits::lowest()}; public: @@ -374,8 +367,7 @@ public: friend constexpr bool operator==(Number const& x, Number const& y) noexcept { - return x.negative_ == y.negative_ && x.mantissa_ == y.mantissa_ && - x.exponent_ == y.exponent_; + return x.mantissa_ == y.mantissa_ && x.exponent_ == y.exponent_; } friend constexpr bool @@ -389,8 +381,8 @@ public: { // If the two amounts have different signs (zero is treated as positive) // then the comparison is true iff the left is negative. - bool const lneg = x.negative_; - bool const rneg = y.negative_; + bool const lneg = x.mantissa_ < 0; + bool const rneg = y.mantissa_ < 0; if (lneg != rneg) return lneg; @@ -418,7 +410,7 @@ public: constexpr int signum() const noexcept { - return negative_ ? -1 : (mantissa_ ? 1 : 0); + return mantissa_ < 0 ? -1 : (mantissa_ ? 1 : 0); } Number @@ -601,6 +593,11 @@ private: std::tuple toInternal() const; + // Set the Number from an internal representation + template + void + fromInternal(bool negative, Rep mantissa, int exponent); + class Guard; public: @@ -612,7 +609,8 @@ inline constexpr Number::Number( internalrep mantissa, int exponent, unchecked) noexcept - : negative_(negative), mantissa_{mantissa}, exponent_{exponent} + : mantissa_{(negative ? -1 : 1) * static_cast(mantissa)} + , exponent_{exponent} { } @@ -626,16 +624,6 @@ inline constexpr Number::Number( constexpr static Number numZero{}; -inline Number::Number( - bool negative, - internalrep mantissa, - int exponent, - normalized) - : Number(negative, mantissa, exponent, unchecked{}) -{ - normalize(); -} - inline Number::Number(internalrep mantissa, int exponent, normalized) : Number(false, mantissa, exponent, normalized{}) { @@ -658,8 +646,7 @@ inline Number::Number(rep mantissa) : Number{mantissa, 0} inline constexpr Number::rep Number::mantissa() const noexcept { - auto const sign = negative_ ? -1 : 1; - return sign * static_cast(mantissa_); + return mantissa_; } /** Returns the exponent of the external view of the Number. @@ -685,7 +672,7 @@ Number::operator-() const noexcept if (mantissa_ == 0) return Number{}; auto x = *this; - x.negative_ = !x.negative_; + x.mantissa_ = -1 * x.mantissa_; return x; } @@ -779,7 +766,8 @@ inline bool Number::isnormal() const noexcept { MantissaRange const& range = range_; - auto const abs_m = mantissa_; + auto const abs_m = mantissa_ < 0 ? -mantissa_ : mantissa_; + return *this == Number{} || (range.min <= abs_m && abs_m <= range.max && // minExponent <= exponent_ && exponent_ <= maxExponent); @@ -789,8 +777,9 @@ template std::pair Number::normalizeToRange(T minMantissa, T maxMantissa) const { - bool negative = negative_; - internalrep mantissa = mantissa_; + bool negative = mantissa_ < 0; + auto const sign = negative ? -1 : 1; + internalrep mantissa = sign * mantissa_; int exponent = exponent_; if constexpr (std::is_unsigned_v) @@ -800,8 +789,7 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const "Number is non-negative for unsigned range."); Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); - auto const sign = negative ? -1 : 1; - return std::make_pair(static_cast(sign * mantissa), exponent); + return std::make_pair(sign * static_cast(mantissa), exponent); } inline constexpr Number diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 541920045a..f35263852c 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -238,7 +238,7 @@ Number::Guard::bringIntoRange( { constexpr Number zero = Number{}; - negative = zero.negative_; + negative = false; mantissa = zero.mantissa_; exponent = zero.exponent_; } @@ -346,25 +346,54 @@ std::tuple Number::toInternal() const { auto exponent = exponent_; - Rep mantissa = mantissa_; - bool const negative = negative_; + bool const negative = mantissa_ < 0; + auto const sign = negative ? -1 : 1; + Rep mantissa = static_cast(sign * mantissa_); - auto const referenceMin = Number::range_.get().referenceMin; + auto const& range = Number::range_.get(); + auto const referenceMin = range.referenceMin; + auto const minMantissa = range.min; - if (mantissa != 0 && mantissa < referenceMin) + if (mantissa != 0 && mantissa >= minMantissa && mantissa < referenceMin) { // Ensure the mantissa has the correct number of digits mantissa *= 10; --exponent; + XRPL_ASSERT_PARTS( + mantissa >= referenceMin && mantissa < referenceMin * 10, + "ripple::Number::toInternal()", + "Number is within reference range and has 'log' digits"); } - XRPL_ASSERT_PARTS( - mantissa >= referenceMin && mantissa < referenceMin * 10, - "ripple::Number::toInternal()", - "Number is within reference range and has 'log' digits"); return {negative, mantissa, exponent}; } +template +void +Number::fromInternal(bool negative, Rep mantissa, int exponent) +{ + auto const sign = negative ? -1 : 1; + + auto const& range = Number::range_.get(); + auto const maxMantissa = range.max; + + XRPL_ASSERT_PARTS( + mantissa <= maxMantissa, + "ripple::Number::fromInternal", + "mantissa in range"); + if constexpr (std::is_signed_v) + XRPL_ASSERT_PARTS( + mantissa >= -maxMantissa, + "xrpl::Number::fromInternal", + "negative mantissa in range"); + + mantissa_ = sign * static_cast(mantissa); + exponent_ = exponent; + + XRPL_ASSERT_PARTS( + isnormal(), "ripple::Number::fromInternal", "Number is normalized"); +} + constexpr Number Number::oneSmall() { @@ -418,7 +447,7 @@ doNormalize( { mantissa = zero.mantissa_; exponent = zero.exponent_; - negative = zero.negative_; + negative = false; return; } @@ -443,7 +472,7 @@ doNormalize( { mantissa = zero.mantissa_; exponent = zero.exponent_; - negative = zero.negative_; + negative = false; return; } @@ -511,7 +540,12 @@ void Number::normalize() { auto const& range = range_.get(); - normalize(negative_, mantissa_, exponent_, range.min, range.max); + + auto [negative, mantissa, exponent] = toInternal(); + + normalize(negative, mantissa, exponent, range.min, range.max); + + fromInternal(negative, mantissa, exponent); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -521,6 +555,9 @@ Number Number::shiftExponent(int exponentDelta) const { XRPL_ASSERT_PARTS(isnormal(), "xrpl::Number::shiftExponent", "normalized"); + + auto const [negative, mantissa, exponent] = toInternal(); + auto const newExponent = exponent_ + exponentDelta; if (newExponent >= maxExponent) throw std::overflow_error("Number::shiftExponent"); @@ -528,7 +565,9 @@ Number::shiftExponent(int exponentDelta) const { return Number{}; } - Number const result{negative_, mantissa_, newExponent, unchecked{}}; + + Number const result{negative, mantissa, newExponent, unchecked{}}; + XRPL_ASSERT_PARTS( result.isnormal(), "xrpl::Number::shiftExponent", @@ -536,6 +575,13 @@ Number::shiftExponent(int exponentDelta) const return result; } +Number::Number(bool negative, internalrep mantissa, int exponent, normalized) +{ + auto const& range = range_.get(); + normalize(negative, mantissa, exponent, range.min, range.max); + fromInternal(negative, mantissa, exponent); +} + Number& Number::operator+=(Number const& y) { @@ -628,10 +674,8 @@ Number::operator+=(Number const& y) g.doRoundDown(xn, xm, xe, minMantissa); } - negative_ = xn; - mantissa_ = static_cast(xm); - exponent_ = xe; - normalize(); + normalize(xn, xm, xe, minMantissa, maxMantissa); + fromInternal(xn, xm, xe); return *this; } @@ -679,15 +723,11 @@ Number::operator*=(Number const& y) // *m = mantissa // *e = exponent - bool xn = negative_; + auto [xn, xm, xe] = toInternal(); int xs = xn ? -1 : 1; - internalrep xm = mantissa_; - auto xe = exponent_; - bool yn = y.negative_; + auto [yn, ym, ye] = y.toInternal(); int ys = yn ? -1 : 1; - internalrep ym = y.mantissa_; - auto ye = y.exponent_; auto zm = uint128_t(xm) * uint128_t(ym); auto ze = xe + ye; @@ -718,11 +758,9 @@ Number::operator*=(Number const& y) minMantissa, maxMantissa, "Number::multiplication overflow : exponent is " + std::to_string(xe)); - negative_ = zn; - mantissa_ = xm; - exponent_ = xe; - normalize(); + normalize(zn, xm, xe, minMantissa, maxMantissa); + fromInternal(zn, xm, xe); return *this; } @@ -741,15 +779,11 @@ Number::operator/=(Number const& y) // *m = mantissa // *e = exponent - bool np = negative_; + auto [np, nm, ne] = toInternal(); int ns = (np ? -1 : 1); - auto nm = mantissa_; - auto ne = exponent_; - bool dp = y.negative_; + auto [dp, dm, de] = y.toInternal(); int ds = (dp ? -1 : 1); - auto dm = y.mantissa_; - auto de = y.exponent_; auto const& range = range_.get(); auto const& minMantissa = range.min; @@ -815,9 +849,7 @@ Number::operator/=(Number const& y) } } normalize(zn, zm, ze, minMantissa, maxMantissa); - negative_ = zn; - mantissa_ = static_cast(zm); - exponent_ = ze; + fromInternal(zn, zm, ze); XRPL_ASSERT_PARTS( isnormal(), "xrpl::Number::operator/=", "result is normalized"); @@ -831,7 +863,7 @@ Number::operator rep() const Guard g; if (drops != 0) { - if (negative_) + if (drops < 0) { g.set_negative(); drops = -drops; @@ -843,7 +875,7 @@ Number::operator rep() const } for (; offset > 0; --offset) { - if (drops > largeRange.max / 10) + if (drops >= largeRange.min) throw std::overflow_error("Number::operator rep() overflow"); drops *= 10; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index db169fc1f9..bf26917800 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1082,7 +1082,6 @@ public: }; */ - auto const maxMantissa = Number::maxMantissa(); auto const maxInternalMantissa = static_cast( static_cast(power(10, Number::mantissaLog()))) *