Revert "Add optional enforcement of valid integer range to Number"

This reverts commit 3cb447a4fe.

include/xrpl/basics/Number.h

@@ -13,47 +13,16 @@ class Number;
 std::string
 to_string(Number const& amount);
 
-template <typename T>
-constexpr bool
-isPowerOfTen(T value)
-{
-    while (value >= 10 && value % 10 == 0)
-        value /= 10;
-    return value == 1;
-}
-
 class Number
 {
-public:
-    /** Describes whether and how to enforce this number as an integer.
-     *
-     * - none: No enforcement. The value may vary freely. This is the default.
-     * - weak: If the absolute value is greater than maxIntValue, valid() will
-     *   return false.
-     * - strong: Assignment operations will throw if the absolute value is above
-     *   maxIntValue.
-     */
-    enum EnforceInteger { none, weak, strong };
-
-private:
     using rep = std::int64_t;
     rep mantissa_{0};
     int exponent_{std::numeric_limits<int>::lowest()};
 
-    // The enforcement setting is not serialized, and does not affect the
-    // ledger. If not "none", the value is checked to be within the valid
-    // integer range. With "strong", the checks will be made as automatic as
-    // possible.
-    EnforceInteger enforceInteger_ = none;
-
 public:
     // The range for the mantissa when normalized
-    constexpr static rep minMantissa = 1'000'000'000'000'000LL;
+    constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL;
-    static_assert(isPowerOfTen(minMantissa));
+    constexpr static std::int64_t maxMantissa = 9'999'999'999'999'999LL;
-    constexpr static rep maxMantissa = minMantissa * 10 - 1;
-    static_assert(maxMantissa == 9'999'999'999'999'999LL);
-
-    constexpr static rep maxIntValue = minMantissa / 10;
 
     // The range for the exponent when normalized
     constexpr static int minExponent = -32768;
@@ -66,33 +35,15 @@ public:
 
     explicit constexpr Number() = default;
 
-    Number(rep mantissa, EnforceInteger enforce = none);
+    Number(rep mantissa);
-    explicit Number(rep mantissa, int exponent, EnforceInteger enforce = none);
+    explicit Number(rep mantissa, int exponent);
     explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept;
-    constexpr Number(Number const& other) = default;
-    constexpr Number(Number&& other) = default;
-
-    ~Number() = default;
-
-    constexpr Number&
-    operator=(Number const& other);
-    constexpr Number&
-    operator=(Number&& other);
 
     constexpr rep
     mantissa() const noexcept;
     constexpr int
     exponent() const noexcept;
 
-    void
-    setIntegerEnforcement(EnforceInteger enforce);
-
-    EnforceInteger
-    integerEnforcement() const noexcept;
-
-    bool
-    valid() const noexcept;
-
     constexpr Number
     operator+() const noexcept;
     constexpr Number
@@ -218,9 +169,6 @@ public:
 private:
     static thread_local rounding_mode mode_;
 
-    void
-    checkInteger(char const* what) const;
-
     void
     normalize();
     constexpr bool
@@ -236,52 +184,16 @@ inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept
 {
 }
 
-inline Number::Number(rep mantissa, int exponent, EnforceInteger enforce)
+inline Number::Number(rep mantissa, int exponent)
-    : mantissa_{mantissa}, exponent_{exponent}, enforceInteger_(enforce)
+    : mantissa_{mantissa}, exponent_{exponent}
 {
     normalize();
-
-    checkInteger("Number::Number integer overflow");
 }
 
-inline Number::Number(rep mantissa, EnforceInteger enforce)
+inline Number::Number(rep mantissa) : Number{mantissa, 0}
-    : Number{mantissa, 0, enforce}
 {
 }
 
-constexpr Number&
-Number::operator=(Number const& other)
-{
-    if (this != &other)
-    {
-        mantissa_ = other.mantissa_;
-        exponent_ = other.exponent_;
-        enforceInteger_ = std::max(enforceInteger_, other.enforceInteger_);
-
-        checkInteger("Number::operator= integer overflow");
-    }
-
-    return *this;
-}
-
-constexpr Number&
-Number::operator=(Number&& other)
-{
-    if (this != &other)
-    {
-        // std::move doesn't really do anything for these types, but
-        // this is future-proof in case the types ever change
-        mantissa_ = std::move(other.mantissa_);
-        exponent_ = std::move(other.exponent_);
-        if (other.enforceInteger_ > enforceInteger_)
-            enforceInteger_ = std::move(other.enforceInteger_);
-
-        checkInteger("Number::operator= integer overflow");
-    }
-
-    return *this;
-}
-
 inline constexpr Number::rep
 Number::mantissa() const noexcept
 {
@@ -294,20 +206,6 @@ Number::exponent() const noexcept
     return exponent_;
 }
 
-inline void
-Number::setIntegerEnforcement(EnforceInteger enforce)
-{
-    enforceInteger_ = enforce;
-
-    checkInteger("Number::setIntegerEnforcement integer overflow");
-}
-
-inline Number::EnforceInteger
-Number::integerEnforcement() const noexcept
-{
-    return enforceInteger_;
-}
-
 inline constexpr Number
 Number::operator+() const noexcept
 {

src/libxrpl/basics/Number.cpp

@@ -222,13 +222,6 @@ Number::Guard::doRound(rep& drops)
 
 constexpr Number one{1000000000000000, -15, Number::unchecked{}};
 
-void
-Number::checkInteger(char const* what) const
-{
-    if (enforceInteger_ == strong && !valid())
-        throw std::overflow_error(what);
-}
-
 void
 Number::normalize()
 {
@@ -270,27 +263,9 @@ Number::normalize()
         mantissa_ = -mantissa_;
 }
 
-bool
-Number::valid() const noexcept
-{
-    if (enforceInteger_ != none)
-    {
-        static Number const max = maxIntValue;
-        static Number const maxNeg = -maxIntValue;
-        // Avoid making a copy
-        if (mantissa_ < 0)
-            return *this >= maxNeg;
-        return *this <= max;
-    }
-    return true;
-}
-
 Number&
 Number::operator+=(Number const& y)
 {
-    // The strictest setting prevails
-    enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_);
-
     if (y == Number{})
         return *this;
     if (*this == Number{})
@@ -378,9 +353,6 @@ Number::operator+=(Number const& y)
     }
     mantissa_ = xm * xn;
     exponent_ = xe;
-
-    checkInteger("Number::addition integer overflow");
-
     return *this;
 }
 
@@ -415,9 +387,6 @@ divu10(uint128_t& u)
 Number&
 Number::operator*=(Number const& y)
 {
-    // The strictest setting prevails
-    enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_);
-
     if (*this == Number{})
         return *this;
     if (y == Number{})
@@ -469,18 +438,12 @@ Number::operator*=(Number const& y)
     XRPL_ASSERT(
         isnormal() || *this == Number{},
         "ripple::Number::operator*=(Number) : result is normal");
-
-    checkInteger("Number::multiplication integer overflow");
-
     return *this;
 }
 
 Number&
 Number::operator/=(Number const& y)
 {
-    // The strictest setting prevails
-    enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_);
-
     if (y == Number{})
         throw std::overflow_error("Number: divide by 0");
     if (*this == Number{})
@@ -508,9 +471,6 @@ Number::operator/=(Number const& y)
     exponent_ = ne - de - 17;
     mantissa_ *= np * dp;
     normalize();
-
-    checkInteger("Number::division integer overflow");
-
     return *this;
 }
 

src/test/basics/Number_test.cpp

@@ -2,7 +2,6 @@
 #include <xrpl/beast/unit_test.h>
 #include <xrpl/protocol/IOUAmount.h>
 #include <xrpl/protocol/STAmount.h>
-#include <xrpl/protocol/SystemParameters.h>
 
 #include <sstream>
 #include <tuple>
@@ -835,173 +834,6 @@ public:
         }
     }
 
-    void
-    testInteger()
-    {
-        testcase("Integer enforcement");
-
-        using namespace std::string_literals;
-
-        {
-            Number a{100};
-            BEAST_EXPECT(a.integerEnforcement() == Number::none);
-            BEAST_EXPECT(a.valid());
-            a = Number{1, 30};
-            BEAST_EXPECT(a.valid());
-            a = -100;
-            BEAST_EXPECT(a.valid());
-        }
-        {
-            Number a{100, Number::weak};
-            BEAST_EXPECT(a.integerEnforcement() == Number::weak);
-            BEAST_EXPECT(a.valid());
-            a = Number{1, 30, Number::none};
-            BEAST_EXPECT(!a.valid());
-            a = -100;
-            BEAST_EXPECT(a.integerEnforcement() == Number::weak);
-            BEAST_EXPECT(a.valid());
-            a = Number{5, Number::strong};
-            BEAST_EXPECT(a.integerEnforcement() == Number::strong);
-            BEAST_EXPECT(a.valid());
-        }
-        {
-            Number a{100, Number::strong};
-            BEAST_EXPECT(a.integerEnforcement() == Number::strong);
-            BEAST_EXPECT(a.valid());
-            try
-            {
-                a = Number{1, 30};
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
-                // The throw is done _after_ the number is updated.
-                BEAST_EXPECT((a == Number{1, 30}));
-            }
-            BEAST_EXPECT(!a.valid());
-            a = -100;
-            BEAST_EXPECT(a.integerEnforcement() == Number::strong);
-            BEAST_EXPECT(a.valid());
-        }
-        {
-            Number a{INITIAL_XRP.drops(), Number::weak};
-            BEAST_EXPECT(!a.valid());
-            a = -a;
-            BEAST_EXPECT(!a.valid());
-
-            try
-            {
-                a.setIntegerEnforcement(Number::strong);
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(
-                    e.what() ==
-                    "Number::setIntegerEnforcement integer overflow"s);
-                // The throw is internal to the operator before the result is
-                // assigned to the Number
-                BEAST_EXPECT(a == -INITIAL_XRP);
-                BEAST_EXPECT(!a.valid());
-            }
-            try
-            {
-                ++a;
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::addition integer overflow"s);
-                // The throw is internal to the operator before the result is
-                // assigned to the Number
-                BEAST_EXPECT(a == -INITIAL_XRP);
-                BEAST_EXPECT(!a.valid());
-            }
-            a = Number::maxIntValue;
-            try
-            {
-                ++a;
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::addition integer overflow"s);
-                // This time, the throw is done _after_ the number is updated.
-                BEAST_EXPECT(a == Number::maxIntValue + 1);
-                BEAST_EXPECT(!a.valid());
-            }
-            a = -Number::maxIntValue;
-            try
-            {
-                --a;
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::addition integer overflow"s);
-                // This time, the throw is done _after_ the number is updated.
-                BEAST_EXPECT(a == -Number::maxIntValue - 1);
-                BEAST_EXPECT(!a.valid());
-            }
-            a = Number(1, 10);
-            try
-            {
-                a *= Number(1, 10);
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(
-                    e.what() == "Number::multiplication integer overflow"s);
-                // The throw is done _after_ the number is updated.
-                BEAST_EXPECT((a == Number{1, 20}));
-                BEAST_EXPECT(!a.valid());
-            }
-            try
-            {
-                a = Number::maxIntValue * 2;
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
-                // The throw is done _after_ the number is updated.
-                BEAST_EXPECT((a == Number{2, 14}));
-                BEAST_EXPECT(!a.valid());
-            }
-            try
-            {
-                a = Number(3, 15, Number::strong);
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::Number integer overflow"s);
-                // The Number doesn't get updated because the ctor throws
-                BEAST_EXPECT((a == Number{2, 14}));
-                BEAST_EXPECT(!a.valid());
-            }
-            a = Number(1, 10);
-            try
-            {
-                a /= Number(1, -10);
-                BEAST_EXPECT(false);
-            }
-            catch (std::overflow_error const& e)
-            {
-                BEAST_EXPECT(e.what() == "Number::division integer overflow"s);
-                // The throw is done _after_ the number is updated.
-                BEAST_EXPECT((a == Number{1, 20}));
-                BEAST_EXPECT(!a.valid());
-            }
-            a /= Number(1, 15);
-            BEAST_EXPECT((a == Number{1, 5}));
-            BEAST_EXPECT(a.valid());
-        }
-    }
-
-    void
     run() override
     {
         testZero();
@@ -1023,7 +855,6 @@ public:
         test_toSTAmount();
         test_truncate();
         testRounding();
-        testInteger();
     }
 };