From 00bd3c4c59aa2d736f6f152333822629b17776f9 Mon Sep 17 00:00:00 2001 From: Chip Hogg Date: Sun, 23 Jun 2024 16:54:02 -0400 Subject: [PATCH] Support `constexpr` in `min` and `max` These have been `constexpr` compatible since C++14. Mechanically, the way I did this was: 1. Find an overload that was not `constexpr`. 2. Make a test that requires it to be `constexpr` to pass (either tweaking an existing test, or making a new one). 3. Make sure the test fails. 4. Change the function until the test passes. The biggest challenge was that our default compiler wouldn't accept our lambda in a `constexpr` context. That feels like a bug, but in any case, we were able to work around it by making a manual function object with an explicitly `constexpr` call operator. Fixes #244. --- au/math.hh | 52 +++++++++++++++++++++++++++------------- au/math_test.cc | 63 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 86 insertions(+), 29 deletions(-) diff --git a/au/math.hh b/au/math.hh index e9f333d3..499194ac 100644 --- a/au/math.hh +++ b/au/math.hh @@ -318,17 +318,27 @@ constexpr bool isnan(QuantityPoint p) { return std::isnan(p.in(U{})); } +namespace detail { +// Some compilers do not support lambdas in constexpr contexts, so we make a manual function object. +struct StdMaxByValue { + template + constexpr auto operator()(T a, T b) const { + return std::max(a, b); + } +}; +} // namespace detail + // The maximum of two values of the same dimension. // // Unlike std::max, returns by value rather than by reference, because the types might differ. template -auto max(Quantity q1, Quantity q2) { - return detail::using_common_type(q1, q2, [](auto a, auto b) { return std::max(a, b); }); +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 -auto max(Quantity a, Quantity b) { +constexpr auto max(Quantity a, Quantity b) { return std::max(a, b); } @@ -336,13 +346,13 @@ auto max(Quantity a, Quantity b) { // // Unlike std::max, returns by value rather than by reference, because the types might differ. template -auto max(QuantityPoint p1, QuantityPoint p2) { - return detail::using_common_point_unit(p1, p2, [](auto a, auto b) { return std::max(a, b); }); +constexpr auto max(QuantityPoint p1, QuantityPoint p2) { + return detail::using_common_point_unit(p1, p2, detail::StdMaxByValue{}); } // Overload to resolve ambiguity with `std::max` for identical `QuantityPoint` types. template -auto max(QuantityPoint a, QuantityPoint b) { +constexpr auto max(QuantityPoint a, QuantityPoint b) { return std::max(a, b); } @@ -351,29 +361,39 @@ auto max(QuantityPoint a, QuantityPoint b) { // 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 -auto max(Zero z, T x) { +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 -auto max(T x, Zero z) { +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 { +// Some compilers do not support lambdas in constexpr contexts, so we make a manual function object. +struct StdMinByValue { + template + constexpr auto operator()(T a, T b) const { + return std::min(a, b); + } +}; +} // namespace detail + // The minimum of two values of the same dimension. // // Unlike std::min, returns by value rather than by reference, because the types might differ. template -auto min(Quantity q1, Quantity q2) { - return detail::using_common_type(q1, q2, [](auto a, auto b) { return std::min(a, b); }); +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 -auto min(Quantity a, Quantity b) { +constexpr auto min(Quantity a, Quantity b) { return std::min(a, b); } @@ -381,13 +401,13 @@ auto min(Quantity a, Quantity b) { // // Unlike std::min, returns by value rather than by reference, because the types might differ. template -auto min(QuantityPoint p1, QuantityPoint p2) { - return detail::using_common_point_unit(p1, p2, [](auto a, auto b) { return std::min(a, b); }); +constexpr auto min(QuantityPoint p1, QuantityPoint p2) { + return detail::using_common_point_unit(p1, p2, detail::StdMinByValue{}); } // Overload to resolve ambiguity with `std::min` for identical `QuantityPoint` types. template -auto min(QuantityPoint a, QuantityPoint b) { +constexpr auto min(QuantityPoint a, QuantityPoint b) { return std::min(a, b); } @@ -396,13 +416,13 @@ auto min(QuantityPoint a, QuantityPoint b) { // 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 -auto min(Zero z, T x) { +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 -auto min(T x, Zero z) { +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}); diff --git a/au/math_test.cc b/au/math_test.cc index d56e75ce..fa3dac52 100644 --- a/au/math_test.cc +++ b/au/math_test.cc @@ -300,12 +300,13 @@ TEST(remainder, CenteredAroundZero) { } TEST(max, ReturnsLarger) { - EXPECT_EQ(max(make_quantity>(1), make_quantity(1)), - make_quantity(1)); + constexpr auto result = max(centi(meters)(1), inches(1)); + EXPECT_EQ(result, inches(1)); } TEST(max, HandlesDifferentOriginQuantityPoints) { - EXPECT_EQ(max(fahrenheit_pt(30), celsius_pt(0)), celsius_pt(0)); + constexpr auto result = max(fahrenheit_pt(30), celsius_pt(0)); + EXPECT_EQ(result, celsius_pt(0)); } TEST(max, ReturnsByValueForSameExactQuantityType) { @@ -318,6 +319,11 @@ TEST(max, ReturnsByValueForSameExactQuantityType) { EXPECT_NE(&max_a_b, &b); } +TEST(max, SupportsConstexprForSameExactQuantityType) { + constexpr auto result = max(meters(1), meters(2)); + EXPECT_EQ(result, meters(2)); +} + TEST(max, ReturnsByValueForSameExactQuantityPointType) { // If two QuantityPoint types are EXACTLY the same, we risk ambiguity with `std::max`. const auto a = meters_pt(1); @@ -328,6 +334,11 @@ TEST(max, ReturnsByValueForSameExactQuantityPointType) { EXPECT_NE(&max_a_b, &b); } +TEST(max, SupportsConstexprForSameExactQuantityPointType) { + constexpr auto result = max(meters_pt(1), meters_pt(2)); + EXPECT_EQ(result, meters_pt(2)); +} + TEST(max, SameAsStdMaxForNumericTypes) { const auto a = 2; const auto b = 3; @@ -338,19 +349,29 @@ TEST(max, SameAsStdMaxForNumericTypes) { } TEST(max, SupportsZeroForFirstArgument) { - EXPECT_THAT(max(ZERO, meters(8)), SameTypeAndValue(meters(8))); - EXPECT_THAT(max(ZERO, meters(-8)), SameTypeAndValue(meters(0))); + constexpr auto positive_result = max(ZERO, meters(8)); + EXPECT_THAT(positive_result, SameTypeAndValue(meters(8))); + + constexpr auto negative_result = max(ZERO, meters(-8)); + EXPECT_THAT(negative_result, SameTypeAndValue(meters(0))); } TEST(max, SupportsZeroForSecondArgument) { - EXPECT_THAT(max(meters(8), ZERO), SameTypeAndValue(meters(8))); - EXPECT_THAT(max(meters(-8), ZERO), SameTypeAndValue(meters(0))); + constexpr auto positive_result = max(meters(8), ZERO); + EXPECT_THAT(positive_result, SameTypeAndValue(meters(8))); + + constexpr auto negative_result = max(meters(-8), ZERO); + EXPECT_THAT(negative_result, SameTypeAndValue(meters(0))); } -TEST(min, ReturnsSmaller) { EXPECT_EQ(min(centi(meters)(1), inches(1)), centi(meters)(1)); } +TEST(min, ReturnsSmaller) { + constexpr auto result = min(centi(meters)(1), inches(1)); + EXPECT_EQ(result, centi(meters)(1)); +} TEST(min, HandlesDifferentOriginQuantityPoints) { - EXPECT_EQ(min(fahrenheit_pt(30), celsius_pt(0)), fahrenheit_pt(30)); + constexpr auto result = min(fahrenheit_pt(30), celsius_pt(0)); + EXPECT_EQ(result, fahrenheit_pt(30)); } TEST(min, ReturnsByValueForSameExactQuantityType) { @@ -363,6 +384,11 @@ TEST(min, ReturnsByValueForSameExactQuantityType) { EXPECT_NE(&min_a_b, &a); } +TEST(min, SupportsConstexprForSameExactQuantityType) { + constexpr auto result = min(meters(1), meters(2)); + EXPECT_EQ(result, meters(1)); +} + TEST(min, ReturnsByValueForSameExactQuantityPointType) { // If two QuantityPoint types are EXACTLY the same, we risk ambiguity with `std::min`. const auto a = meters_pt(1); @@ -373,6 +399,11 @@ TEST(min, ReturnsByValueForSameExactQuantityPointType) { EXPECT_NE(&min_a_b, &a); } +TEST(min, SupportsConstexprForSameExactQuantityPointType) { + constexpr auto result = min(meters_pt(1), meters_pt(2)); + EXPECT_EQ(result, meters_pt(1)); +} + TEST(min, SameAsStdMinForNumericTypes) { const auto a = 2; const auto b = 3; @@ -383,13 +414,19 @@ TEST(min, SameAsStdMinForNumericTypes) { } TEST(min, SupportsZeroForFirstArgument) { - EXPECT_THAT(min(ZERO, meters(8)), SameTypeAndValue(meters(0))); - EXPECT_THAT(min(ZERO, meters(-8)), SameTypeAndValue(meters(-8))); + constexpr auto positive_result = min(ZERO, meters(8)); + EXPECT_THAT(positive_result, SameTypeAndValue(meters(0))); + + constexpr auto negative_result = min(ZERO, meters(-8)); + EXPECT_THAT(negative_result, SameTypeAndValue(meters(-8))); } TEST(min, SupportsZeroForSecondArgument) { - EXPECT_THAT(min(meters(8), ZERO), SameTypeAndValue(meters(0))); - EXPECT_THAT(min(meters(-8), ZERO), SameTypeAndValue(meters(-8))); + constexpr auto positive_result = min(meters(8), ZERO); + EXPECT_THAT(positive_result, SameTypeAndValue(meters(0))); + + constexpr auto negative_result = min(meters(-8), ZERO); + EXPECT_THAT(negative_result, SameTypeAndValue(meters(-8))); } TEST(int_pow, OutputRepMatchesInputRep) {