From 6d148e8007854193de7e049b93af231e010a0f89 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sun, 24 Mar 2024 16:14:22 -0400
Subject: [PATCH] Permit arithmetic operations with more types

The goal of these `std::is_arithmetic` SFINAE uses was to avoid
ambiguous overloads when we multiply a `Quantity` with another
`Quantity`.  It was only ever a shortcut.  It seems to work well in most
cases, but recently, a user pointed out that it doesn't help
`std::complex`.  (To be clear, we could _form_ a `Quantity` with
`std::complex` rep, but we couldn't multiply or divide a `Quantity` with
a `std::complex` scalar.)

This PR takes a different approach.

First, we define what constitutes a valid "rep".  We're very permissive
here, using a "deny-list" approach that filters out empty types, known
Au types, and other libraries' units types (which we detect via our
`CorrespondingQuantity` trait).  Incidentally, this should give us a
nice head start on #52.

Next, we permit an operation (multiplication or division) depending on
how the "candidate scalar" type would interact with the Quantity's rep:
simply put, the result must itself both exist, and be a valid rep.

The net result should be that we automatically consider a much wider
variety of types --- including complex number types from outside the
standard library --- to be valid scalars.  Crucially, this should also
be able to avoid breaking existing code: we are introducing potentially
a _lot_ of new overloads for these operators, and we can't allow any to
create an ambiguity with existing overloads.
---
 au/BUILD.bazel      |  25 +++++++
 au/quantity.hh      |   9 +--
 au/quantity_test.cc |  32 +++++++++
 au/rep.hh           | 148 +++++++++++++++++++++++++++++++++++++++
 au/rep_test.cc      | 166 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 376 insertions(+), 4 deletions(-)
 create mode 100644 au/rep.hh
 create mode 100644 au/rep_test.cc

diff --git a/au/BUILD.bazel b/au/BUILD.bazel
index 7e945707..5843a97b 100644
--- a/au/BUILD.bazel
+++ b/au/BUILD.bazel
@@ -395,6 +395,7 @@ cc_library(
         ":apply_magnitude",
         ":conversion_policy",
         ":operators",
+        ":rep",
         ":unit_of_measure",
         ":zero",
     ],
@@ -438,6 +439,30 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "rep",
+    hdrs = ["rep.hh"],
+    deps = [":stdx"],
+)
+
+cc_test(
+    name = "rep_test",
+    size = "small",
+    srcs = ["rep_test.cc"],
+    deps = [
+        ":chrono_interop",
+        ":constant",
+        ":magnitude",
+        ":prefix",
+        ":quantity",
+        ":quantity_point",
+        ":rep",
+        ":unit_symbol",
+        ":units",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "stdx",
     srcs = glob([
diff --git a/au/quantity.hh b/au/quantity.hh
index fb3752b9..b567d43a 100644
--- a/au/quantity.hh
+++ b/au/quantity.hh
@@ -19,6 +19,7 @@
 #include "au/apply_magnitude.hh"
 #include "au/conversion_policy.hh"
 #include "au/operators.hh"
+#include "au/rep.hh"
 #include "au/stdx/functional.hh"
 #include "au/unit_of_measure.hh"
 #include "au/zero.hh"
@@ -293,21 +294,21 @@ class Quantity {
     }
 
     // Scalar multiplication.
-    template <typename T, typename = std::enable_if_t<std::is_arithmetic<T>::value>>
+    template <typename T, typename = std::enable_if_t<IsProductValidRep<RepT, T>::value>>
     friend constexpr auto operator*(Quantity a, T s) {
         return make_quantity<UnitT>(a.value_ * s);
     }
-    template <typename T, typename = std::enable_if_t<std::is_arithmetic<T>::value>>
+    template <typename T, typename = std::enable_if_t<IsProductValidRep<T, RepT>::value>>
     friend constexpr auto operator*(T s, Quantity a) {
         return make_quantity<UnitT>(s * a.value_);
     }
 
     // Scalar division.
-    template <typename T, typename = std::enable_if_t<std::is_arithmetic<T>::value>>
+    template <typename T, typename = std::enable_if_t<IsQuotientValidRep<RepT, T>::value>>
     friend constexpr auto operator/(Quantity a, T s) {
         return make_quantity<UnitT>(a.value_ / s);
     }
-    template <typename T, typename = std::enable_if_t<std::is_arithmetic<T>::value>>
+    template <typename T, typename = std::enable_if_t<IsQuotientValidRep<T, RepT>::value>>
     friend constexpr auto operator/(T s, Quantity a) {
         warn_if_integer_division<T>();
         return make_quantity<decltype(pow<-1>(unit))>(s / a.value_);
diff --git a/au/quantity_test.cc b/au/quantity_test.cc
index 8e582333..733fa619 100644
--- a/au/quantity_test.cc
+++ b/au/quantity_test.cc
@@ -14,11 +14,14 @@
 
 #include "au/quantity.hh"
 
+#include <complex>
+
 #include "au/prefix.hh"
 #include "au/testing.hh"
 #include "au/utility/type_traits.hh"
 #include "gtest/gtest.h"
 
+using ::testing::DoubleEq;
 using ::testing::StaticAssertTypeEq;
 
 namespace au {
@@ -425,6 +428,35 @@ TEST(Quantity, ScalarMultiplicationWorks) {
     EXPECT_EQ(feet(9), d * 3);
 }
 
+TEST(Quantity, SupportsMultiplicationForComplexRep) {
+    constexpr auto a = (miles / hour)(std::complex<double>{1.0, -2.0});
+    constexpr auto b = hours(std::complex<double>{-3.0, 4.0});
+    EXPECT_THAT(a * b, SameTypeAndValue(miles(std::complex<double>{5.0, 10.0})));
+}
+
+TEST(Quantity, SupportsMultiplicationOfRealQuantityByComplexCoefficient) {
+    constexpr auto a = miles(10.0);
+    constexpr auto b = std::complex<double>{-3.0, 4.0};
+    EXPECT_THAT(a * b, SameTypeAndValue(miles(std::complex<double>{-30.0, 40.0})));
+    EXPECT_THAT(b * a, SameTypeAndValue(miles(std::complex<double>{-30.0, 40.0})));
+}
+
+TEST(Quantity, SupportsDivisionOfRealQuantityByComplexCoefficient) {
+    constexpr auto a = miles(100.0);
+    constexpr auto b = std::complex<double>{-3.0, 4.0};
+    const auto quotient = (a / b).in(miles);
+    EXPECT_THAT(quotient.real(), DoubleEq(-12.0));
+    EXPECT_THAT(quotient.imag(), DoubleEq(-16.0));
+}
+
+TEST(Quantity, SupportsDivisionOfRealQuantityIntoComplexCoefficient) {
+    constexpr auto a = std::complex<double>{-30.0, 40.0};
+    constexpr auto b = miles(10.0);
+    const auto quotient = (a / b).in(inverse(miles));
+    EXPECT_THAT(quotient.real(), DoubleEq(-3.0));
+    EXPECT_THAT(quotient.imag(), DoubleEq(4.0));
+}
+
 TEST(Quantity, CanDivideArbitraryQuantities) {
     constexpr auto d = feet(6.);
     constexpr auto t = hours(3.);
diff --git a/au/rep.hh b/au/rep.hh
new file mode 100644
index 00000000..562e8226
--- /dev/null
+++ b/au/rep.hh
@@ -0,0 +1,148 @@
+// Copyright 2024 Aurora Operations, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include <type_traits>
+
+#include "au/stdx/experimental/is_detected.hh"
+#include "au/stdx/type_traits.hh"
+
+namespace au {
+
+//
+// A type trait that determines if a type is a valid representation type for `Quantity` or
+// `QuantityPoint`.
+//
+template <typename T>
+struct IsValidRep;
+
+//
+// A type trait to indicate whether the product of two types is a valid rep.
+//
+// Will validly return `false` if the product does not exist.
+//
+template <typename T, typename U>
+struct IsProductValidRep;
+
+//
+// A type trait to indicate whether the quotient of two types is a valid rep.
+//
+// Will validly return `false` if the quotient does not exist.
+//
+template <typename T, typename U>
+struct IsQuotientValidRep;
+
+////////////////////////////////////////////////////////////////////////////////////////////////////
+// Implementation details below.
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
+// Forward declarations for main Au container types.
+template <typename U, typename R>
+class Quantity;
+template <typename U, typename R>
+class QuantityPoint;
+template <typename T>
+struct CorrespondingQuantity;
+
+namespace detail {
+template <typename T>
+struct IsAuType : std::false_type {};
+
+template <typename U, typename R>
+struct IsAuType<::au::Quantity<U, R>> : std::true_type {};
+
+template <typename U, typename R>
+struct IsAuType<::au::QuantityPoint<U, R>> : std::true_type {};
+
+template <typename T>
+using CorrespondingUnit = typename CorrespondingQuantity<T>::Unit;
+
+template <typename T>
+using CorrespondingRep = typename CorrespondingQuantity<T>::Rep;
+
+template <typename T>
+struct HasCorrespondingQuantity
+    : stdx::conjunction<stdx::experimental::is_detected<CorrespondingUnit, T>,
+                        stdx::experimental::is_detected<CorrespondingRep, T>> {};
+
+template <typename T>
+using LooksLikeAuOrOtherQuantity = stdx::disjunction<IsAuType<T>, HasCorrespondingQuantity<T>>;
+
+// We need a way to form an "operation on non-quantity types only".  That is: it's some operation,
+// but _if either input is a quantity_, then we _don't even form the type_.
+//
+// The reason this very specific machinery lives in `rep.hh` is because when we're dealing with
+// operations on "types that might be a rep", we know we can exclude quantity types right away.
+// (Note that we're using the term "quantity" in an expansive sense, which includes not just
+// `au::Quantity`, but also `au::QuantityPoint`, and "quantity-like" types from other libraries
+// (which we consider as "anything that has a `CorrespondingQuantity`".
+template <template <class...> class Op, typename... Ts>
+struct ResultIfNoneAreQuantity;
+template <template <class...> class Op, typename... Ts>
+using ResultIfNoneAreQuantityT = typename ResultIfNoneAreQuantity<Op, Ts...>::type;
+
+// Default implementation where we know that none are quantities.
+template <bool AreAnyQuantity, template <class...> class Op, typename... Ts>
+struct ResultIfNoneAreQuantityImpl : stdx::type_identity<Op<Ts...>> {};
+
+// Implementation if any of the types are quantities.
+template <template <class...> class Op, typename... Ts>
+struct ResultIfNoneAreQuantityImpl<true, Op, Ts...> : stdx::type_identity<void> {};
+
+// The main implementation.
+template <template <class...> class Op, typename... Ts>
+struct ResultIfNoneAreQuantity
+    : ResultIfNoneAreQuantityImpl<stdx::disjunction<LooksLikeAuOrOtherQuantity<Ts>...>::value,
+                                  Op,
+                                  Ts...> {};
+
+// The `std::is_empty` is a good way to catch all of the various unit and other monovalue types in
+// our library, which have little else in common.  It's also just intrinsically true that it
+// wouldn't make much sense to use an empty type as a rep.
+template <typename T>
+struct IsKnownInvalidRep
+    : stdx::disjunction<std::is_empty<T>, LooksLikeAuOrOtherQuantity<T>, std::is_same<void, T>> {};
+
+// The type of the product of two types.
+template <typename T, typename U>
+using ProductType = decltype(std::declval<T>() * std::declval<U>());
+
+template <typename T, typename U>
+using ProductTypeOrVoid = stdx::experimental::detected_or_t<void, ProductType, T, U>;
+
+// The type of the quotient of two types.
+template <typename T, typename U>
+using QuotientType = decltype(std::declval<T>() / std::declval<U>());
+
+template <typename T, typename U>
+using QuotientTypeOrVoid = stdx::experimental::detected_or_t<void, QuotientType, T, U>;
+}  // namespace detail
+
+// Implementation for `IsValidRep`.
+//
+// For now, we'll accept anything that isn't explicitly known to be invalid.  We may tighten this up
+// later, but this seems like a reasonable starting point.
+template <typename T>
+struct IsValidRep : stdx::negation<detail::IsKnownInvalidRep<T>> {};
+
+template <typename T, typename U>
+struct IsProductValidRep
+    : IsValidRep<detail::ResultIfNoneAreQuantityT<detail::ProductTypeOrVoid, T, U>> {};
+
+template <typename T, typename U>
+struct IsQuotientValidRep
+    : IsValidRep<detail::ResultIfNoneAreQuantityT<detail::QuotientTypeOrVoid, T, U>> {};
+
+}  // namespace au
diff --git a/au/rep_test.cc b/au/rep_test.cc
new file mode 100644
index 00000000..8afe306f
--- /dev/null
+++ b/au/rep_test.cc
@@ -0,0 +1,166 @@
+// Copyright 2024 Aurora Operations, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "au/rep.hh"
+
+#include <complex>
+
+#include "au/chrono_interop.hh"
+#include "au/constant.hh"
+#include "au/magnitude.hh"
+#include "au/prefix.hh"
+#include "au/quantity.hh"
+#include "au/quantity_point.hh"
+#include "au/unit_symbol.hh"
+#include "au/units/liters.hh"
+#include "au/units/miles.hh"
+#include "au/units/webers.hh"
+#include "gtest/gtest.h"
+
+namespace au {
+namespace {
+
+using ::testing::StaticAssertTypeEq;
+
+// A custom quantity that corresponds to `QuantityI<Meters>`.
+struct MyMeters {
+    int value;
+};
+
+// A custom rep with no operations defined on it.
+struct IntWithNoOps {
+    int value;
+};
+
+// A custom type that can left-multiply a `double`.
+struct LeftMultiplyDoubleByThree {
+    friend double operator*(const LeftMultiplyDoubleByThree &, double x) { return 3.0 * x; }
+};
+
+// A custom type that divides a `float` into `10.0f`.
+struct DivideTenByFloat {
+    friend float operator/(const DivideTenByFloat &, float x) { return 10.0f / x; }
+};
+}  // namespace
+
+// Set up the correspondence between `MyMeters` and `QuantityI<Meters>`.
+template <>
+struct CorrespondingQuantity<MyMeters> {
+    using Unit = Meters;
+    using Rep = int;
+};
+
+TEST(IsValidRep, FalseForVoid) { EXPECT_FALSE(IsValidRep<void>::value); }
+
+TEST(IsValidRep, TrueForArithmeticTypes) {
+    EXPECT_TRUE(IsValidRep<int>::value);
+    EXPECT_TRUE(IsValidRep<float>::value);
+    EXPECT_TRUE(IsValidRep<double>::value);
+    EXPECT_TRUE(IsValidRep<uint8_t>::value);
+    EXPECT_TRUE(IsValidRep<int64_t>::value);
+}
+
+TEST(IsValidRep, TrueForStdComplex) {
+    EXPECT_TRUE(IsValidRep<std::complex<float>>::value);
+    EXPECT_TRUE(IsValidRep<std::complex<uint16_t>>::value);
+}
+
+TEST(IsValidRep, FalseForMagnitude) {
+    EXPECT_FALSE(IsValidRep<decltype(mag<84>())>::value);
+    EXPECT_FALSE(IsValidRep<decltype(sqrt(PI))>::value);
+}
+
+TEST(IsValidRep, FalseForUnits) {
+    EXPECT_FALSE(IsValidRep<Liters>::value);
+    EXPECT_FALSE(IsValidRep<Nano<Webers>>::value);
+}
+
+TEST(IsValidRep, FalseForQuantity) {
+    EXPECT_FALSE((IsValidRep<Quantity<Milli<Liters>, int>>::value));
+}
+
+TEST(IsValidRep, FalseForQuantityPoint) {
+    EXPECT_FALSE((IsValidRep<QuantityPoint<Miles, double>>::value));
+}
+
+TEST(IsValidRep, FalseForConstant) {
+    EXPECT_FALSE(IsValidRep<decltype(make_constant(liters / mile))>::value);
+}
+
+TEST(IsValidRep, FalseForSymbol) { EXPECT_FALSE(IsValidRep<SymbolFor<Webers>>::value); }
+
+TEST(IsValidRep, FalseForTypeWithCorrespondingQuantity) {
+    EXPECT_FALSE(IsValidRep<MyMeters>::value);
+    EXPECT_FALSE(IsValidRep<std::chrono::nanoseconds>::value);
+}
+
+TEST(IsProductValidRep, FalseIfProductDoesNotExist) {
+    EXPECT_FALSE((IsProductValidRep<IntWithNoOps, int>::value));
+    EXPECT_FALSE((IsProductValidRep<int, IntWithNoOps>::value));
+}
+
+TEST(IsProductValidRep, TrueOnlyForSideWhereProductExists) {
+    ASSERT_EQ(LeftMultiplyDoubleByThree{} * 4.5, 13.5);
+
+    EXPECT_TRUE((IsProductValidRep<LeftMultiplyDoubleByThree, double>::value));
+    EXPECT_FALSE((IsProductValidRep<double, LeftMultiplyDoubleByThree>::value));
+}
+
+TEST(IsQuotientValidRep, FalseIfQuotientDoesNotExist) {
+    EXPECT_FALSE((IsQuotientValidRep<IntWithNoOps, int>::value));
+    EXPECT_FALSE((IsQuotientValidRep<int, IntWithNoOps>::value));
+}
+
+TEST(IsQuotientValidRep, FalseIfQuotientIsQuantity) {
+    // Dividing by a Quantity can complicate matters because it involves hard compiler errors when
+    // that quantity has an integral rep.  Make sure we handle this gracefully.
+    EXPECT_FALSE((IsQuotientValidRep<int, Quantity<Miles, int>>::value));
+}
+
+TEST(IsQuotientValidRep, TrueOnlyForSideWhereQuotientExists) {
+    ASSERT_EQ(DivideTenByFloat{} / 2.0f, 5.0f);
+
+    EXPECT_FALSE((IsQuotientValidRep<float, DivideTenByFloat>::value));
+    EXPECT_TRUE((IsQuotientValidRep<DivideTenByFloat, float>::value));
+}
+
+namespace detail {
+TEST(ResultIfNoneAreQuantity, GivesResultWhenNoneAreQuantity) {
+    StaticAssertTypeEq<int, ResultIfNoneAreQuantityT<std::common_type_t, int, int>>();
+    StaticAssertTypeEq<std::tuple<int, double, float>,
+                       ResultIfNoneAreQuantityT<std::tuple, int, double, float>>();
+}
+
+TEST(ResultIfNoneAreQuantity, GivesVoidWhenAnyIsQuantity) {
+    StaticAssertTypeEq<void,
+                       ResultIfNoneAreQuantityT<std::common_type_t, int, Quantity<Miles, int>>>();
+    StaticAssertTypeEq<void,
+                       ResultIfNoneAreQuantityT<std::tuple, int, Quantity<Miles, int>, float>>();
+}
+
+TEST(ResultIfNoneAreQuantity, GivesVoidWhenAnyIsCorrespondingQuantity) {
+    StaticAssertTypeEq<void, ResultIfNoneAreQuantityT<std::common_type_t, int, MyMeters>>();
+    StaticAssertTypeEq<void, ResultIfNoneAreQuantityT<std::tuple, int, std::chrono::nanoseconds>>();
+}
+
+TEST(ProductTypeOrVoid, GivesProductTypeForArithmeticInputs) {
+    StaticAssertTypeEq<int, ProductTypeOrVoid<int, int>>();
+}
+
+TEST(ProductTypeOrVoid, GivesVoidForInputsWithNoProductType) {
+    StaticAssertTypeEq<void, ProductTypeOrVoid<IntWithNoOps, int>>();
+    StaticAssertTypeEq<void, ProductTypeOrVoid<int, IntWithNoOps>>();
+}
+}  // namespace detail
+}  // namespace au