Skip to content

Commit 860ae5f

Browse files
authored
Support SFINAE in Constant representability check (#669)
Right now, we have an unconditional `UnitRatio` invocation, which produces a hard compiler error. We thought this was fine, because nobody should be invoking this check with different-dimension units. However, it somehow does get invoked in Aurora-internal code. I tried reproducing the error in the Au repo only, and couldn't do it. But in any case, we can fix it by just avoiding the hard error. The most straightforward way to fix it was to just make a new trait, `IsUnitRatioRepresentableIn<T, U1, U2>`. This doesn't seem like something I want to expose to end users, so I kept it as a library internal detail.
1 parent 19fe0e5 commit 860ae5f

4 files changed

Lines changed: 55 additions & 2 deletions

File tree

au/constant.hh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ struct Constant : detail::MakesQuantityFromNumber<Constant, Unit>,
109109
// Static function to check whether this constant can be exactly-represented in the given rep
110110
// `T` and unit `OtherUnit`.
111111
template <typename T, typename OtherUnit>
112-
static constexpr bool can_store_value_in(OtherUnit other) {
113-
return representable_in<T>(unit_ratio(Unit{}, other));
112+
static constexpr bool can_store_value_in(OtherUnit) {
113+
return detail::IsUnitRatioRepresentableIn<T, Unit, AssociatedUnit<OtherUnit>>::value;
114114
}
115115

116116
// Implicitly convert to type with an exactly corresponding quantity that passes safety checks.

au/constant_test.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,11 @@ TEST(CanStoreValueIn, ChecksRangeOfTypeForIntegers) {
437437
EXPECT_THAT(decltype(c)::can_store_value_in<int16_t>(meters / second), IsFalse());
438438
}
439439

440+
TEST(CanStoreValueIn, ReturnsFalseRatherThanHardErrorForDifferentDimensions) {
441+
constexpr bool result = Constant<Meters>::can_store_value_in<long>(Nano<Seconds>{});
442+
EXPECT_THAT(result, IsFalse());
443+
}
444+
440445
TEST(Constant, SupportsEqualityComparison) {
441446
constexpr auto one_meter = make_constant(meters);
442447
constexpr auto another_meter = make_constant(meters);

au/unit_of_measure.hh

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,15 @@ constexpr auto pow(SingularNameFor<Unit>) {
482482
template <typename U>
483483
struct UnitOrderTiebreaker;
484484

485+
// Library-internal helper to check representability of ratio between two units in a numeric type.
486+
//
487+
// This will return `false` (rather than producing a compiler error) if the units don't have the
488+
// same dimension.
489+
namespace detail {
490+
template <typename T, typename U1, typename U2>
491+
struct IsUnitRatioRepresentableIn;
492+
} // namespace detail
493+
485494
////////////////////////////////////////////////////////////////////////////////////////////////////
486495
// Implementation details below
487496
////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -1385,4 +1394,22 @@ struct InOrderFor<UnitSumPack, A, B>
13851394
detail::OrderByUnscaledUnit,
13861395
detail::OrderByMag> {};
13871396

1397+
//////////////////////////////////////////////////////////////////////////////////////////////////
1398+
// `IsUnitRatioRepresentableIn` implementation.
1399+
1400+
namespace detail {
1401+
// Helper simply delegates to `representable_in` in the usual case.
1402+
template <typename T, typename U1, typename U2, bool SameDim = HasSameDimension<U1, U2>::value>
1403+
struct IsUnitRatioRepresentableInImpl
1404+
: stdx::bool_constant<representable_in<T>(UnitRatio<U1, U2>{})> {};
1405+
1406+
// If dimensions differ, just return false.
1407+
template <typename T, typename U1, typename U2>
1408+
struct IsUnitRatioRepresentableInImpl<T, U1, U2, false> : std::false_type {};
1409+
1410+
// Delegate to the appropriate helper.
1411+
template <typename T, typename U1, typename U2>
1412+
struct IsUnitRatioRepresentableIn : IsUnitRatioRepresentableInImpl<T, U1, U2> {};
1413+
} // namespace detail
1414+
13881415
} // namespace au

au/unit_of_measure_test.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "au/testing.hh"
2222
#include "au/units/fahrenheit.hh"
2323
#include "au/units/feet.hh"
24+
#include "au/units/hertz.hh"
2425
#include "au/units/inches.hh"
2526
#include "au/units/kelvins.hh"
2627
#include "au/units/meters.hh"
@@ -1142,5 +1143,25 @@ TEST(SimplifyIfOnlyOneUnscaledUnit, IdentityForNonCommonUnit) {
11421143
decltype(Feet{} * mag<3>())>();
11431144
}
11441145

1146+
TEST(IsUnitRatioRepresentableIn, TrueForIdentityRatio) {
1147+
EXPECT_THAT((IsUnitRatioRepresentableIn<int, Meters, Meters>::value), IsTrue());
1148+
}
1149+
1150+
TEST(IsUnitRatioRepresentableIn, TrueForRepresentableRatio) {
1151+
EXPECT_THAT((IsUnitRatioRepresentableIn<int, Feet, Inches>::value), IsTrue());
1152+
}
1153+
1154+
TEST(IsUnitRatioRepresentableIn, FalseForNonRepresentableRatio) {
1155+
EXPECT_THAT((IsUnitRatioRepresentableIn<int, Inches, Feet>::value), IsFalse());
1156+
}
1157+
1158+
TEST(IsUnitRatioRepresentableIn, FalseForOverflowingRatio) {
1159+
EXPECT_THAT((IsUnitRatioRepresentableIn<int, Tera<Hertz>, Hertz>::value), IsFalse());
1160+
}
1161+
1162+
TEST(IsUnitRatioRepresentableIn, FalseRatherThanHardErrorForDifferentDimensions) {
1163+
EXPECT_THAT((IsUnitRatioRepresentableIn<int, Meters, Minutes>::value), IsFalse());
1164+
}
1165+
11451166
} // namespace detail
11461167
} // namespace au

0 commit comments

Comments
 (0)