fix inclusive() in integer axis (#320)

It was possible to trigger this bug, which triggered an assertion in debug mode. In release mode, it would have probably caused a segfault.

Author: HDembinski

include/boost/histogram/axis/integer.hpp

@@ -153,9 +153,14 @@ class integer : public iterator_mixin<integer<Value, MetaData, Options>>,
 
   /// Whether the axis is inclusive (see axis::traits::is_inclusive).
   static constexpr bool inclusive() noexcept {
-    return (options() & option::underflow || options() & option::overflow) ||
-           (std::is_integral<value_type>::value &&
-            (options() & (option::growth | option::circular)));
+    // If axis has underflow and overflow, it is inclusive.
+    // If axis is growing or circular:
+    // - it is inclusive if value_type is int.
+    // - it is not inclusive if value_type is float, because of nan and inf.
+    constexpr bool full_flow =
+        options() & option::underflow && options() & option::overflow;
+    return full_flow || (std::is_integral<value_type>::value &&
+                         (options() & (option::growth | option::circular)));
   }
 
   template <class V, class M, class O>
@@ -205,12 +210,12 @@ class integer : public iterator_mixin<integer<Value, MetaData, Options>>,
 #if __cpp_deduction_guides >= 201606
 
 template <class T>
-integer(T, T)->integer<detail::convert_integer<T, index_type>, null_type>;
+integer(T, T) -> integer<detail::convert_integer<T, index_type>, null_type>;
 
 template <class T, class M>
 integer(T, T, M)
-    ->integer<detail::convert_integer<T, index_type>,
-              detail::replace_type<std::decay_t<M>, const char*, std::string>>;
+    -> integer<detail::convert_integer<T, index_type>,
+               detail::replace_type<std::decay_t<M>, const char*, std::string>>;
 
 #endif
 

include/boost/histogram/detail/linearize.hpp

@@ -19,30 +19,34 @@ namespace boost {
 namespace histogram {
 namespace detail {
 
-// initial offset to out must be set
-template <class Index, class Opts>
-std::size_t linearize(Opts, Index& out, const std::size_t stride,
+// initial offset to out must be set;
+// this faster code can be used if all axes are inclusive
+template <class Opts>
+std::size_t linearize(Opts, std::size_t& out, const std::size_t stride,
                       const axis::index_type size, const axis::index_type idx) {
   constexpr bool u = Opts::test(axis::option::underflow);
   constexpr bool o = Opts::test(axis::option::overflow);
+  assert(idx >= (u ? -1 : 0));
+  assert(idx < (o ? size + 1 : size));
+  assert(idx >= 0 || static_cast<std::size_t>(-idx * stride) <= out);
+  out += idx * stride;
+  return size + u + o;
+}
 
-  // must be non-const to avoid if constexpr warning from msvc
-  bool fast_track = std::is_same<Index, std::size_t>::value || (u && o);
-  if (fast_track) {
-    assert(idx >= (u ? -1 : 0));
-    assert(idx < (o ? size + 1 : size));
-    assert(idx >= 0 || static_cast<std::size_t>(-idx * stride) <= out);
+// initial offset to out must be set
+// this slower code must be used if not all axes are inclusive
+template <class Opts>
+std::size_t linearize(Opts, optional_index& out, const std::size_t stride,
+                      const axis::index_type size, const axis::index_type idx) {
+  constexpr bool u = Opts::test(axis::option::underflow);
+  constexpr bool o = Opts::test(axis::option::overflow);
+  assert(idx >= -1);
+  assert(idx < size + 1);
+  const bool is_valid = (u || idx >= 0) && (o || idx < size);
+  if (is_valid)
     out += idx * stride;
-  } else {
-    assert(idx >= -1);
-    assert(idx < size + 1);
-    // must be non-const to avoid if constexpr warning from msvc
-    bool is_valid = (u || idx >= 0) && (o || idx < size);
-    if (is_valid)
-      out += idx * stride;
-    else
-      out = invalid_index;
-  }
+  else
+    out = invalid_index;
   return size + u + o;
 }
 

test/axis_traits_test.cpp

@@ -140,18 +140,34 @@ int main() {
         (traits::is_inclusive<integer<int, boost::use_default, option::growth_t>>));
     BOOST_TEST_TRAIT_TRUE(
         (traits::is_inclusive<integer<int, boost::use_default, option::circular_t>>));
+    BOOST_TEST_TRAIT_FALSE(
+        (traits::is_inclusive<integer<int, boost::use_default, option::underflow_t>>));
+    BOOST_TEST_TRAIT_FALSE(
+        (traits::is_inclusive<integer<int, boost::use_default, option::overflow_t>>));
+    BOOST_TEST_TRAIT_TRUE(
+        (traits::is_inclusive<integer<int, boost::use_default,
+                                      decltype(option::underflow | option::overflow)>>));
 
     BOOST_TEST_TRAIT_TRUE((traits::is_inclusive<integer<double>>));
     BOOST_TEST_TRAIT_FALSE(
         (traits::is_inclusive<integer<double, boost::use_default, option::growth_t>>));
     BOOST_TEST_TRAIT_FALSE(
         (traits::is_inclusive<integer<double, boost::use_default, option::circular_t>>));
+    BOOST_TEST_TRAIT_FALSE(
+        (traits::is_inclusive<integer<double, boost::use_default, option::underflow_t>>));
+    BOOST_TEST_TRAIT_FALSE(
+        (traits::is_inclusive<integer<double, boost::use_default, option::overflow_t>>));
+    BOOST_TEST_TRAIT_TRUE(
+        (traits::is_inclusive<integer<double, boost::use_default,
+                                      decltype(option::underflow | option::overflow)>>));
 
     BOOST_TEST_TRAIT_TRUE((traits::is_inclusive<category<int>>));
     BOOST_TEST_TRAIT_TRUE(
         (traits::is_inclusive<category<int, boost::use_default, option::growth_t>>));
     BOOST_TEST_TRAIT_FALSE(
         (traits::is_inclusive<category<int, boost::use_default, option::none_t>>));
+    BOOST_TEST_TRAIT_TRUE(
+        (traits::is_inclusive<category<int, boost::use_default, option::overflow_t>>));
   }
 
   // is_ordered, ordered()

test/histogram_test.cpp

@@ -216,6 +216,20 @@ void run_tests() {
     BOOST_TEST_EQ(h.at(1), 0);
   }
 
+  // 1D without underflow
+  {
+    using opt = axis::option::overflow_t;
+    auto h = make(Tag(), axis::integer<int, axis::null_type, opt>{1, 3});
+
+    int x[] = {-1, 0, 1, 2, 3, 4, 5};
+    for (auto&& xi : x) h(xi);
+
+    BOOST_TEST_EQ(algorithm::sum(h), 5);
+    BOOST_TEST_EQ(h.at(0), 1);
+    BOOST_TEST_EQ(h.at(1), 1);
+    BOOST_TEST_EQ(h.at(2), 3);
+  }
+
   // 1D category axis
   {
     auto h = make(Tag(), axis::category<>({1, 2}));