diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h index 77de58b2..a73796f0 100644 --- a/src/core/include/units/magnitude.h +++ b/src/core/include/units/magnitude.h @@ -34,7 +34,7 @@ namespace units { * * We have two categories. * - * The first is just an `int`. This is for prime number bases. These can always be used directly as NTTPs. + * The first is just an `std::intmax_t`. This is for prime number bases. These can always be used directly as NTTPs. * * The second category is a _custom type_, which has a static member variable of type `long double` that holds its * value. We choose `long double` to get the greatest degree of precision; users who need a different type can convert @@ -45,13 +45,13 @@ namespace units { * GCC 10) which don't yet permit floating point NTTPs. */ template -concept BaseRep = std::is_same_v || std::is_same_v, long double>; +concept BaseRep = std::is_same_v || std::is_same_v, long double>; /** * @brief A basis vector in our magnitude representation, raised to some rational power. * * The public API is that there is a `power` member variable (of type `ratio`), and a `get_base()` member function (of - * type either `int` or `long double`, as appropriate), for any specialization. + * type either `std::intmax_t` or `long double`, as appropriate), for any specialization. * * These types exist to be used as NTTPs for the variadic `magnitude<...>` template. We represent a magnitude (which is * a positive real number) as the product of rational powers of "basis vectors", where each "basis vector" is a positive @@ -64,7 +64,7 @@ concept BaseRep = std::is_same_v || std::is_same_v -struct base_power { +struct base_power { // The value of the basis "vector". Must be prime to be used with `magnitude` (below). - int base; + std::intmax_t base; // The rational power to which the base is raised. ratio power{1}; - constexpr int get_base() const { return base; } + constexpr std::intmax_t get_base() const { return base; } }; /** * @brief Deduction guides for base_power: only permit deducing integral bases. */ template U> -base_power(T, U) -> base_power; +base_power(T, U) -> base_power; template -base_power(T) -> base_power; +base_power(T) -> base_power; // Implementation for BasePower concept (below). namespace detail { @@ -266,7 +266,7 @@ constexpr bool is_prime(std::intmax_t n) { return (n > 1) && (find_first_factor( constexpr bool is_valid_base_power(const BasePower auto &bp) { if (bp.power == 0) { return false; } - if constexpr (std::is_same_v) { return is_prime(bp.get_base()); } + if constexpr (std::is_same_v) { return is_prime(bp.get_base()); } else { return bp.get_base() > 0; } } @@ -442,7 +442,7 @@ namespace detail { template requires (N > 0) struct prime_factorization { - static constexpr int first_base = find_first_factor(N); + static constexpr std::intmax_t first_base = find_first_factor(N); static constexpr std::intmax_t first_power = multiplicity(first_base, N); static constexpr std::intmax_t remainder = remove_power(first_base, first_power, N); diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp index 76dcb16c..3def404b 100644 --- a/test/unit_test/runtime/magnitude_test.cpp +++ b/test/unit_test/runtime/magnitude_test.cpp @@ -47,9 +47,9 @@ TEST_CASE("base_power") { SECTION("base rep deducible for integral base") { - CHECK(base_power{2} == base_power{2, ratio{1}}); - CHECK(base_power{2, 3} == base_power{2, ratio{3}}); - CHECK(base_power{2, ratio{3, 4}} == base_power{2, ratio{3, 4}}); + CHECK(base_power{2} == base_power{2, ratio{1}}); + CHECK(base_power{2, 3} == base_power{2, ratio{3}}); + CHECK(base_power{2, ratio{3, 4}} == base_power{2, ratio{3, 4}}); } SECTION("get_base retrieves base for integral base") @@ -133,6 +133,13 @@ TEST_CASE("make_ratio performs prime factorization correctly") REQUIRE(r.exp == 2); CHECK(as_magnitude() == as_magnitude<300>()); } + + SECTION("Can handle prime factor which would be large enough to overflow int") + { + // This was taken from a case which failed when we used `int` for our base to store prime numbers. + // The failure was due to a prime factor which is larger than 2^31. + as_magnitude(); + } } TEST_CASE("magnitude converts to numerical value")