fix indexed crash for histograms with axes of zero size (physical or logical) (#356)

include/boost/histogram/indexed.hpp

@@ -286,7 +286,7 @@ public:
   private:
     iterator(value_iterator i, histogram_type& h) : iter_(i), indices_(&h) {}
 
-    value_iterator iter_;
+    value_iterator iter_; // original histogram iterator
 
     struct index_data {
       axis::index_type idx, begin, end;
@@ -319,6 +319,9 @@ public:
   template <class Iterable, class = detail::requires_iterable<Iterable>>
   indexed_range(histogram_type& hist, Iterable&& range)
       : begin_(hist.begin(), hist), end_(hist.end(), hist) {
+    // if histogram is empty, incrementing begin_.iter_ may be undefined behavior
+    if (begin_ == end_) return;
+
     auto r_begin = std::begin(range);
     assert(std::distance(r_begin, std::end(range)) == static_cast<int>(hist.rank()));
 
@@ -339,12 +342,20 @@ public:
       ca->begin_skip = static_cast<std::size_t>(ca->begin - start) * stride;
       ca->end_skip = static_cast<std::size_t>(stop - ca->end) * stride;
       begin_.iter_ += ca->begin_skip;
+      end_.iter_ -= ca->end_skip;
 
       stride *= stop - start;
 
       ++ca;
       ++r_begin;
     });
+    // check if selected range is empty
+    if (end_.iter_ < begin_.iter_) {
+      begin_ = end_;
+    } else {
+      // reset end_ to hist.end(), since end_skips are done in operator++
+      end_.iter_ = hist.end();
+    }
   }
 
   iterator begin() noexcept { return begin_; }
@@ -358,8 +369,8 @@ private:
       (*it)[0] = 0;
       (*it)[1] = a.size();
       if (cov == coverage::all) {
-        (*it)[0] -= 1;
-        (*it)[1] += 1;
+        (*it)[0] -= 1; // making this wider than actual range is safe
+        (*it)[1] += 1; // making this wider than actual range is safe
       } else
         assert(cov == coverage::inner);
       ++it;

test/CMakeLists.txt

@@ -89,6 +89,7 @@ boost_test(TYPE run SOURCES storage_adaptor_test.cpp)
 boost_test(TYPE run SOURCES unlimited_storage_test.cpp)
 boost_test(TYPE run SOURCES utility_test.cpp)
 boost_test(TYPE run SOURCES issue_327_test.cpp)
+boost_test(TYPE run SOURCES issue_353_test.cpp)
 
 find_package(Threads)
 if (Threads_FOUND)

test/Jamfile

@@ -88,6 +88,7 @@ alias cxx14 :
     [ run unlimited_storage_test.cpp ]
     [ run utility_test.cpp ]
     [ run issue_327_test.cpp ]
+    [ run issue_353_test.cpp ]
     ;
 
 alias cxx17 :

test/issue_353_test.cpp

@@ -0,0 +1,48 @@
+#include <boost/core/lightweight_test.hpp>
+#include <boost/histogram.hpp>
+#include <vector>
+#include "throw_exception.hpp"
+
+namespace bh = boost::histogram;
+
+struct empty {
+  int index(double) { return 0; }
+  int size() const { return 0; }
+};
+
+int main() {
+  {
+    auto h =
+        bh::make_histogram_with(std::vector<int>(), bh::axis::integer<>(0, 2), empty());
+
+    auto ind1 = bh::indexed(h, bh::coverage::all);
+    BOOST_TEST_EQ(std::distance(ind1.begin(), ind1.end()), 0);
+
+    auto ind2 = bh::indexed(h, bh::coverage::inner);
+    BOOST_TEST_EQ(std::distance(ind2.begin(), ind2.end()), 0);
+  }
+
+  {
+    auto h = bh::make_histogram_with(std::vector<int>(), bh::axis::integer<>(0, 2),
+                                     bh::axis::integer<>(0, 1));
+
+    auto ind1 = bh::indexed(h, bh::coverage::all);
+    BOOST_TEST_EQ(std::distance(ind1.begin(), ind1.end()), 12);
+
+    auto ind2 = bh::indexed(h, bh::coverage::inner);
+    BOOST_TEST_EQ(std::distance(ind2.begin(), ind2.end()), 2);
+  }
+
+  {
+    auto h = bh::make_histogram_with(std::vector<int>(), bh::axis::integer<>(0, 2),
+                                     bh::axis::integer<>(0, 0));
+
+    auto ind1 = bh::indexed(h, bh::coverage::all);
+    BOOST_TEST_EQ(std::distance(ind1.begin(), ind1.end()), 8);
+
+    auto ind2 = bh::indexed(h, bh::coverage::inner);
+    BOOST_TEST_EQ(std::distance(ind2.begin(), ind2.end()), 0);
+  }
+
+  return boost::report_errors();
+}