Skip to content

Commit

Permalink
Check that every magnitude fits in its target type (#211)
Browse files Browse the repository at this point in the history
`get_value<T>(m)` converts a magnitude `m` to its representation in type
`T`.  It produces a hard compiler error if the value can't fit.  Its
relative `get_value_result<T>(m)` always returns a result, but produces
an error code if the value can't be represented.

Previously, there was a hole in this system.  We assumed that each
magnitude could be represented in `std::uintmax_t`.  Of course, if it
couldn't, `get_value` would fail to compile as it should.  But
`get_value_result` would _also_ fail to compile, instead of producing an
error condition.

To fix this, we need to move our checking utilities further up in the
file, so that all of our utilities can use them.  `product`, `int_pow`,
`base_power_value` --- they all need to _check their computations_ at
every step.  When we do this, we're able to gracefully detect the
non-representability of huge numbers even for the biggest types.
  • Loading branch information
chiphogg authored Dec 15, 2023
1 parent 1e84a95 commit a831130
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 23 deletions.
89 changes: 66 additions & 23 deletions au/magnitude.hh
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,21 @@ struct NumeratorImpl<Magnitude<BPs...>>

namespace detail {

enum class MagRepresentationOutcome {
OK,
ERR_NON_INTEGER_IN_INTEGER_TYPE,
ERR_RATIONAL_POWERS,
ERR_CANNOT_FIT,
};

template <typename T>
struct MagRepresentationOrError {
MagRepresentationOutcome outcome;

// Only valid/meaningful if `outcome` is `OK`.
T value = {0};
};

// The widest arithmetic type in the same category.
//
// Used for intermediate computations.
Expand All @@ -278,19 +293,61 @@ using Widen = std::conditional_t<
std::conditional_t<std::is_signed<T>::value, std::intmax_t, std::uintmax_t>>,
T>;

template <typename T>
constexpr MagRepresentationOrError<T> checked_int_pow(T base, std::uintmax_t exp) {
MagRepresentationOrError<T> result = {MagRepresentationOutcome::OK, T{1}};
while (exp > 0u) {
if (exp % 2u == 1u) {
if (base > std::numeric_limits<T>::max() / result.value) {
return MagRepresentationOrError<T>{MagRepresentationOutcome::ERR_CANNOT_FIT};
}
result.value *= base;
}

exp /= 2u;

if (base > std::numeric_limits<T>::max() / base) {
return (exp == 0u)
? result
: MagRepresentationOrError<T>{MagRepresentationOutcome::ERR_CANNOT_FIT};
}
base *= base;
}
return result;
}

template <typename T, std::intmax_t N, typename B>
constexpr Widen<T> base_power_value(B base) {
return (N < 0) ? (Widen<T>{1} / base_power_value<T, -N>(base))
: int_pow(static_cast<Widen<T>>(base), static_cast<std::uintmax_t>(N));
constexpr MagRepresentationOrError<Widen<T>> base_power_value(B base) {
if (N < 0) {
const auto inverse_result = base_power_value<T, -N>(base);
if (inverse_result.outcome != MagRepresentationOutcome::OK) {
return inverse_result;
}
return {
MagRepresentationOutcome::OK,
Widen<T>{1} / inverse_result.value,
};
}

return checked_int_pow(static_cast<Widen<T>>(base), static_cast<std::uintmax_t>(N));
}

template <typename T, std::size_t N>
constexpr T product(const T (&values)[N]) {
constexpr MagRepresentationOrError<T> product(const MagRepresentationOrError<T> (&values)[N]) {
for (const auto &x : values) {
if (x.outcome != MagRepresentationOutcome::OK) {
return x;
}
}

T result{1};
for (const auto &x : values) {
result *= x;
if ((x.value > 1) && (result > std::numeric_limits<T>::max() / x.value)) {
return {MagRepresentationOutcome::ERR_CANNOT_FIT};
}
result *= x.value;
}
return result;
return {MagRepresentationOutcome::OK, result};
}

template <std::size_t N>
Expand Down Expand Up @@ -327,21 +384,6 @@ constexpr bool safe_to_cast_to(InputT x) {
return SafeCastingChecker<T>{}(x);
}

enum class MagRepresentationOutcome {
OK,
ERR_NON_INTEGER_IN_INTEGER_TYPE,
ERR_RATIONAL_POWERS,
ERR_CANNOT_FIT,
};

template <typename T>
struct MagRepresentationOrError {
MagRepresentationOutcome outcome;

// Only valid/meaningful if `outcome` is `OK`.
T value = {0};
};

template <typename T, typename... BPs>
constexpr MagRepresentationOrError<T> get_value_result(Magnitude<BPs...>) {
// Representing non-integer values in integral types is something we never plan to support.
Expand All @@ -361,11 +403,12 @@ constexpr MagRepresentationOrError<T> get_value_result(Magnitude<BPs...>) {
constexpr auto widened_result =
product({base_power_value<T, (ExpT<BPs>::num / ExpT<BPs>::den)>(BaseT<BPs>::value())...});

if (!safe_to_cast_to<T>(widened_result)) {
if ((widened_result.outcome != MagRepresentationOutcome::OK) ||
!safe_to_cast_to<T>(widened_result.value)) {
return {MagRepresentationOutcome::ERR_CANNOT_FIT};
}

return {MagRepresentationOutcome::OK, static_cast<T>(widened_result)};
return {MagRepresentationOutcome::OK, static_cast<T>(widened_result.value)};
}

// This simple overload avoids edge cases with creating and passing zero-sized arrays.
Expand Down
37 changes: 37 additions & 0 deletions au/magnitude_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "au/testing.hh"
#include "gtest/gtest.h"

using ::testing::DoubleEq;
using ::testing::StaticAssertTypeEq;

namespace au {
Expand Down Expand Up @@ -265,6 +266,42 @@ TEST(CommonMagnitude, ZeroResultIndicatesAllInputsAreZero) {

namespace detail {

MATCHER(CannotFit, "") {
return (arg.outcome == MagRepresentationOutcome::ERR_CANNOT_FIT) && (arg.value == 0);
}

template <typename T, typename ValueMatcher>
auto FitsAndMatchesValue(ValueMatcher &&matcher) {
return ::testing::AllOf(
::testing::Field(&MagRepresentationOrError<T>::outcome,
::testing::Eq(MagRepresentationOutcome::OK)),
::testing::Field(&MagRepresentationOrError<T>::value, std::forward<ValueMatcher>(matcher)));
}

template <typename T>
auto FitsAndProducesValue(T val) {
return FitsAndMatchesValue<T>(SameTypeAndValue(val));
}

TEST(CheckedIntPow, FindsAppropriateLimits) {
EXPECT_THAT(checked_int_pow(int16_t{2}, 14), FitsAndProducesValue(int16_t{16384}));
EXPECT_THAT(checked_int_pow(int16_t{2}, 15), CannotFit());

EXPECT_THAT(checked_int_pow(uint16_t{2}, 15), FitsAndProducesValue(uint16_t{32768}));
EXPECT_THAT(checked_int_pow(uint16_t{2}, 16), CannotFit());

EXPECT_THAT(checked_int_pow(uint64_t{2}, 63),
FitsAndProducesValue(uint64_t{9'223'372'036'854'775'808u}));
EXPECT_THAT(checked_int_pow(uint64_t{2}, 64), CannotFit());

EXPECT_THAT(checked_int_pow(10.0, 308), FitsAndMatchesValue<double>(DoubleEq(1e308)));
EXPECT_THAT(checked_int_pow(10.0, 309), CannotFit());
}

TEST(GetValueResult, HandlesNumbersTooBigForUintmax) {
EXPECT_THAT(get_value_result<std::uintmax_t>(pow<64>(mag<2>())), CannotFit());
}

TEST(PrimeFactorizationT, NullMagnitudeFor1) {
StaticAssertTypeEq<PrimeFactorizationT<1u>, Magnitude<>>();
}
Expand Down

0 comments on commit a831130

Please sign in to comment.