From f2e63fdeb1fe0b143876f5e16e342283834c7abd Mon Sep 17 00:00:00 2001
From: Mateusz Pusz <mateusz.pusz@gmail.com>
Date: Wed, 9 Nov 2022 18:39:51 -1000
Subject: [PATCH] refactor: `std::common_type` usage replaced with dedicated
 functions for units, dimensions, and references

---
 src/core/include/units/dimension.h       | 29 +++++--------
 src/core/include/units/quantity.h        | 25 +++++------
 src/core/include/units/reference.h       | 27 ++++++------
 src/core/include/units/unit.h            | 17 ++------
 test/unit_test/static/dimension_test.cpp | 19 ++++-----
 test/unit_test/static/unit_test.cpp      | 54 ++++++++++++------------
 6 files changed, 74 insertions(+), 97 deletions(-)

diff --git a/src/core/include/units/dimension.h b/src/core/include/units/dimension.h
index 22f3aeac..546d3067 100644
--- a/src/core/include/units/dimension.h
+++ b/src/core/include/units/dimension.h
@@ -386,31 +386,22 @@ template<Unit auto U, Dimension auto Dim>
   requires requires { detail::get_dimension_for(U); } && (interconvertible(Dim, detail::get_dimension_for(U)))
 inline constexpr bool is_valid_unit_for_dimension<U, Dim> = true;
 
-}  // namespace detail
+template<Dimension D1, Dimension D2>
+[[nodiscard]] consteval auto common_dimension(D1 d1, D2 d2)
+  requires(interconvertible(d1, d2))
+{
+  if constexpr (std::derived_from<D1, D2>)
+    return d1;
+  else
+    return d2;
+}
 
+}  // namespace detail
 
 // TODO consider adding the support for text output of the dimensional equation
 
 }  // namespace units
 
-namespace std {
-
-/**
- * @brief Partial specialization of `std::common_type` for dimensions
- *
- * Defined only for convertible types and returns the most derived/specific dimension type of
- * the two provided.
- */
-template<units::Dimension D1, units::Dimension D2>
-  requires(units::interconvertible(D1{}, D2{}))
-struct common_type<D1, D2> {
-  using type = ::units::conditional<std::derived_from<std::remove_const_t<D1>, std::remove_const_t<D2>>,
-                                    std::remove_const_t<D1>, std::remove_const_t<D2>>;
-};
-
-}  // namespace std
-
-
 #ifdef __cpp_explicit_this_parameter
 
 #define BASE_DIMENSION(name, symbol)                      \
diff --git a/src/core/include/units/quantity.h b/src/core/include/units/quantity.h
index 1d4c65b4..ad21ef4b 100644
--- a/src/core/include/units/quantity.h
+++ b/src/core/include/units/quantity.h
@@ -474,8 +474,8 @@ template<Quantity Q1, Quantity Q2>
           quantity_value_for_<std::plus<>, typename Q1::rep, typename Q2::rep>
 [[nodiscard]] constexpr Quantity auto operator+(const Q1& lhs, const Q2& rhs)
 {
-  using ref = std::common_type_t<decltype(Q1::reference), decltype(Q2::reference)>;
-  using ret = quantity<ref{}, decltype(lhs.number() + rhs.number())>;
+  constexpr auto ref = detail::common_reference(Q1::reference, Q2::reference);
+  using ret = quantity<ref, decltype(lhs.number() + rhs.number())>;
   return ret(ret(lhs).number() + ret(rhs).number());
 }
 
@@ -484,8 +484,8 @@ template<Quantity Q1, Quantity Q2>
           quantity_value_for_<std::minus<>, typename Q1::rep, typename Q2::rep>
 [[nodiscard]] constexpr Quantity auto operator-(const Q1& lhs, const Q2& rhs)
 {
-  using ref = std::common_type_t<decltype(Q1::reference), decltype(Q2::reference)>;
-  using ret = quantity<ref{}, decltype(lhs.number() - rhs.number())>;
+  constexpr auto ref = detail::common_reference(Q1::reference, Q2::reference);
+  using ret = quantity<ref, decltype(lhs.number() - rhs.number())>;
   return ret(ret(lhs).number() - ret(rhs).number());
 }
 
@@ -521,8 +521,8 @@ template<Quantity Q1, Quantity Q2>
           std::three_way_comparable_with<typename Q1::rep, typename Q2::rep>
 [[nodiscard]] constexpr auto operator<=>(const Q1& lhs, const Q2& rhs)
 {
-  using ref = std::common_type_t<decltype(Q1::reference), decltype(Q2::reference)>;
-  return quantity_cast<ref{}>(lhs).number() <=> quantity_cast<ref{}>(rhs).number();
+  constexpr auto ref = detail::common_reference(Q1::reference, Q2::reference);
+  return quantity_cast<ref>(lhs).number() <=> quantity_cast<ref>(rhs).number();
 }
 
 template<Quantity Q1, Quantity Q2>
@@ -530,8 +530,8 @@ template<Quantity Q1, Quantity Q2>
           std::equality_comparable_with<typename Q1::rep, typename Q2::rep>
 [[nodiscard]] constexpr bool operator==(const Q1& lhs, const Q2& rhs)
 {
-  using ref = std::common_type_t<decltype(Q1::reference), decltype(Q2::reference)>;
-  return quantity_cast<ref{}>(lhs).number() == quantity_cast<ref{}>(rhs).number();
+  constexpr auto ref = detail::common_reference(Q1::reference, Q2::reference);
+  return quantity_cast<ref>(lhs).number() == quantity_cast<ref>(rhs).number();
 }
 
 }  // namespace units
@@ -540,14 +540,15 @@ namespace std {
 
 template<units::Quantity Q1, units::Quantity Q2>
   requires requires {
-             typename common_type_t<remove_const_t<decltype(Q1::reference)>, remove_const_t<decltype(Q2::reference)>>;
+             {
+               units::detail::common_reference(Q1::reference, Q2::reference)
+               } -> units::Reference;
              typename common_type_t<typename Q1::rep, typename Q2::rep>;
            }
 struct common_type<Q1, Q2> {
-private:
-  using ref = common_type_t<remove_const_t<decltype(Q1::reference)>, remove_const_t<decltype(Q2::reference)>>;
 public:
-  using type = units::quantity<ref{}, common_type_t<typename Q1::rep, typename Q2::rep>>;
+  using type = units::quantity<units::detail::common_reference(Q1::reference, Q2::reference),
+                               common_type_t<typename Q1::rep, typename Q2::rep>>;
 };
 
 }  // namespace std
diff --git a/src/core/include/units/reference.h b/src/core/include/units/reference.h
index d50c32e5..63dbdcfa 100644
--- a/src/core/include/units/reference.h
+++ b/src/core/include/units/reference.h
@@ -107,21 +107,20 @@ template<Reference R1, Reference R2>
   return interconvertible(R1::dimension, R2::dimension) && interconvertible(R1::unit, R2::unit);
 }
 
-}  // namespace units
+namespace detail {
 
-namespace std {
-
-template<units::Reference R1, units::Reference R2>
+[[nodiscard]] consteval auto common_reference(Reference auto r1, Reference auto r2)
   requires requires {
-             typename common_type_t<remove_const_t<decltype(R1::dimension)>, remove_const_t<decltype(R2::dimension)>>;
-             typename common_type_t<remove_const_t<decltype(R1::unit)>, remove_const_t<decltype(R2::unit)>>;
+             {
+               common_dimension(r1.dimension, r2.dimension)
+               } -> Dimension;
+             {
+               common_unit(r1.unit, r2.unit)
+               } -> Unit;
            }
-struct common_type<R1, R2> {
-private:
-  using dim = common_type_t<remove_const_t<decltype(R1::dimension)>, remove_const_t<decltype(R2::dimension)>>;
-  using unit = common_type_t<remove_const_t<decltype(R1::unit)>, remove_const_t<decltype(R2::unit)>>;
-public:
-  using type = units::reference<dim{}, unit{}>;
-};
+{
+  return reference<common_dimension(r1.dimension, r2.dimension), common_unit(r1.unit, r2.unit)>{};
+}
 
-}  // namespace std
+}  // namespace detail
+}  // namespace units
diff --git a/src/core/include/units/unit.h b/src/core/include/units/unit.h
index 6001a8ec..a2fe3d19 100644
--- a/src/core/include/units/unit.h
+++ b/src/core/include/units/unit.h
@@ -796,9 +796,9 @@ template<typename CharT = char, Unit U>
 
 namespace detail {
 
-
-template<typename U1, typename U2>
-[[nodiscard]] consteval auto common_type_impl(const U1 u1, const U2 u2)
+template<Unit U1, Unit U2>
+[[nodiscard]] consteval auto common_unit(U1 u1, U2 u2)
+  requires(interconvertible(u1, u2))
 {
   if constexpr (U1{} == U2{}) {
     if constexpr (std::derived_from<U1, U2>)
@@ -821,15 +821,4 @@ template<typename U1, typename U2>
 }
 
 }  // namespace detail
-
 }  // namespace units
-
-namespace std {
-
-template<units::Unit U1, units::Unit U2>
-  requires(units::interconvertible(U1{}, U2{}))
-struct common_type<U1, U2> {
-  using type = std::remove_const_t<decltype(::units::detail::common_type_impl(U1{}, U2{}))>;
-};
-
-}  // namespace std
diff --git a/test/unit_test/static/dimension_test.cpp b/test/unit_test/static/dimension_test.cpp
index c4e46edf..62e3be17 100644
--- a/test/unit_test/static/dimension_test.cpp
+++ b/test/unit_test/static/dimension_test.cpp
@@ -174,15 +174,15 @@ static_assert(1 / time != frequency);
 static_assert(interconvertible(1 / time, frequency));
 static_assert(1 / frequency == time);
 static_assert(frequency * time == dimension_one);
-static_assert(std::is_same_v<std::common_type_t<decltype(1 / time), decltype(frequency)>, frequency_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(frequency), decltype(1 / time)>, frequency_>);
+static_assert(is_of_type<detail::common_dimension(1 / time, frequency), frequency_>);
+static_assert(is_of_type<detail::common_dimension(frequency, 1 / time), frequency_>);
 
 static_assert(length * length != area);
 static_assert(interconvertible(length * length, area));
 static_assert(length * length != volume);
 static_assert(area / length == length);
-static_assert(std::is_same_v<std::common_type_t<decltype(length * length), decltype(area)>, area_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(area), decltype(length * length)>, area_>);
+static_assert(is_of_type<detail::common_dimension(length* length, area), area_>);
+static_assert(is_of_type<detail::common_dimension(area, length* length), area_>);
 
 static_assert(length * length * length != volume);
 static_assert(area * length != volume);
@@ -197,10 +197,9 @@ static_assert(length * time != speed);
 static_assert(length / time / time != speed);
 static_assert(length / speed == time);
 static_assert(speed * time == length);
-static_assert(std::is_same_v<std::common_type_t<decltype(length / time), decltype(speed)>, speed_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(speed), decltype(length / time)>, speed_>);
-static_assert(
-  std::is_same_v<std::common_type_t<decltype(length / time), decltype(length / time)>, decltype(length / time)>);
+static_assert(is_of_type<detail::common_dimension(length / time, speed), speed_>);
+static_assert(is_of_type<detail::common_dimension(speed, length / time), speed_>);
+static_assert(is_of_type<detail::common_dimension(length / time, length / time), decltype(length / time)>);
 
 static_assert(length / time / time != acceleration);
 static_assert(length / (time * time) != acceleration);
@@ -213,8 +212,8 @@ static_assert(acceleration / speed != frequency);
 // comparison of convertible named dimensions
 static_assert(velocity != speed);
 static_assert(interconvertible(speed, velocity));
-static_assert(std::is_same_v<std::common_type_t<decltype(velocity), decltype(speed)>, velocity_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(speed), decltype(velocity)>, velocity_>);
+static_assert(is_of_type<detail::common_dimension(velocity, speed), velocity_>);
+static_assert(is_of_type<detail::common_dimension(speed, velocity), velocity_>);
 
 // comparison of convertible unnamed dimensions
 static_assert(is_of_type<mass * acceleration, derived_dimension<length_, mass_, per<units::power<time_, 2>>>>);
diff --git a/test/unit_test/static/unit_test.cpp b/test/unit_test/static/unit_test.cpp
index cb1c2ddd..3b0427ed 100644
--- a/test/unit_test/static/unit_test.cpp
+++ b/test/unit_test/static/unit_test.cpp
@@ -469,35 +469,33 @@ static_assert(
   is_of_type<pow<2>(mag<3600>* second), scaled_unit<mag<3600> * mag<3600>, derived_unit<power<second_, 2>>>>);
 
 // common_type
-static_assert(std::is_same_v<std::common_type_t<decltype(gram), decltype(gram)>, gram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(kilogram), decltype(kilogram)>, kilogram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(si::kilo<gram>), decltype(kilogram)>, kilogram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(kilogram), decltype(si::kilo<gram>)>, kilogram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(mag<1000> * gram), decltype(kilogram)>, kilogram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(kilogram), decltype(mag<1000> * gram)>, kilogram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(1 / second), decltype(hertz)>, hertz_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(hertz), decltype(1 / second)>, hertz_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(gram), decltype(kilogram)>, gram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(kilogram), decltype(gram)>, gram_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(second), decltype(hour)>, second_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(hour), decltype(second)>, second_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(minute), decltype(hour)>, minute_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(hour), decltype(minute)>, minute_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(si::kilo<metre>), decltype(si::milli<metre>)>,
-                             std::remove_const_t<decltype(si::milli<metre>)>>);
-static_assert(std::is_same_v<std::common_type_t<decltype(si::milli<metre>), decltype(si::kilo<metre>)>,
-                             std::remove_const_t<decltype(si::milli<metre>)>>);
-static_assert(std::is_same_v<std::common_type_t<decltype(yard), decltype(mile)>, yard_>);
-static_assert(std::is_same_v<std::common_type_t<decltype(mile), decltype(yard)>, yard_>);
+static_assert(is_of_type<detail::common_unit(gram, gram), gram_>);
+static_assert(is_of_type<detail::common_unit(kilogram, kilogram), kilogram_>);
+static_assert(is_of_type<detail::common_unit(si::kilo<gram>, kilogram), kilogram_>);
+static_assert(is_of_type<detail::common_unit(kilogram, si::kilo<gram>), kilogram_>);
+static_assert(is_of_type<detail::common_unit(mag<1000>* gram, kilogram), kilogram_>);
+static_assert(is_of_type<detail::common_unit(kilogram, mag<1000>* gram), kilogram_>);
+static_assert(is_of_type<detail::common_unit(1 / second, hertz), hertz_>);
+static_assert(is_of_type<detail::common_unit(hertz, 1 / second), hertz_>);
+static_assert(is_of_type<detail::common_unit(gram, kilogram), gram_>);
+static_assert(is_of_type<detail::common_unit(kilogram, gram), gram_>);
+static_assert(is_of_type<detail::common_unit(second, hour), second_>);
+static_assert(is_of_type<detail::common_unit(hour, second), second_>);
+static_assert(is_of_type<detail::common_unit(minute, hour), minute_>);
+static_assert(is_of_type<detail::common_unit(hour, minute), minute_>);
+static_assert(
+  is_of_type<detail::common_unit(si::kilo<metre>, si::milli<metre>), std::remove_const_t<decltype(si::milli<metre>)>>);
+static_assert(
+  is_of_type<detail::common_unit(si::milli<metre>, si::kilo<metre>), std::remove_const_t<decltype(si::milli<metre>)>>);
+static_assert(is_of_type<detail::common_unit(yard, mile), yard_>);
+static_assert(is_of_type<detail::common_unit(mile, yard), yard_>);
 // TODO The below have long/unreadable magnitude types
-static_assert(std::is_same_v<std::common_type_t<decltype(kilometre / hour), decltype(metre / second)>,
-                             scaled_unit<mag<ratio{1, 18}>, derived_unit<metre_, per<second_>>>>);
-static_assert(std::is_same_v<std::common_type_t<decltype(metre / second), decltype(kilometre / hour)>,
-                             scaled_unit<mag<ratio{1, 18}>, derived_unit<metre_, per<second_>>>>);
-static_assert(
-  std::is_same_v<std::common_type_t<decltype(kilometre), decltype(mile)>, scaled_unit<mag<ratio{8, 125}>, metre_>>);
-static_assert(
-  std::is_same_v<std::common_type_t<decltype(mile), decltype(kilometre)>, scaled_unit<mag<ratio{8, 125}>, metre_>>);
+static_assert(is_of_type<detail::common_unit(kilometre / hour, metre / second),
+                         scaled_unit<mag<ratio{1, 18}>, derived_unit<metre_, per<second_>>>>);
+static_assert(is_of_type<detail::common_unit(metre / second, kilometre / hour),
+                         scaled_unit<mag<ratio{1, 18}>, derived_unit<metre_, per<second_>>>>);
+static_assert(is_of_type<detail::common_unit(kilometre, mile), scaled_unit<mag<ratio{8, 125}>, metre_>>);
+static_assert(is_of_type<detail::common_unit(mile, kilometre), scaled_unit<mag<ratio{8, 125}>, metre_>>);
 
 // unit symbols
 #ifdef __cpp_lib_constexpr_string