From 16b41d3cdc907d8dabac4749d24bbd8939f223a6 Mon Sep 17 00:00:00 2001 From: Chip Hogg Date: Fri, 22 Nov 2024 14:58:10 -0500 Subject: [PATCH 1/2] Support mixed `Constant`-`Quantity` math functions Specifically, we add support for `min`, `max`, `clamp`, and `%`. It turns out that the most economical way to do this is via hidden friends. The downside is that this change is invasive to `Quantity`, whereas we'd generally rather add functionality from the outside. But the upsides are that we get to remove a fair bit of extra special casing that we had done for `Zero` overloads, and even some disambiguating overloads. Not only that, but we can now support some combinations that we hadn't added before, simply because it would have been too much work! Here's how it works. The hidden friend approach covers us whenever somebody calls a function with two exactly-identical types, _or_ whenever _one_ of the types is an exact match, and the _other_ can _implicitly convert_ to it. This lets us cover all "shapeshifter types" --- `Zero`, `Constant` --- at one stroke. It even automatically covers _new shapeshifter types we don't know about_: anything implicitly convertible to `Quantity` will work! For the `min` and `max` implementation, I went with the Walter Brown approach where `min` prefers to return `a`, and `max` prefers `b`. This is the most general and correct approach w.r.t. how it handles "ties", although in our specific case this doesn't matter because we're not returning a reference. Still, I'm glad to put one more example of the Right Approach out in the wild, and I prefer it to a call to `std::min` because it doesn't force us to take a direct dependency on ``. We have two "disambiguating" overloads remaining in `math.hh`, both applying to `QuantityPoint`: one for `min`, one for `max`. I decided not to add hidden friends there, because the cost of an invasive change, plus the cost of moving these implementations far from the other overloads in `math.hh`, outweighs the smaller benefits we would obtain in this case. Helps #90. At this point, the `Constant` _implementation_ is feature complete, and all we need to do is add concrete examples of `Constant` to our library, updating the single-file package script and documentation! --- au/code/au/constant_test.cc | 25 ++++++++++++++ au/code/au/math.hh | 66 ------------------------------------- au/code/au/math_test.cc | 12 +++++++ au/code/au/quantity.hh | 21 ++++++++++++ 4 files changed, 58 insertions(+), 66 deletions(-) diff --git a/au/code/au/constant_test.cc b/au/code/au/constant_test.cc index 4cd10460..ff82c49f 100644 --- a/au/code/au/constant_test.cc +++ b/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(meters / second)); EXPECT_FALSE(decltype(c)::can_store_value_in(meters / second)); diff --git a/au/code/au/math.hh b/au/code/au/math.hh index 5566978d..b61b7aaf 100644 --- a/au/code/au/math.hh +++ b/au/code/au/math.hh @@ -161,26 +161,6 @@ constexpr auto clamp(Quantity v, Quantity lo, Quantity -constexpr auto clamp(Quantity v, Zero z, Quantity hi) { - using U = CommonUnitT; - using R = std::common_type_t; - using ResultT = Quantity; - return (v < z) ? ResultT{z} : (hi < v) ? ResultT{hi} : ResultT{v}; -} -template -constexpr auto clamp(Quantity v, Quantity lo, Zero z) { - using U = CommonUnitT; - using R = std::common_type_t; - using ResultT = Quantity; - return (v < lo) ? ResultT{lo} : (z < v) ? ResultT{z} : ResultT{v}; -} - // Clamp the first point to within the range of the second two. template constexpr auto clamp(QuantityPoint v, @@ -336,12 +316,6 @@ constexpr auto max(Quantity q1, Quantity q2) { return detail::using_common_type(q1, q2, detail::StdMaxByValue{}); } -// Overload to resolve ambiguity with `std::max` for identical `Quantity` types. -template -constexpr auto max(Quantity a, Quantity 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 a, QuantityPoint 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 -constexpr auto max(Zero z, T x) { - static_assert(std::is_convertible::value, - "Cannot compare type to abstract notion Zero"); - return std::max(T{z}, x); -} -template -constexpr auto max(T x, Zero z) { - static_assert(std::is_convertible::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 q1, Quantity q2) { return detail::using_common_type(q1, q2, detail::StdMinByValue{}); } -// Overload to resolve ambiguity with `std::min` for identical `Quantity` types. -template -constexpr auto min(Quantity a, Quantity 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 a, QuantityPoint 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 -constexpr auto min(Zero z, T x) { - static_assert(std::is_convertible::value, - "Cannot compare type to abstract notion Zero"); - return std::min(T{z}, x); -} -template -constexpr auto min(T x, Zero z) { - static_assert(std::is_convertible::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 auto remainder(Quantity q1, Quantity q2) { diff --git a/au/code/au/math_test.cc b/au/code/au/math_test.cc index fa3dac52..4c2c7f8a 100644 --- a/au/code/au/math_test.cc +++ b/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}) { diff --git a/au/code/au/quantity.hh b/au/code/au/quantity.hh index bb85748f..3cdf2b93 100644 --- a/au/code/au/quantity.hh +++ b/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 static constexpr void warn_if_integer_division() { From 8ef88a8e942cd9e93253551edc7855fe2e126f71 Mon Sep 17 00:00:00 2001 From: Chip Hogg Date: Sat, 23 Nov 2024 09:57:28 -0500 Subject: [PATCH 2/2] Update docs Tested with `au-docs-serve` --- docs/reference/math.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/reference/math.md b/docs/reference/math.md index 359d54df..8bd1be28 100644 --- a/docs/reference/math.md +++ b/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`