diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h index dfd9632a..55b295cf 100644 --- a/src/core/include/units/magnitude.h +++ b/src/core/include/units/magnitude.h @@ -139,6 +139,14 @@ constexpr T int_power(T base, std::integral auto exp){ // "parameter-compatible static_asserts", and should not result in exceptions at runtime. if (exp < 0) { throw std::invalid_argument{"int_power only supports positive integer powers"}; } + constexpr auto checked_multiply = [] (auto a, auto b) { + const auto result = a * b; + if (result / a != b) { throw std::overflow_error{"Wraparound detected"}; } + return result; + }; + + constexpr auto checked_square = [checked_multiply] (auto a) { return checked_multiply(a, a); }; + // TODO(chogg): Unify this implementation with the one in pow.h. That one takes its exponent as a // template parameter, rather than a function parameter. @@ -147,17 +155,10 @@ constexpr T int_power(T base, std::integral auto exp){ } if (exp % 2 == 1) { - return base * int_power(base, exp - 1); + return checked_multiply(base, int_power(base, exp - 1)); } - const auto square_root = int_power(base, exp / 2); - const auto result = square_root * square_root; - - if constexpr(std::is_unsigned_v) { - if (result / square_root != square_root) { throw std::overflow_error{"Unsigned wraparound"}; } - } - - return result; + return checked_square(int_power(base, exp / 2)); } @@ -331,14 +332,6 @@ struct magnitude { // Whether this magnitude represents a rational number. friend constexpr bool is_rational(const magnitude&) { return (detail::is_rational(BPs) && ...); } - - // The value of this magnitude, expressed in a given type. - template - requires ( - std::is_floating_point_v - || (std::is_integral_v && (detail::is_integral(BPs) && ...))) - static constexpr T value = detail::checked_static_cast( - (detail::compute_base_power(BPs) * ...)); }; // Implementation for Magnitude concept (below). @@ -356,12 +349,16 @@ template concept Magnitude = detail::is_magnitude; /** - * @brief Free-function access to the value of a Magnitude in a desired type. - * - * Can avoid the need for an unsightly `.template` keyword. + * @brief The value of a Magnitude in a desired type T. */ -template -constexpr T get_value(Magnitude auto m) { return decltype(m)::template value; } +template + requires (std::is_floating_point_v || (std::is_integral_v && is_integral(magnitude{}))) +constexpr T get_value(const magnitude &) { + // Force the expression to be evaluated in a constexpr context, to catch, e.g., overflow. + constexpr auto result = detail::checked_static_cast((detail::compute_base_power(BPs) * ...)); + + return result; +} /** * @brief A base to represent pi. diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp index 42449f95..76dcb16c 100644 --- a/test/unit_test/runtime/magnitude_test.cpp +++ b/test/unit_test/runtime/magnitude_test.cpp @@ -170,7 +170,7 @@ TEST_CASE("magnitude converts to numerical value") constexpr auto via_float = cube(std::numbers::pi_v); constexpr auto via_long_double = static_cast(cube(std::numbers::pi_v)); - constexpr auto pi_cubed_value = pi_cubed.value; + constexpr auto pi_cubed_value = get_value(pi_cubed); REQUIRE(pi_cubed_value != via_float); CHECK(pi_cubed_value == via_long_double); } @@ -181,22 +181,22 @@ TEST_CASE("magnitude converts to numerical value") // Naturally, we cannot actually write a test to verify a compiler error. But any of these can // be uncommented if desired to verify that it breaks the build. - // (void)as_magnitude<412>().value; + // get_value(as_magnitude<412>()); // Would work for pow<62>: - // (void)pow<63>(as_magnitude<2>()).value; + // get_value(pow<63>(as_magnitude<2>())); // Would work for pow<63>: - // (void)pow<64>(as_magnitude<2>()).value; + // get_value(pow<64>(as_magnitude<2>())); - (void)pow<308>(as_magnitude<10>()).value; // Compiles, correctly. - // (void)pow<309>(as_magnitude<10>()).value; - // (void)pow<3099>(as_magnitude<10>()).value; - // (void)pow<3099999>(as_magnitude<10>()).value; + get_value(pow<308>(as_magnitude<10>())); // Compiles, correctly. + // get_value(pow<309>(as_magnitude<10>())); + // get_value(pow<3099>(as_magnitude<10>())); + // get_value(pow<3099999>(as_magnitude<10>())); auto sqrt_2 = pow(as_magnitude<2>()); CHECK(!is_integral(sqrt_2)); - // (void)sqrt_2.value; + // get_value(sqrt_2); } }