Support mixed Constant-Quantity math functions

au/code/au/constant_test.cc

@@ -18,10 +18,12 @@
 
 #include "au/chrono_interop.hh"
 #include "au/testing.hh"
+#include "au/units/degrees.hh"
 #include "au/units/joules.hh"
 #include "au/units/meters.hh"
 #include "au/units/newtons.hh"
 #include "au/units/radians.hh"
+#include "au/units/revolutions.hh"
 #include "au/units/seconds.hh"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -278,6 +280,29 @@ TEST(Constant, SupportsUnitSlotAPIs) {
     EXPECT_THAT(three_c_mps.in(c), SameTypeAndValue(3.f));
 }
 
+TEST(Constant, SupportsMinWithQuantity) {
+    EXPECT_THAT(min(c, (meters / second)(100)), SameTypeAndValue((meters / second)(100)));
+    EXPECT_THAT(min((meters / second)(1'000'000'000), c),
+                SameTypeAndValue((meters / second)(299'792'458)));
+}
+
+TEST(Constant, SupportsMaxWithQuantity) {
+    EXPECT_THAT(max(c, (meters / second)(100)), SameTypeAndValue((meters / second)(299'792'458)));
+    EXPECT_THAT(max((meters / second)(1'000'000'000), c),
+                SameTypeAndValue((meters / second)(1'000'000'000)));
+}
+
+TEST(Constant, SupportsClampWithQuantity) {
+    EXPECT_THAT(clamp((meters / second)(100), c / mag<2>(), c),
+                SameTypeAndValue((meters / second)(149'896'229)));
+}
+
+TEST(Constant, SupportsModWithQuantity) {
+    constexpr auto half_rev = make_constant(revolutions / mag<2>());
+    EXPECT_THAT(half_rev % degrees(100), SameTypeAndValue(degrees(80)));
+    EXPECT_THAT(degrees(300) % half_rev, SameTypeAndValue(degrees(120)));
+}
+
 TEST(CanStoreValueIn, ChecksRangeOfTypeForIntegers) {
     EXPECT_TRUE(decltype(c)::can_store_value_in<int32_t>(meters / second));
     EXPECT_FALSE(decltype(c)::can_store_value_in<int16_t>(meters / second));

au/code/au/math.hh

@@ -161,26 +161,6 @@ constexpr auto clamp(Quantity<UV, RV> v, Quantity<ULo, RLo> lo, Quantity<UHi, RH
     return (v < lo) ? ResultT{lo} : (hi < v) ? ResultT{hi} : ResultT{v};
 }
 
-// `clamp` overloads for when either boundary is `Zero`.
-//
-// NOTE: these will not work if _both_ boundaries are `Zero`, or if the quantity being clamped is
-// `Zero`.  We do not think these use cases are very useful, but we're open to revisiting this if we
-// receive a persuasive argument otherwise.
-template <typename UV, typename UHi, typename RV, typename RHi>
-constexpr auto clamp(Quantity<UV, RV> v, Zero z, Quantity<UHi, RHi> hi) {
-    using U = CommonUnitT<UV, UHi>;
-    using R = std::common_type_t<RV, RHi>;
-    using ResultT = Quantity<U, R>;
-    return (v < z) ? ResultT{z} : (hi < v) ? ResultT{hi} : ResultT{v};
-}
-template <typename UV, typename ULo, typename RV, typename RLo>
-constexpr auto clamp(Quantity<UV, RV> v, Quantity<ULo, RLo> lo, Zero z) {
-    using U = CommonUnitT<UV, ULo>;
-    using R = std::common_type_t<RV, RLo>;
-    using ResultT = Quantity<U, R>;
-    return (v < lo) ? ResultT{lo} : (z < v) ? ResultT{z} : ResultT{v};
-}
-
 // Clamp the first point to within the range of the second two.
 template <typename UV, typename ULo, typename UHi, typename RV, typename RLo, typename RHi>
 constexpr auto clamp(QuantityPoint<UV, RV> v,
@@ -336,12 +316,6 @@ constexpr auto max(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {
     return detail::using_common_type(q1, q2, detail::StdMaxByValue{});
 }
 
-// Overload to resolve ambiguity with `std::max` for identical `Quantity` types.
-template <typename U, typename R>
-constexpr auto max(Quantity<U, R> a, Quantity<U, R> b) {
-    return std::max(a, b);
-}
-
 // The maximum of two point values of the same dimension.
 //
 // Unlike std::max, returns by value rather than by reference, because the types might differ.
@@ -356,23 +330,6 @@ constexpr auto max(QuantityPoint<U, R> a, QuantityPoint<U, R> b) {
     return std::max(a, b);
 }
 
-// `max` overloads for when Zero is one of the arguments.
-//
-// NOTE: these will not work if _both_ arguments are `Zero`, but we don't plan to support this
-// unless we find a compelling use case.
-template <typename T>
-constexpr auto max(Zero z, T x) {
-    static_assert(std::is_convertible<Zero, T>::value,
-                  "Cannot compare type to abstract notion Zero");
-    return std::max(T{z}, x);
-}
-template <typename T>
-constexpr auto max(T x, Zero z) {
-    static_assert(std::is_convertible<Zero, T>::value,
-                  "Cannot compare type to abstract notion Zero");
-    return std::max(x, T{z});
-}
-
 namespace detail {
 // We can't use lambdas in `constexpr` contexts until C++17, so we make a manual function object.
 struct StdMinByValue {
@@ -391,12 +348,6 @@ constexpr auto min(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {
     return detail::using_common_type(q1, q2, detail::StdMinByValue{});
 }
 
-// Overload to resolve ambiguity with `std::min` for identical `Quantity` types.
-template <typename U, typename R>
-constexpr auto min(Quantity<U, R> a, Quantity<U, R> b) {
-    return std::min(a, b);
-}
-
 // The minimum of two point values of the same dimension.
 //
 // Unlike std::min, returns by value rather than by reference, because the types might differ.
@@ -411,23 +362,6 @@ constexpr auto min(QuantityPoint<U, R> a, QuantityPoint<U, R> b) {
     return std::min(a, b);
 }
 
-// `min` overloads for when Zero is one of the arguments.
-//
-// NOTE: these will not work if _both_ arguments are `Zero`, but we don't plan to support this
-// unless we find a compelling use case.
-template <typename T>
-constexpr auto min(Zero z, T x) {
-    static_assert(std::is_convertible<Zero, T>::value,
-                  "Cannot compare type to abstract notion Zero");
-    return std::min(T{z}, x);
-}
-template <typename T>
-constexpr auto min(T x, Zero z) {
-    static_assert(std::is_convertible<Zero, T>::value,
-                  "Cannot compare type to abstract notion Zero");
-    return std::min(x, T{z});
-}
-
 // The (zero-centered) floating point remainder of two values of the same dimension.
 template <typename U1, typename R1, typename U2, typename R2>
 auto remainder(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {

au/code/au/math_test.cc

@@ -147,6 +147,18 @@ TEST(clamp, SupportsZeroForUpperBoundaryArgument) {
     EXPECT_THAT(clamp(feet(+1), inches(-18), ZERO), SameTypeAndValue(inches(0)));
 }
 
+TEST(clamp, SupportsZeroForValueArgument) {
+    EXPECT_THAT(clamp(ZERO, inches(-18), inches(18)), SameTypeAndValue(inches(0)));
+    EXPECT_THAT(clamp(ZERO, inches(24), inches(60)), SameTypeAndValue(inches(24)));
+    EXPECT_THAT(clamp(ZERO, feet(2), inches(60)), SameTypeAndValue(inches(24)));
+}
+
+TEST(clamp, SupportsZeroForMultipleArguments) {
+    EXPECT_THAT(clamp(ZERO, inches(-8), ZERO), SameTypeAndValue(inches(0)));
+    EXPECT_THAT(clamp(ZERO, ZERO, feet(2)), SameTypeAndValue(feet(0)));
+    EXPECT_THAT(clamp(feet(6), ZERO, ZERO), SameTypeAndValue(feet(0)));
+}
+
 TEST(copysign, ReturnsSameTypesAsStdCopysignForSameUnitInputs) {
     auto expect_consistent_with_std_copysign = [](auto mag, auto raw_sgn) {
         for (const auto test_sgn : {-1, 0, +1}) {

au/code/au/quantity.hh

@@ -366,6 +366,9 @@ class Quantity {
         return *this;
     }
 
+    // Modulo operator (defined only for integral rep).
+    friend constexpr Quantity operator%(Quantity a, Quantity b) { return {a.value_ % b.value_}; }
+
     // Unary plus and minus.
     constexpr Quantity operator+() const { return {+value_}; }
     constexpr Quantity operator-() const { return {-value_}; }
@@ -413,6 +416,24 @@ class Quantity {
 
     friend constexpr Quantity from_nttp(NTTP val) { return val; }
 
+    ////////////////////////////////////////////////////////////////////////////////////////////////
+    // Hidden friends for select math functions.
+    //
+    // Moving the implementation here lets us effortlessly support callsites where any number of
+    // arguments are "shapeshifter" types that are compatible with this Quantity (such as `ZERO`, or
+    // various physical constant).
+    //
+    // Note that the min/max implementations return by _value_, for consistency with other Quantity
+    // implementations (because in the general case, the return type can differ from the inputs).
+    // Note, too, that we use the Walter Brown implementation for min/max, where min prefers `a`,
+    // max prefers `b`, and they never return the same input (although this matters less when we're
+    // returning by value).
+    friend constexpr Quantity min(Quantity a, Quantity b) { return b < a ? b : a; }
+    friend constexpr Quantity max(Quantity a, Quantity b) { return b < a ? a : b; }
+    friend constexpr Quantity clamp(Quantity v, Quantity lo, Quantity hi) {
+        return (v < lo) ? lo : ((hi < v) ? hi : v);
+    }
+
  private:
     template <typename OtherUnit, typename OtherRep>
     static constexpr void warn_if_integer_division() {

docs/reference/math.md

@@ -12,6 +12,10 @@ and you want the "max", just write plain `max(...)`.
 - Don't write `std::max(...)`, because that would give the wrong function.
 - Don't write `au::max(...)`, because that's neither necessary nor idiomatic.
 
+!!! warning
+    For some functions, including `min`, `max`, and `clamp`, this advice is _mandatory_ in many
+    cases, such as when the arguments have the same type.
+
 ## Function categories
 
 Here are the functions we provide, grouped roughly into related categories.
@@ -110,6 +114,11 @@ disambiguate our `min` or `max` implementations with respect to `std::min` and `
     support combining different units.  This means the return type will generally be different from
     the types of the inputs.
 
+!!! warning
+    You _must_ use _unqualified_ calls to `min` and `max` in many cases, including the common case
+    where the arguments have the same type.  Write `min(a, b)`, not `au::min(a, b)`: the latter will
+    frequently result in the right overload not being found.
+
 #### `clamp`
 
 "Clamp" the first parameter to the range defined by the second and third.  This is a _unit-aware_
@@ -180,6 +189,11 @@ expand the note below for further details.
     - We do not currently plan to provide the four-parameter overload, unless we get a compelling
       use case.
 
+!!! warning
+    You _must_ use _unqualified_ calls to `clamp` in many cases, including the common case where the
+    arguments have the same type.  Write `clamp(a, b, c)`, not `au::clamp(a, b, c)`: the latter will
+    frequently result in the right overload not being found.
+
 ### Exponentiation
 
 #### `int_pow`