From 827edd8111c29d44cdcdf7b1ed79896495878046 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Mon, 2 Dec 2024 13:42:29 -0500
Subject: [PATCH] Simplify common unit if label has just one unit (#338)

If we don't do this, it's a weird situation: we end up with a
`CommonUnit<A, B, ...>` type, whose _label_ is just the label for a
single _simple_ unit `X`... which is quantity-equivalent to
`CommonUnit<A, B, ...>`.  This means that when we print it, it will
_look just like it is_ `X`, and people might get confused.

With this PR, when we're in this situation, we simply make
`CommonUnitT<A, B, ...>` (note the `T`, i.e., "compute the common unit")
return `X` itself!

To do this, we _could_ try to piggyback on the label definition.  I
found that way introduces some really confusing circular dependencies. I
find it is much simpler to simply count the _distinct unscaled units_ in
the common unit, and do the simplification exactly when we get down to
only one.

Helps #105.
---
 au/code/au/unit_of_measure.hh      | 37 ++++++++++++++++++++++++++++--
 au/code/au/unit_of_measure_test.cc | 35 ++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/au/code/au/unit_of_measure.hh b/au/code/au/unit_of_measure.hh
index 45e06463..0f8580c2 100644
--- a/au/code/au/unit_of_measure.hh
+++ b/au/code/au/unit_of_measure.hh
@@ -516,6 +516,11 @@ struct CommonUnit {
 template <typename A, typename B>
 struct InOrderFor<CommonUnit, A, B> : InOrderFor<UnitProduct, A, B> {};
 
+template <typename... Us>
+struct UnitList {};
+template <typename A, typename B>
+struct InOrderFor<UnitList, A, B> : InOrderFor<UnitProduct, A, B> {};
+
 namespace detail {
 // This machinery searches a unit list for one that "matches" a target unit.
 //
@@ -524,7 +529,7 @@ namespace detail {
 // Generic template.
 template <template <class, class> class Matcher,
           typename TargetUnit,
-          typename UnitList = TargetUnit>
+          typename UnitListT = TargetUnit>
 struct FirstMatchingUnit;
 
 // Base case for an empty list: the target unit is the best match.
@@ -612,6 +617,32 @@ constexpr typename CommonUnitLabelImpl<Us...>::LabelT CommonUnitLabelImpl<Us...>
 template <typename U>
 struct CommonUnitLabelImpl<U> : UnitLabel<U> {};
 
+template <typename U>
+struct UnscaledUnitImpl : stdx::type_identity<U> {};
+template <typename U, typename M>
+struct UnscaledUnitImpl<ScaledUnit<U, M>> : stdx::type_identity<U> {};
+template <typename U>
+using UnscaledUnit = typename UnscaledUnitImpl<U>::type;
+
+template <typename U>
+struct DistinctUnscaledUnitsImpl : stdx::type_identity<UnitList<UnscaledUnit<U>>> {};
+template <typename U>
+using DistinctUnscaledUnits = typename DistinctUnscaledUnitsImpl<U>::type;
+template <typename... Us>
+struct DistinctUnscaledUnitsImpl<CommonUnit<Us...>>
+    : stdx::type_identity<FlatDedupedTypeListT<UnitList, UnscaledUnit<Us>...>> {};
+
+template <typename U, typename DistinctUnits>
+struct SimplifyIfOnlyOneUnscaledUnitImpl;
+template <typename U>
+using SimplifyIfOnlyOneUnscaledUnit =
+    typename SimplifyIfOnlyOneUnscaledUnitImpl<U, DistinctUnscaledUnits<U>>::type;
+template <typename U, typename SoleUnscaledUnit>
+struct SimplifyIfOnlyOneUnscaledUnitImpl<U, UnitList<SoleUnscaledUnit>>
+    : stdx::type_identity<decltype(SoleUnscaledUnit{} * UnitRatioT<U, SoleUnscaledUnit>{})> {};
+template <typename U, typename... Us>
+struct SimplifyIfOnlyOneUnscaledUnitImpl<U, UnitList<Us...>> : stdx::type_identity<U> {};
+
 }  // namespace detail
 
 template <typename A, typename B>
@@ -626,7 +657,9 @@ using ComputeCommonUnitImpl =
 
 template <typename... Us>
 struct ComputeCommonUnit
-    : detail::FirstMatchingUnit<AreUnitsQuantityEquivalent, ComputeCommonUnitImpl<Us...>> {};
+    : stdx::type_identity<detail::SimplifyIfOnlyOneUnscaledUnit<
+          typename detail::FirstMatchingUnit<AreUnitsQuantityEquivalent,
+                                             ComputeCommonUnitImpl<Us...>>::type>> {};
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // `CommonPointUnitT` helper implementation.
diff --git a/au/code/au/unit_of_measure_test.cc b/au/code/au/unit_of_measure_test.cc
index 2c03f9d6..83e151f3 100644
--- a/au/code/au/unit_of_measure_test.cc
+++ b/au/code/au/unit_of_measure_test.cc
@@ -466,6 +466,11 @@ TEST(CommonUnit, DownranksAnonymousScaledUnits) {
     StaticAssertTypeEq<CommonUnitT<Yards, decltype(Feet{} * mag<3>())>, Yards>();
 }
 
+TEST(CommonUnit, WhenCommonUnitLabelWouldBeIdenticalToSomeUnitJustUsesThatUnit) {
+    StaticAssertTypeEq<CommonUnitT<decltype(Feet{} * mag<6>()), decltype(Feet{} * mag<10>())>,
+                       decltype(Feet{} * mag<2>())>();
+}
+
 // Four coprime units of the same dimension.
 struct W : decltype(Inches{} * mag<2>()) {};
 struct X : decltype(Inches{} * mag<3>()) {};
@@ -701,5 +706,35 @@ TEST(EliminateRedundantUnits, AlwaysRemovesSameUnitAmongQuantityEquivalentChoice
                        EliminateRedundantUnits<SomePack<Twelvinch, Feet>>>();
 }
 
+TEST(UnscaledUnit, IdentityForGeneralUnits) {
+    StaticAssertTypeEq<UnscaledUnit<Feet>, Feet>();
+    StaticAssertTypeEq<UnscaledUnit<Celsius>, Celsius>();
+}
+
+TEST(UnscaledUnit, RemovesScaleFactorFromScaledUnit) {
+    StaticAssertTypeEq<UnscaledUnit<decltype(Feet{} * mag<3>())>, Feet>();
+    StaticAssertTypeEq<UnscaledUnit<decltype(Celsius{} / mag<2>())>, Celsius>();
+}
+
+TEST(DistinctUnscaledUnits, UnitListOfOneElementForNonCommonUnit) {
+    StaticAssertTypeEq<DistinctUnscaledUnits<Feet>, UnitList<Feet>>();
+    StaticAssertTypeEq<DistinctUnscaledUnits<decltype(Feet{} / mag<12>())>, UnitList<Feet>>();
+}
+
+TEST(DistinctUnscaledUnits, RemovesDupesFromCommonUnit) {
+    StaticAssertTypeEq<
+        DistinctUnscaledUnits<decltype(common_unit(Feet{} * mag<3>(), Feet{} / mag<12>()))>,
+        UnitList<Feet>>();
+    StaticAssertTypeEq<DistinctUnscaledUnits<decltype(common_unit(
+                           Feet{} * mag<3>(), Inches{} * mag<48>(), Feet{} * mag<5>()))>,
+                       UnitList<Inches, Feet>>();
+}
+
+TEST(SimplifyIfOnlyOneUnscaledUnit, IdentityForNonCommonUnit) {
+    StaticAssertTypeEq<SimplifyIfOnlyOneUnscaledUnit<Feet>, Feet>();
+    StaticAssertTypeEq<SimplifyIfOnlyOneUnscaledUnit<decltype(Feet{} * mag<3>())>,
+                       decltype(Feet{} * mag<3>())>();
+}
+
 }  // namespace detail
 }  // namespace au