fix: ambiguous comparison between indexed accessor and cell reference

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<T>().operator==(u)));
 
+BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_ne, (cref<T>().operator!=(u)));
+
+BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_lt, (cref<T>().operator<(u)));
+
+BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_gt, (cref<T>().operator>(u)));
+
+BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_le, (cref<T>().operator<=(u)));
+
+BOOST_HISTOGRAM_DETAIL_DETECT_BINARY(has_method_ge, (cref<T>().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

include/boost/histogram/detail/operators.hpp

@@ -20,10 +20,11 @@ namespace detail {
 template <class T, class U>
 using if_not_same = std::enable_if_t<(!std::is_same<T, U>::value), bool>;
 
-// template <class T, class U>
-// using if_not_same_and_has_eq =
-//     std::enable_if_t<(!std::is_same<T, U>::value && !has_method_eq<T, U>::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 <class T, class U, class HasMethod>
+using if_not_same_and_no_method =
+    std::enable_if_t<(!std::is_same<T, U>::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 <class T>
 struct mirrored<T, void> {
   template <class U>
-  friend if_not_same<T, U> operator<(const U& a, const T& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_lt<U, T>> operator<(
+      const U& a, const T& b) noexcept {
     return b > a;
   }
   template <class U>
-  friend if_not_same<T, U> operator>(const U& a, const T& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_gt<U, T>> operator>(
+      const U& a, const T& b) noexcept {
     return b < a;
   }
   template <class U>
@@ -57,15 +60,18 @@ struct mirrored<T, void> {
     return b.operator==(a);
   }
   template <class U>
-  friend if_not_same<T, U> operator<=(const U& a, const T& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_le<U, T>> operator<=(
+      const U& a, const T& b) noexcept {
     return b >= a;
   }
   template <class U>
-  friend if_not_same<T, U> operator>=(const U& a, const T& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_ge<U, T>> operator>=(
+      const U& a, const T& b) noexcept {
     return b <= a;
   }
   template <class U>
-  friend if_not_same<T, U> operator!=(const U& a, const T& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_ne<U, T>> operator!=(
+      const U& a, const T& b) noexcept {
     return b != a;
   }
 };
@@ -83,7 +89,8 @@ struct equality {
 template <class T>
 struct equality<T, void> {
   template <class U>
-  friend if_not_same<T, U> operator!=(const T& a, const U& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_ne<U, T>> operator!=(
+      const T& a, const U& b) noexcept {
     return !(a == b);
   }
 };
@@ -97,18 +104,20 @@ struct totally_ordered_impl : equality<T, U>, mirrored<T, U> {
 template <class T>
 struct totally_ordered_impl<T, void> : equality<T, void>, mirrored<T, void> {
   template <class U>
-  friend if_not_same<T, U> operator<=(const T& a, const U& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_le<U, T>> operator<=(
+      const T& a, const U& b) noexcept {
     return !(a > b);
   }
   template <class U>
-  friend if_not_same<T, U> operator>=(const T& a, const U& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_ge<U, T>> operator>=(
+      const T& a, const U& b) noexcept {
     return !(a < b);
   }
 };
 
 template <class T, class... Ts>
 using totally_ordered = mp11::mp_rename<
-    mp11::mp_product<totally_ordered_impl, mp11::mp_list<T>, mp11::mp_list<Ts...> >,
+    mp11::mp_product<totally_ordered_impl, mp11::mp_list<T>, mp11::mp_list<Ts...>>,
     mp11::mp_inherit>;
 
 template <class T, class U>
@@ -120,18 +129,20 @@ struct partially_ordered_impl : equality<T, U>, mirrored<T, U> {
 template <class T>
 struct partially_ordered_impl<T, void> : equality<T, void>, mirrored<T, void> {
   template <class U>
-  friend if_not_same<T, U> operator<=(const T& a, const U& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_le<U, T>> operator<=(
+      const T& a, const U& b) noexcept {
     return a < b || a == b;
   }
   template <class U>
-  friend if_not_same<T, U> operator>=(const T& a, const U& b) noexcept {
+  friend if_not_same_and_no_method<T, U, has_method_ge<U, T>> operator>=(
+      const T& a, const U& b) noexcept {
     return a > b || a == b;
   }
 };
 
 template <class T, class... Ts>
 using partially_ordered = mp11::mp_rename<
-    mp11::mp_product<partially_ordered_impl, mp11::mp_list<T>, mp11::mp_list<Ts...> >,
+    mp11::mp_product<partially_ordered_impl, mp11::mp_list<T>, mp11::mp_list<Ts...>>,
     mp11::mp_inherit>;
 
 } // namespace detail

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 <class U>
     bool operator<(const U& o) const noexcept {

test/indexed_test.cpp

@@ -156,6 +156,60 @@ void run_stdlib_tests(mp_list<Tag, Coverage>) {
   }
 }
 
+// regression test for https://github.com/boostorg/histogram/issues/403; with the
+// default storage, comparisons of accessors and cell references were ambiguous
+template <class Tag>
+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 <class Tag>
 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();