Skip to content

Commit

Permalink
Unify in/as implementations using unit slots (aurora-opensource#195)
Browse files Browse the repository at this point in the history
With a little more care in our definitions, we no longer need separate
overloads for `QuantityMaker`!  We can just use a unified definition.

Someday, this will also let us replace SFINAE with more readable
`static_assert`, which is good on the assumption that nobody will ever
call `in` or `as` with something that is not compatible with a unit
slot.  Unfortunately, this is blocked on aurora-opensource#139, because sometimes
function calls can get resolved to the deprecated legacy APIs... and
this manifests in really weird ways, like the compiler thinking we want
"units" of `int` or `double`.

This paves the way for aurora-opensource#43, because `SymbolFor` will also be compatible
with unit slots.  So, not only are we removing one set of overloads;
we're also avoiding the need to add _another_ set in the very near
future.

Test plan:

- [x] All Au tests pass
- [x] Make sure this would enable upcoming `.in(m)` test to pass
- [x] Test AV repo
- [x] Measure compile time impact
  • Loading branch information
chiphogg authored Nov 20, 2023
1 parent 07cd168 commit 12e45b8
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 29 deletions.
6 changes: 4 additions & 2 deletions au/conversion_policy.hh
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,10 @@ struct PermitAsCarveOutForIntegerPromotion
template <typename Rep, typename ScaleFactor>
struct ImplicitRepPermitted : detail::CoreImplicitConversionPolicy<Rep, ScaleFactor, Rep> {};

template <typename Rep, typename SourceUnit, typename TargetUnit>
constexpr bool implicit_rep_permitted_from_source_to_target(SourceUnit, TargetUnit) {
template <typename Rep, typename SourceUnitSlot, typename TargetUnitSlot>
constexpr bool implicit_rep_permitted_from_source_to_target(SourceUnitSlot, TargetUnitSlot) {
using SourceUnit = AssociatedUnitT<SourceUnitSlot>;
using TargetUnit = AssociatedUnitT<TargetUnitSlot>;
static_assert(HasSameDimension<SourceUnit, TargetUnit>::value,
"Can only convert same-dimension units");

Expand Down
35 changes: 8 additions & 27 deletions au/quantity.hh
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,17 @@ class Quantity {

template <typename NewRep,
typename NewUnit,
typename = std::enable_if_t<IsUnit<NewUnit>::value>>
typename = std::enable_if_t<IsUnit<AssociatedUnitT<NewUnit>>::value>>
constexpr auto as(NewUnit) const {
using Common = std::common_type_t<Rep, NewRep>;
using Factor = UnitRatioT<Unit, NewUnit>;
using Factor = UnitRatioT<AssociatedUnitT<Unit>, AssociatedUnitT<NewUnit>>;

return make_quantity<NewUnit>(
return make_quantity<AssociatedUnitT<NewUnit>>(
static_cast<NewRep>(detail::apply_magnitude(static_cast<Common>(value_), Factor{})));
}

template <typename NewUnit, typename = std::enable_if_t<IsUnit<NewUnit>::value>>
template <typename NewUnit,
typename = std::enable_if_t<IsUnit<AssociatedUnitT<NewUnit>>::value>>
constexpr auto as(NewUnit u) const {
constexpr bool IMPLICIT_OK =
implicit_rep_permitted_from_source_to_target<Rep>(unit, NewUnit{});
Expand All @@ -169,7 +170,7 @@ class Quantity {

template <typename NewRep,
typename NewUnit,
typename = std::enable_if_t<IsUnit<NewUnit>::value>>
typename = std::enable_if_t<IsUnit<AssociatedUnitT<NewUnit>>::value>>
constexpr NewRep in(NewUnit u) const {
if (are_units_quantity_equivalent(unit, u) && std::is_same<Rep, NewRep>::value) {
return static_cast<NewRep>(value_);
Expand All @@ -178,7 +179,8 @@ class Quantity {
}
}

template <typename NewUnit, typename = std::enable_if_t<IsUnit<NewUnit>::value>>
template <typename NewUnit,
typename = std::enable_if_t<IsUnit<AssociatedUnitT<NewUnit>>::value>>
constexpr Rep in(NewUnit u) const {
if (are_units_quantity_equivalent(unit, u)) {
return value_;
Expand All @@ -188,27 +190,6 @@ class Quantity {
}
}

// Overloads for passing a QuantityMaker.
//
// This is the "magic" that lets us write things like `distance.in(meters)`, instead of just
// `distance.in(Meters{})`.
template <typename NewRep, typename NewUnit>
constexpr auto as(QuantityMaker<NewUnit>) const {
return as<NewRep>(NewUnit{});
}
template <typename NewUnit>
constexpr auto as(QuantityMaker<NewUnit>) const {
return as(NewUnit{});
}
template <typename NewRep, typename NewUnit>
constexpr NewRep in(QuantityMaker<NewUnit>) const {
return in<NewRep>(NewUnit{});
}
template <typename NewUnit>
constexpr Rep in(QuantityMaker<NewUnit>) const {
return in(NewUnit{});
}

// "Old-style" overloads with <U, R> template parameters, and no function parameters.
//
// Matches the syntax from the CppCon 2021 talk, and legacy Aurora usage.
Expand Down

0 comments on commit 12e45b8

Please sign in to comment.