diff --git a/include/boost/histogram/detail/detect.hpp b/include/boost/histogram/detail/detect.hpp index d62ab4dd..f08b79f4 100644 --- a/include/boost/histogram/detail/detect.hpp +++ b/include/boost/histogram/detail/detect.hpp @@ -111,6 +111,16 @@ BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_operator_rdiv, (t /= u)); BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_eq, (cref().operator==(u))); +BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_ne, (cref().operator!=(u))); + +BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_lt, (cref().operator<(u))); + +BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_gt, (cref().operator>(u))); + +BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_le, (cref().operator<=(u))); + +BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_ge, (cref().operator>=(u))); + BOOST_HISTOGRAM_DETAIL_DETECT(has_threading_support, (T::has_threading_support)); // stronger form of std::is_convertible that works with explicit operator T and ctors diff --git a/include/boost/histogram/detail/operators.hpp b/include/boost/histogram/detail/operators.hpp index dead8902..16a54780 100644 --- a/include/boost/histogram/detail/operators.hpp +++ b/include/boost/histogram/detail/operators.hpp @@ -20,10 +20,11 @@ namespace detail { template using if_not_same = std::enable_if_t<(!std::is_same::value), bool>; -// template -// using if_not_same_and_has_eq = -// std::enable_if_t<(!std::is_same::value && !has_method_eq::value), -// bool>; +// A generic template operator must yield to U's own member operator when U has one that +// accepts T, otherwise the two catch-all templates are ambiguous (issue #403). +template +using if_not_same_and_no_method = + std::enable_if_t<(!std::is_same::value && !HasMethod::value), bool>; // totally_ordered is for types with a <= b == !(a > b) [floats with NaN violate this] // Derived must implement <,== for symmetric form and <,>,== for non-symmetric. @@ -44,11 +45,13 @@ struct mirrored { template struct mirrored { template - friend if_not_same operator<(const U& a, const T& b) noexcept { + friend if_not_same_and_no_method> operator<( + const U& a, const T& b) noexcept { return b > a; } template - friend if_not_same operator>(const U& a, const T& b) noexcept { + friend if_not_same_and_no_method> operator>( + const U& a, const T& b) noexcept { return b < a; } template @@ -57,15 +60,18 @@ struct mirrored { return b.operator==(a); } template - friend if_not_same operator<=(const U& a, const T& b) noexcept { + friend if_not_same_and_no_method> operator<=( + const U& a, const T& b) noexcept { return b >= a; } template - friend if_not_same operator>=(const U& a, const T& b) noexcept { + friend if_not_same_and_no_method> operator>=( + const U& a, const T& b) noexcept { return b <= a; } template - friend if_not_same operator!=(const U& a, const T& b) noexcept { + friend if_not_same_and_no_method> operator!=( + const U& a, const T& b) noexcept { return b != a; } }; @@ -83,7 +89,8 @@ struct equality { template struct equality { template - friend if_not_same operator!=(const T& a, const U& b) noexcept { + friend if_not_same_and_no_method> operator!=( + const T& a, const U& b) noexcept { return !(a == b); } }; @@ -97,18 +104,20 @@ struct totally_ordered_impl : equality, mirrored { template struct totally_ordered_impl : equality, mirrored { template - friend if_not_same operator<=(const T& a, const U& b) noexcept { + friend if_not_same_and_no_method> operator<=( + const T& a, const U& b) noexcept { return !(a > b); } template - friend if_not_same operator>=(const T& a, const U& b) noexcept { + friend if_not_same_and_no_method> operator>=( + const T& a, const U& b) noexcept { return !(a < b); } }; template using totally_ordered = mp11::mp_rename< - mp11::mp_product, mp11::mp_list >, + mp11::mp_product, mp11::mp_list>, mp11::mp_inherit>; template @@ -120,18 +129,20 @@ struct partially_ordered_impl : equality, mirrored { template struct partially_ordered_impl : equality, mirrored { template - friend if_not_same operator<=(const T& a, const U& b) noexcept { + friend if_not_same_and_no_method> operator<=( + const T& a, const U& b) noexcept { return a < b || a == b; } template - friend if_not_same operator>=(const T& a, const U& b) noexcept { + friend if_not_same_and_no_method> operator>=( + const T& a, const U& b) noexcept { return a > b || a == b; } }; template using partially_ordered = mp11::mp_rename< - mp11::mp_product, mp11::mp_list >, + mp11::mp_product, mp11::mp_list>, mp11::mp_inherit>; } // namespace detail diff --git a/include/boost/histogram/indexed.hpp b/include/boost/histogram/indexed.hpp index 66690e60..ccf92345 100644 --- a/include/boost/histogram/indexed.hpp +++ b/include/boost/histogram/indexed.hpp @@ -179,12 +179,12 @@ class BOOST_ATTRIBUTE_NODISCARD indexed_range { } // forward all comparison operators to the value - bool operator<(const accessor& o) noexcept { return get() < o.get(); } - bool operator>(const accessor& o) noexcept { return get() > o.get(); } - bool operator==(const accessor& o) noexcept { return get() == o.get(); } - bool operator!=(const accessor& o) noexcept { return get() != o.get(); } - bool operator<=(const accessor& o) noexcept { return get() <= o.get(); } - bool operator>=(const accessor& o) noexcept { return get() >= o.get(); } + bool operator<(const accessor& o) const noexcept { return get() < o.get(); } + bool operator>(const accessor& o) const noexcept { return get() > o.get(); } + bool operator==(const accessor& o) const noexcept { return get() == o.get(); } + bool operator!=(const accessor& o) const noexcept { return get() != o.get(); } + bool operator<=(const accessor& o) const noexcept { return get() <= o.get(); } + bool operator>=(const accessor& o) const noexcept { return get() >= o.get(); } template bool operator<(const U& o) const noexcept { diff --git a/test/indexed_test.cpp b/test/indexed_test.cpp index 8ddbadff..4f183d46 100644 --- a/test/indexed_test.cpp +++ b/test/indexed_test.cpp @@ -156,6 +156,60 @@ void run_stdlib_tests(mp_list) { } } +// regression test for https://github.com/boostorg/histogram/issues/403; with the +// default storage, comparisons of accessors and cell references were ambiguous +template +void run_comparison_tests(Tag) { + auto h = make(Tag(), axis::integer<>(0, 2), axis::integer<>(0, 2)); + h(0, 0); + h(1, 1); + h(1, 1); + + auto ind = indexed(h); + + // libc++ passes accessors as const references to the comparator + auto imin = std::min_element(ind.begin(), ind.end()); + BOOST_TEST_EQ(**imin, 0); + auto imax = std::max_element(ind.begin(), ind.end()); + BOOST_TEST_EQ(**imax, 2); + + auto it = ind.begin(); // points to cell (0, 0) with value 1 + auto&& a = *it; + const auto& ca = a; + auto&& r = h.at(1, 1); // value 2 + + BOOST_TEST(a < r); + BOOST_TEST(a <= r); + BOOST_TEST(a != r); + BOOST_TEST(!(a > r)); + BOOST_TEST(!(a >= r)); + BOOST_TEST(!(a == r)); + + BOOST_TEST(r > a); + BOOST_TEST(r >= a); + BOOST_TEST(r != a); + BOOST_TEST(!(r < a)); + BOOST_TEST(!(r <= a)); + BOOST_TEST(!(r == a)); + + BOOST_TEST(ca == ca); + BOOST_TEST(ca <= ca); + BOOST_TEST(ca >= ca); + BOOST_TEST(!(ca != ca)); + BOOST_TEST(!(ca < ca)); + BOOST_TEST(!(ca > ca)); + BOOST_TEST(ca < r); + + BOOST_TEST(a == 1); + BOOST_TEST(1 == a); + BOOST_TEST(a != 2); + BOOST_TEST(2 != a); + BOOST_TEST(a < 2); + BOOST_TEST(2 > a); + BOOST_TEST(a <= 1); + BOOST_TEST(1 >= a); +} + template void run_indexed_with_range_tests(Tag) { { @@ -201,6 +255,9 @@ int main() { run_stdlib_tests(x); }); + run_comparison_tests(static_tag{}); + run_comparison_tests(dynamic_tag{}); + run_indexed_with_range_tests(static_tag{}); run_indexed_with_range_tests(dynamic_tag{}); return boost::report_errors();