Add most basic runtime conversion checkers #208
Conversation
Each magnitude applying type gets its own dedicated utility. This is nice because the overflow condition appears directly next to the function that applies the magnitude, so it's easy to check. For example, when applying a rational magnitude to an integral type, we check whether the _numerator alone_ would cause overflow, because we know we'd be multiplying by that numerator before dividing by the denominator. We also make a new type trait to make it easy to get the appropriate type of magnitude applicator. This made it a lot easier to write the tests.
Once again, each magnitude-applying type gets its own dedicated utility, bringing the same benefits as for the overflow checker. When it comes to truncation, most helpers get a much simpler implementation: it's simply always `false` if we're doing a pure integer multiply, or if the type itself is floating point. On the _implementation_, we do get an extra level of indirection, because the pure-integer-divide application results are different depending on whether or not our type is floating point.
Every signature looks something like: ```cpp constexpr bool is_foo(Quantity q, TargetUnit target); ``` The instances of `is_foo` that we provide are specifically: - `will_conversion_overflow` - `will_conversion_truncate` - `is_conversion_lossy` The third is the disjunction of the first two. So we would use them something like this: ```cpp constexpr bool ok = is_conversion_lossy(inches(36), feet); ``` The above would return `false`, but would return `true` if we replace `inches(36)` with `inches(37)`.
chiphogg
added
the
release notes: ✨ lib (enhancement)
I forgot I had already computed `is_lossy`...
Hey, it still finishes pretty quickly!
|
It turns out, this PR's implementation isn't quite right. Due to integer promotion, the type of This only occurs for (non-integer, non-inverse-integer) rational factors applied to integral types, and only when the integral types are small enough for integer promotion to come into play. I think the best approach is to solve this in a follow-up PR at some point. Otherwise, it would probably add too much complexity to this PR. |
au/quantity_test.cc
Outdated
| TEST(IsConversionLossy, CorrectlyDiscriminatesBetweenLossyAndLosslessConversions) { | ||
| // We will check literally every representable value in the type, and make sure that the result | ||
| // of `is_conversion_lossy()` matches perfectly with the inability to recover the initial value. | ||
| auto test_round_trip_for_every_uint8_value = [](auto source_units, auto target_units) { |
There was a problem hiding this comment.
Did you mean for this to be named uint16?
There was a problem hiding this comment.
Yep, thanks! Fixed.
au/quantity_test.cc
Outdated
| // We will check literally every representable value in the type, and make sure that the result | ||
| // of `is_conversion_lossy()` matches perfectly with the inability to recover the initial value. | ||
| auto test_round_trip_for_every_uint8_value = [](auto source_units, auto target_units) { | ||
| for (int i = 0; i <= 65535; ++i) { |
There was a problem hiding this comment.
Does it make sense to do the follow?
for (int i = numeric_limits<uint16_t>::min(); i <= numeric_limits<uint16_t>::max() ; ++i) {| { | ||
| using ApplyOneBillionToF = ApplyMagnitudeT<float, decltype(ONE_BILLION)>; | ||
|
|
||
| EXPECT_TRUE(ApplyOneBillionToF::would_overflow(3.403e29f)); |
There was a problem hiding this comment.
Is there any significance to this value or was it found emperically?
There was a problem hiding this comment.
It is one one-billionth of the maximum float value, which I looked up.
(Well, slightly above one one-billionth of that value.)
To start out with, this is only for
Quantity(notQuantityPoint),and only for same-rep conversions (so no explicit-rep variants). The
plan is to start getting some experience with these, check them out on
godbolt, make some runtime-checking converters that use them, etc. We
can follow up later with a more complete set of APIs. Oh, and docs, too
--- this is starting out as an undocumented feature.
Every utility's signature looks something like:
The instances of
is_foothat we provide are specifically:will_conversion_overflowwill_conversion_truncateis_conversion_lossyThe third is the disjunction of the first two. So we would use them
something like this:
The above would return
false, but would returntrueif we replaceinches(36)withinches(37).On the implementation side, each magnitude-applying type gets its own
dedicated utility. This is nice, because then the function that checks
each condition appears directly next to the function that applies the
magnitude, so it's easy to check that they're consistent! (For example,
when applying a rational magnitude to an integral type, we check whether
the numerator alone would cause overflow, because we know we'd be
multiplying by that numerator before dividing by the denominator.)
Along the way, we also make a new type trait to make it easy to get the
appropriate type of magnitude applicator. This made it a lot easier to
write the tests.
And speaking of tests, we concentrate the vast majority of them in the
detail-namespaced helpers, which separately check for overflow andtruncation. For the
:quantity_testtests, we just pick onerepresentative test case that is just complicated enough to give us
confidence that we're correctly using the utilities.
The most exciting test case is the one for
is_conversion_lossy(). Wetake one type (
uint8_t), and check for every representable valuethat a round-trip unit coversion is the identity, if and only if we
have identified that conversion as "not lossy". We got 100% correct
results for both inches-to-feet (truncation), and feet-to-inches
(overflow). Nice!