From 8c43a0db93fa21949de4a11c106547590ce042ac Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 1 Dec 2025 18:36:18 -0500 Subject: [PATCH] Clean up some loose ends - Get rid of commented and unused code. - Add some asserts and static asserts. - Use the range_ when possible. --- include/xrpl/basics/Number.h | 22 ++++----------- src/libxrpl/basics/Number.cpp | 53 ++++++++++++++--------------------- 2 files changed, 26 insertions(+), 49 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 77f545b983..8392fc973b 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -3,10 +3,6 @@ #include -#ifdef _MSC_VER -#include -#endif - #include #include #include @@ -42,14 +38,6 @@ isPowerOfTen(T value) return logTen(value).has_value(); } -// #ifdef _MSC_VER -// using numberuint = boost::multiprecision::uint128_t; -// using numberint = boost::multiprecision::int128_t; -// #else // !defined(_MSC_VER) -// using numberuint = __uint128_t; -// using numberint = __int128_t; -// #endif // !defined(_MSC_VER) - struct MantissaRange { using rep = std::uint64_t; @@ -71,9 +59,6 @@ struct MantissaRange class Number { - // using uint128_t = numberuint; - // using int128_t = numberint; - using rep = std::int64_t; using internalrep = MantissaRange::rep; @@ -172,6 +157,7 @@ public: { return !(x == y); } + friend constexpr bool operator<(Number const& x, Number const& y) noexcept { @@ -313,6 +299,7 @@ private: // The available ranges for mantissa constexpr static internalrep maxRep = std::numeric_limits::max(); + static_assert(maxRep == 9'223'372'036'854'775'807); constexpr static MantissaRange smallRange{ MantissaRange::small, @@ -320,11 +307,12 @@ private: static_assert(isPowerOfTen(smallRange.min)); static_assert(smallRange.max == 9'999'999'999'999'999LL); static_assert(smallRange.log == 15); + static_assert(smallRange.min < maxRep); + static_assert(smallRange.max < maxRep); constexpr static MantissaRange largeRange{ MantissaRange::large, 1'000'000'000'000'000'000LL}; static_assert(isPowerOfTen(largeRange.min)); - // maxRep 9,223,372,036,854,775,807 static_assert(largeRange.max == internalrep(9'999'999'999'999'999'999ULL)); static_assert(largeRange.log == 18); static_assert(largeRange.min < maxRep); @@ -649,7 +637,7 @@ public: // This class may only end up needed in tests class NumberMantissaScaleGuard { - MantissaRange::mantissa_scale saved_; + MantissaRange::mantissa_scale const saved_; public: explicit NumberMantissaScaleGuard( diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 5da4a25d32..08e4a3ab97 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -1,5 +1,5 @@ #include -// +// Keep Number.h first to ensure it can build without hidden dependencies #include #include @@ -21,17 +21,6 @@ using uint128_t = boost::multiprecision::uint128_t; #else // !defined(_MSC_VER) using uint128_t = __uint128_t; #endif // !defined(_MSC_VER) -// static_assert(std::is_same_v); -// -// namespace std { -// -// template <> -// struct make_unsigned -//{ -// using type = uint128_t; -// }; -// -// } // namespace std namespace ripple { @@ -228,6 +217,7 @@ Number::one() } // Use the member names in this static function for now so the diff is cleaner +// TODO: Rename the function parameters to get rid of the "_" suffix template void Number::normalize( @@ -273,12 +263,12 @@ Number::normalize( // the final mantissa_ is going to end up larger to fit within the range. // Cut it down here so that the rounding will be done while it's smaller. // - // Example: 9,900,000,000,000,555,555 > 9,223,372,036,854,775,808, - // so "m" will be modified to 990,000,000,000,055,555. Then that value - // will be rounded to 990,000,000,000,055,555 or - // 990,000,000,000,055,556, depending on the rounding mode. Finally, + // Example: 9,900,000,000,000,123,456 > 9,223,372,036,854,775,808, + // so "m" will be modified to 990,000,000,000,012,345. Then that value + // will be rounded to 990,000,000,000,012,345 or + // 990,000,000,000,012,346, depending on the rounding mode. Finally, // mantissa_ will be m*10 so it fits within the range, and end up as - // 9,900,000,000,000,555,550 or 9,900,000,000,000,555,560. + // 9,900,000,000,000,123,450 or 9,900,000,000,000,123,460. // mantissa() will return mantissa_ / 10, and exponent() will return // exponent_ + 1. if (m > maxRep) @@ -324,12 +314,8 @@ Number::normalize( void Number::normalize() { - normalize( - negative_, - mantissa_, - exponent_, - Number::minMantissa(), - Number::maxMantissa()); + auto const& range = range_.get(); + normalize(negative_, mantissa_, exponent_, range.min, range.max); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -413,12 +399,12 @@ Number::operator+=(Number const& y) } while (xe > ye); } - auto const minMantissa = Number::minMantissa(); + auto const& range = range_.get(); + auto const& minMantissa = range.min; + auto const& maxMantissa = range.max; if (xn == yn) { - auto const maxMantissa = Number::maxMantissa(); - xm += ym; if (xm > maxMantissa || xm > maxRep) { @@ -555,8 +541,9 @@ Number::operator*=(Number const& y) if (zn) g.set_negative(); - auto const minMantissa = Number::minMantissa(); - auto const maxMantissa = Number::maxMantissa(); + auto const& range = range_.get(); + auto const& minMantissa = range.min; + auto const& maxMantissa = range.max; while (zm > maxMantissa || zm > maxRep) { @@ -628,6 +615,10 @@ Number::operator/=(Number const& y) auto dm = y.mantissa_; auto de = y.exponent_; + auto const& range = range_.get(); + auto const& minMantissa = range.min; + auto const& maxMantissa = range.max; + // Shift by 10^17 gives greatest precision while not overflowing // uint128_t or the cast back to int64_t // TODO: Can/should this be made bigger for largeRange? @@ -640,9 +631,7 @@ Number::operator/=(Number const& y) uint128_t const f = small ? 100'000'000'000'000'000 : 10'000'000'000'000'000'000ULL; XRPL_ASSERT_PARTS( - f >= Number::minMantissa() * 10, - "Number::operator/=", - "factor expected size"); + f >= minMantissa * 10, "Number::operator/=", "factor expected size"); // unsigned denominator auto const dmu = static_cast(dm); @@ -689,7 +678,7 @@ Number::operator/=(Number const& y) ze -= 3; } } - normalize(zn, zm, ze, minMantissa(), maxMantissa()); + normalize(zn, zm, ze, minMantissa, maxMantissa); negative_ = zn; mantissa_ = static_cast(zm); exponent_ = ze;