Skip to content

Commit

Permalink
Support constexpr in min and max (#251)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@chiphogg
chiphogg authored Jun 24, 2024
1 parent 50de6e9 commit 32ae57d
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 29 deletions.
52 changes: 36 additions & 16 deletions au/math.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,31 +318,41 @@ constexpr bool isnan(QuantityPoint<U, R> 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 <typename T>
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 <typename U1, typename U2, typename R1, typename R2>
auto max(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {
return detail::using_common_type(q1, q2, [](auto a, auto b) { return std::max(a, b); });
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>
auto max(Quantity<U, R> a, Quantity<U, R> b) {
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.
template <typename U1, typename U2, typename R1, typename R2>
auto max(QuantityPoint<U1, R1> p1, QuantityPoint<U2, R2> p2) {
return detail::using_common_point_unit(p1, p2, [](auto a, auto b) { return std::max(a, b); });
constexpr auto max(QuantityPoint<U1, R1> p1, QuantityPoint<U2, R2> p2) {
return detail::using_common_point_unit(p1, p2, detail::StdMaxByValue{});
}

// Overload to resolve ambiguity with `std::max` for identical `QuantityPoint` types.
template <typename U, typename R>
auto max(QuantityPoint<U, R> a, QuantityPoint<U, R> b) {
constexpr auto max(QuantityPoint<U, R> a, QuantityPoint<U, R> b) {
return std::max(a, b);
}

Expand All @@ -351,43 +361,53 @@ auto max(QuantityPoint<U, R> a, QuantityPoint<U, R> 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 <typename T>
auto max(Zero z, T x) {
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>
auto max(T x, Zero z) {
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 {
// Some compilers do not support lambdas in constexpr contexts, so we make a manual function object.
struct StdMinByValue {
template <typename T>
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 <typename U1, typename U2, typename R1, typename R2>
auto min(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {
return detail::using_common_type(q1, q2, [](auto a, auto b) { return std::min(a, b); });
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>
auto min(Quantity<U, R> a, Quantity<U, R> b) {
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.
template <typename U1, typename U2, typename R1, typename R2>
auto min(QuantityPoint<U1, R1> p1, QuantityPoint<U2, R2> p2) {
return detail::using_common_point_unit(p1, p2, [](auto a, auto b) { return std::min(a, b); });
constexpr auto min(QuantityPoint<U1, R1> p1, QuantityPoint<U2, R2> p2) {
return detail::using_common_point_unit(p1, p2, detail::StdMinByValue{});
}

// Overload to resolve ambiguity with `std::min` for identical `QuantityPoint` types.
template <typename U, typename R>
auto min(QuantityPoint<U, R> a, QuantityPoint<U, R> b) {
constexpr auto min(QuantityPoint<U, R> a, QuantityPoint<U, R> b) {
return std::min(a, b);
}

Expand All @@ -396,13 +416,13 @@ auto min(QuantityPoint<U, R> a, QuantityPoint<U, R> 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 <typename T>
auto min(Zero z, T x) {
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>
auto min(T x, Zero z) {
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});
Expand Down
63 changes: 50 additions & 13 deletions au/math_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,13 @@ TEST(remainder, CenteredAroundZero) {
}

TEST(max, ReturnsLarger) {
EXPECT_EQ(max(make_quantity<Centi<Meters>>(1), make_quantity<Inches>(1)),
make_quantity<Inches>(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) {
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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) {
Expand Down

0 comments on commit 32ae57d

Please sign in to comment.