From 308e46d0dabdb7fef09b526590d773a1bcea9a51 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 8 Jun 2026 19:00:24 -0400 Subject: [PATCH] Clean up tests --- src/libxrpl/basics/Number.cpp | 11 +- src/test/basics/Number_test.cpp | 257 +++++++++++++------------------- 2 files changed, 109 insertions(+), 159 deletions(-) diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 9fc464f487..9e61cdc76d 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -474,7 +474,7 @@ Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) // If there was any remainder, subtract 1 from the result. This is sufficient to get the // best rounding. XRPL_ASSERT( - empty() || mantissa > maxMantissa_, + r == Round::Exact || mantissa > maxMantissa_, "xrpl::Number::Guard::doRoundDown : mantissa is expected size"); if (r != Round::Exact) { @@ -759,7 +759,7 @@ Number::operator+=(Number const& y) // Bring the exponents of both values into agreement, so the mantissas are on the same scale // and can be added directly together. - // Shrink the mantissa and bring the exponent up of the value with the lower exponent. Store any + // Shrink the mantissa and raise the exponent of the value with the lower exponent. Store any // dropped digits in the Guard. if (xe < ye) { @@ -801,13 +801,13 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled) { // Grow xm/xe and pull digits out of the Guard until it's a little bit larger than // maxMantissa, so that normalize will have enough information to make an accurate // rounding decision, but stop if the Guard empties out, because no rounding will be // necessary. (Normalize will pad it back into range.) Note that if any digits were lost - // (xbit), the Guard will never be empty, so xm will get big. + // (xbit), the Guard will never be empty, so xm will get larger than upperLimit. auto const upperLimit = static_cast(minMantissa) * 1000; while (xm < upperLimit && !g.empty()) { @@ -818,7 +818,8 @@ Number::operator+=(Number const& y) } else { - // Grow xm/xe and pull digits out of the Guard until it's back in range. + // Grow xm/xe and pull digits out of the Guard until it's back in the + // minMantissa/maxMantissa range. while (xm < minMantissa && xm * 10 <= kMaxRep) { xm *= 10; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 4b929799ac..50be994b7c 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1928,172 +1928,121 @@ public: } { - testcase << "subtraction rounding " << to_string(scale); - auto const exp = Number::mantissaLog(); - Number const a{1LL, exp + 2}; - Number const b{-(Number{1, exp} + 1)}; - - if (scale == MantissaRange::MantissaScale::Small) + // SubCase is + // * offset: offset from exp + // * extraB: whether to include 1e"exp" in "b" + // * aString: expected string value for "a" + // * bString: expected string value for "b" + // There aren't too many valid combinations for test cases here. If extraB is true, + // offset can really only be 2, because any larger and the mantissa can't be represented + // without loss. Offset can't be less than 2, or there's no error. + using SubCase = std::tuple; + auto const c = std::to_array({ + {2, + true, + scale == MantissaRange::MantissaScale::Small ? "100000000000000000" + : "100000000000000000000", + scale == MantissaRange::MantissaScale::Small ? "-1000000000000001" + : "-1000000000000000001"}, + {2, + false, + scale == MantissaRange::MantissaScale::Small ? "100000000000000000" + : "100000000000000000000", + "-1"}, + {30, + false, + scale == MantissaRange::MantissaScale::Small + ? "1000000000000000000000000000000000000000000000" + : "1000000000000000000000000000000000000000000000000", + "-1"}, + }); + for (auto const& [offset, extraB, aString, bString] : c) { - BEAST_EXPECT(toBigInt(a) == BigInt{"100000000000000000"}); - BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000001"}); - } - else - { - BEAST_EXPECT(toBigInt(a) == BigInt{"100000000000000000000"}); - BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000000001"}); - } + testcase << "subtraction rounding. offset: " << offset + << ", scale: " << to_string(scale); - auto construct = [&a, &b, this](Number::RoundingMode r) { - NumberRoundModeGuard const roundGuard{r}; - auto const sum = a + b; - BigInt const stored = toBigInt(sum); - return std::make_pair(r, std::make_pair(stored, sum)); - }; + Number const a{1LL, exp + offset}; + Number const b{-((extraB ? Number{1, exp} : kNumZero) + 1)}; - auto const bigA = toBigInt(a); - auto const bigB = toBigInt(b); - BigInt const exact = bigA + bigB; + auto const bigA = toBigInt(a); + auto const bigB = toBigInt(b); - auto const sums = [&]() { - std::map> sums; - sums.emplace(construct(Number::RoundingMode::TowardsZero)); - sums.emplace(construct(Number::RoundingMode::Upward)); - sums.emplace(construct(Number::RoundingMode::Downward)); - sums.emplace(construct(Number::RoundingMode::ToNearest)); - return sums; - }(); + BEAST_EXPECT(bigA == BigInt{aString}); + BEAST_EXPECT(bigB == BigInt{bString}); - log << "\n a = " << a << " (" << fmt(bigA) << ")\n b = " << b - << " (" << fmt(bigB) << ")\n exact a + b = " << fmt(exact) << "\n"; - for (auto const& [r, sum] : sums) - { - auto const diff = sum.first - exact; - auto const rLabel = to_string(r); - log << std::string(15 - rLabel.length(), ' ') << rLabel << " = " << fmt(sum.first) - << "\n difference = " << fmt(diff) << "\n"; - } - log.flush(); + auto construct = [&a, &b, this](Number::RoundingMode r) { + NumberRoundModeGuard const roundGuard{r}; + auto const sum = a + b; + BigInt const stored = toBigInt(sum); + return std::make_pair(r, std::make_pair(stored, sum)); + }; - for (auto const& [r, sum] : sums) - { - auto const epsilon = pow10(sum.second.exponent()); - auto diff = sum.first - exact; - auto const rLabel = to_string(r); - switch (scale) + BigInt const exact = bigA + bigB; + + auto const sums = [&]() { + std::map> sums; + sums.emplace(construct(Number::RoundingMode::TowardsZero)); + sums.emplace(construct(Number::RoundingMode::Upward)); + sums.emplace(construct(Number::RoundingMode::Downward)); + sums.emplace(construct(Number::RoundingMode::ToNearest)); + return sums; + }(); + + log << "\n a = " << a << " (" << fmt(bigA) + << ")\n b = " << b << " (" << fmt(bigB) + << ")\n exact a + b = " << fmt(exact) << "\n"; + for (auto const& [r, sum] : sums) { - case MantissaRange::MantissaScale::Small: - case MantissaRange::MantissaScale::LargeLegacy: { - // Without the fix, all the results but one round up - if (r == Number::RoundingMode::Downward) - { - // Downward works because the Guard sign is negative, and Downward - // returns Up instead of Down if negative and there's a remainder, - // whereas TowardsZero always returns Down. - BEAST_EXPECTS(sum.first < exact, rLabel); - BEAST_EXPECTS(diff == -(epsilon - 1), rLabel); - } - else - { - BEAST_EXPECTS(sum.first > exact, rLabel); - BEAST_EXPECTS(diff == 1, rLabel); - } - break; - } - default: { - BEAST_EXPECT(epsilon == 100); - switch (r) - { - case Number::RoundingMode::Upward: - case Number::RoundingMode::ToNearest: - BEAST_EXPECTS(sum.first > exact, rLabel); - BEAST_EXPECTS(diff == 1, rLabel); - break; - default: + auto const diff = sum.first - exact; + auto const rLabel = to_string(r); + log << std::string(15 - rLabel.length(), ' ') << rLabel << " = " + << fmt(sum.first) << "\n difference = " << fmt(diff) << "\n"; + } + log.flush(); + + auto const expectedExponent = + offset - (scale == MantissaRange::MantissaScale::Small && extraB ? 1 : 0); + auto const epsilon = pow10(expectedExponent); + for (auto const& [r, sum] : sums) + { + auto diff = sum.first - exact; + auto const rLabel = to_string(r); + switch (scale) + { + case MantissaRange::MantissaScale::Small: + case MantissaRange::MantissaScale::LargeLegacy: { + // Without the fix, all the results but one round up + if (r == Number::RoundingMode::Downward) + { + // Downward works because the Guard sign is negative, and Downward + // returns Up instead of Down if negative and there's a remainder, + // whereas TowardsZero always returns Down. BEAST_EXPECTS(sum.first < exact, rLabel); BEAST_EXPECTS(diff == -(epsilon - 1), rLabel); + } + else + { + BEAST_EXPECTS(sum.first > exact, rLabel); + BEAST_EXPECTS(diff == 1, rLabel); + } + break; } - } - } - } - } - { - auto const offset = 30; - testcase << "subtraction rounding offset of " << offset << " " << to_string(scale); - - auto const exp = Number::mantissaLog(); - Number const a{1LL, exp + offset}; - Number const b{-1}; - - auto construct = [&a, &b, this](Number::RoundingMode r) { - NumberRoundModeGuard const roundGuard{r}; - auto const sum = a + b; - BigInt const stored = toBigInt(sum); - return std::make_pair(r, std::make_pair(stored, sum)); - }; - - auto const bigA = toBigInt(a); - auto const bigB = toBigInt(b); - BigInt const exact = bigA + bigB; - - auto const sums = [&]() { - std::map> sums; - sums.emplace(construct(Number::RoundingMode::TowardsZero)); - sums.emplace(construct(Number::RoundingMode::Upward)); - sums.emplace(construct(Number::RoundingMode::Downward)); - sums.emplace(construct(Number::RoundingMode::ToNearest)); - return sums; - }(); - - log << "\n a = " << a << " (" << fmt(bigA) << ")\n b = " << b - << " (" << fmt(bigB) << ")\n exact a + b = " << fmt(exact) << "\n"; - for (auto const& [r, sum] : sums) - { - auto const diff = sum.first - exact; - auto const rLabel = to_string(r); - log << std::string(15 - rLabel.length(), ' ') << rLabel << " = " << fmt(sum.first) - << "\n difference = " << fmt(diff) << "\n"; - } - log.flush(); - - switch (scale) - { - case MantissaRange::MantissaScale::Small: - case MantissaRange::MantissaScale::LargeLegacy: { - for (auto const& [r, sum] : sums) - { - if (r == Number::RoundingMode::Downward) - { - // Downward works because the Guard sign is negative, and Downward - // returns Up instead of Down if negative and there's a remainder, - // whereas TowardsZero always returns Down. + default: { BEAST_EXPECTS( - sums.at(Number::RoundingMode::Downward).first < exact, - to_string(r)); - } - else - { - BEAST_EXPECTS(sums.at(r).first > exact, to_string(r)); - } - } - break; - } - default: { - for (auto const& [r, sum] : sums) - { - auto const epsilon = pow10(sum.second.exponent()); - auto diff = sum.first - exact; - switch (r) - { - case Number::RoundingMode::Upward: - case Number::RoundingMode::ToNearest: - BEAST_EXPECTS(sum.first > exact, to_string(r)); - BEAST_EXPECTS(diff < epsilon, to_string(r)); - break; - default: - BEAST_EXPECTS(sum.first < exact, to_string(r)); - BEAST_EXPECTS(-diff < epsilon, to_string(r)); + sum.second.exponent() <= expectedExponent, + to_string(sum.second.exponent())); + switch (r) + { + case Number::RoundingMode::Upward: + case Number::RoundingMode::ToNearest: + BEAST_EXPECTS(sum.first > exact, rLabel); + BEAST_EXPECTS(diff == 1, rLabel); + break; + default: + BEAST_EXPECTS(sum.first < exact, rLabel); + BEAST_EXPECTS(diff == -(epsilon - 1), rLabel); + } } } }