diff --git a/include/boost/histogram/detail/axes.hpp b/include/boost/histogram/detail/axes.hpp index 9058ce17..f0da6834 100644 --- a/include/boost/histogram/detail/axes.hpp +++ b/include/boost/histogram/detail/axes.hpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -577,12 +578,18 @@ optional_index call_impl(dynamic_container_tag, const dynamic_axes& axes, return i; } +/* In all at_impl, we throw instead of asserting when an index is out of + * bounds, because wrapping code cannot check this condition without spending + * a lot of extra cycles. For the wrapping code it is much easier to catch + * the exception and do something sensible. + */ + template std::size_t at_impl(detail::no_container_tag, const A& axes, const Us&... us) { dimension_check(axes, mp11::mp_size_t()); auto index = detail::optional_index(); detail::indices_to_index<0>(index, axes, static_cast(us)...); - BOOST_ASSERT_MSG(index, "indices out of bounds"); + if (!index) throw std::out_of_range("indices out of bounds"); return *index; } @@ -591,7 +598,7 @@ std::size_t at_impl(detail::static_container_tag, const A& axes, const U& u) { dimension_check(axes, mp_size()); auto index = detail::optional_index(); detail::indices_to_index_get(mp_size(), index, axes, u); - BOOST_ASSERT_MSG(index, "indices out of bounds"); + if (!index) throw std::out_of_range("indices out of bounds"); return *index; } @@ -602,7 +609,7 @@ std::size_t at_impl(detail::dynamic_container_tag, const static_axes& axe auto index = detail::optional_index(); detail::indices_to_index_iter(mp11::mp_size_t(), index, axes, std::begin(u)); - BOOST_ASSERT_MSG(index, "indices out of bounds"); + if (!index) throw std::out_of_range("indices out of bounds"); return *index; } @@ -612,7 +619,7 @@ std::size_t at_impl(detail::dynamic_container_tag, const dynamic_axes& ax dimension_check(axes, std::distance(std::begin(u), std::end(u))); auto index = detail::optional_index(); detail::indices_to_index_iter(index, axes, std::begin(u)); - BOOST_ASSERT_MSG(index, "indices out of bounds"); + if (!index) throw std::out_of_range("indices out of bounds"); return *index; } diff --git a/include/boost/histogram/histogram.hpp b/include/boost/histogram/histogram.hpp index aaf3b342..bfd1de50 100644 --- a/include/boost/histogram/histogram.hpp +++ b/include/boost/histogram/histogram.hpp @@ -7,6 +7,7 @@ #ifndef _BOOST_HISTOGRAM_HISTOGRAM_HPP_ #define _BOOST_HISTOGRAM_HISTOGRAM_HPP_ +#include #include #include #include @@ -18,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -213,18 +215,21 @@ public: return hr; } + // precondition: range represents sequence of strictly ascending axis indices template , typename = detail::requires_iterator> histogram reduce_to(Iterator begin, Iterator end) const { - std::set indices(begin, end); - BOOST_ASSERT_MSG(*indices.rbegin() < dim(), "index out of range"); + BOOST_ASSERT_MSG(std::is_sorted(begin, end, std::less_equal()), + "integer sequence must be strictly ascending"); + BOOST_ASSERT_MSG(begin == end || static_cast(*(end - 1)) < dim(), + "index out of range"); auto sub_axes = histogram::axes_type(axes_.get_allocator()); - sub_axes.reserve(indices.size()); + sub_axes.reserve(std::distance(begin, end)); auto b = std::vector(dim(), false); - for (auto k : indices) { - sub_axes.push_back(axes_[k]); - b[k] = true; + for (auto it = begin; it != end; ++it) { + sub_axes.push_back(axes_[*it]); + b[*it] = true; } auto hr = histogram(std::move(sub_axes), storage_type(storage_.get_allocator())); std::vector shape(dim()); diff --git a/src/python/axis.cpp b/src/python/axis.cpp index 4d3ab020..85e0c04b 100644 --- a/src/python/axis.cpp +++ b/src/python/axis.cpp @@ -146,7 +146,7 @@ bp::str axis_get_label(const T& t) { template bp::object axis_getitem(const A& a, int i) { - if (i < -1 * a.uoflow() || i >= a.size() + 1 * a.uoflow()) + if (i < -(a.uoflow() == 2) || i >= (a.size() + int(a.uoflow() > 0))) throw std::out_of_range("index out of bounds"); return bp::make_tuple(a.lower(i), a.lower(i + 1)); } diff --git a/src/python/histogram.cpp b/src/python/histogram.cpp index cbeb95d8..814339a5 100644 --- a/src/python/histogram.cpp +++ b/src/python/histogram.cpp @@ -316,9 +316,6 @@ bp::object histogram_reduce_to(bp::tuple args, bp::dict kwargs) { const auto nargs = bp::len(args) - 1; if (nargs == 0) { throw std::invalid_argument("at least one argument required"); } - if (nargs < self.dim()) { - throw std::invalid_argument("fewer arguments than histogram axes required"); - } if (nargs > BOOST_HISTOGRAM_AXIS_LIMIT) { throw std::invalid_argument( @@ -327,7 +324,11 @@ bp::object histogram_reduce_to(bp::tuple args, bp::dict kwargs) { } int idx[BOOST_HISTOGRAM_AXIS_LIMIT]; - for (auto i = 0u; i < nargs; ++i) idx[i] = bp::extract(args[1 + i]); + for (auto i = 0u; i < nargs; ++i) { + idx[i] = bp::extract(args[1 + i]); + if (i > 0 && idx[i] <= idx[i - 1]) + throw std::invalid_argument("indices must be strictly ascending"); + } return bp::object(self.reduce_to(idx, idx + nargs)); } diff --git a/src/python/module.cpp b/src/python/module.cpp index c57ea2e2..8639b725 100644 --- a/src/python/module.cpp +++ b/src/python/module.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #ifdef HAVE_NUMPY #include #endif @@ -16,6 +17,10 @@ void register_histogram(); BOOST_PYTHON_MODULE(histogram) { using namespace boost::python; + + register_exception_translator( + [](const std::out_of_range& e) { PyErr_SetString(PyExc_IndexError, e.what()); }); + scope current; #ifdef HAVE_NUMPY numpy::initialize(); diff --git a/test/fail_histogram_dynamic_bin_1_test.cpp b/test/fail_histogram_dynamic_at_tuple_wrong_dimension_test.cpp similarity index 73% rename from test/fail_histogram_dynamic_bin_1_test.cpp rename to test/fail_histogram_dynamic_at_tuple_wrong_dimension_test.cpp index 4a86e789..c5389157 100644 --- a/test/fail_histogram_dynamic_bin_1_test.cpp +++ b/test/fail_histogram_dynamic_at_tuple_wrong_dimension_test.cpp @@ -1,7 +1,8 @@ #include +#include using namespace boost::histogram; int main() { auto h = make_dynamic_histogram(axis::integer<>(0, 2)); - h.at(-2); + h.at(std::make_pair(0, 0)); } diff --git a/test/fail_histogram_dynamic_at_vector_wrong_dimension_test.cpp b/test/fail_histogram_dynamic_at_vector_wrong_dimension_test.cpp new file mode 100644 index 00000000..bb5eb0bc --- /dev/null +++ b/test/fail_histogram_dynamic_at_vector_wrong_dimension_test.cpp @@ -0,0 +1,8 @@ +#include +#include + +using namespace boost::histogram; +int main() { + auto h = make_dynamic_histogram(axis::integer<>(0, 2)); + h.at(std::vector({0, 0})); +} diff --git a/test/fail_histogram_dynamic_bin_0_test.cpp b/test/fail_histogram_dynamic_at_wrong_dimension_test.cpp similarity index 100% rename from test/fail_histogram_dynamic_bin_0_test.cpp rename to test/fail_histogram_dynamic_at_wrong_dimension_test.cpp diff --git a/test/fail_histogram_dynamic_bin_2_test.cpp b/test/fail_histogram_dynamic_bin_2_test.cpp deleted file mode 100644 index 9993558a..00000000 --- a/test/fail_histogram_dynamic_bin_2_test.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include -#include - -using namespace boost::histogram; -int main() { - auto h = - make_dynamic_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(std::make_pair(-2, 0)); -} diff --git a/test/fail_histogram_dynamic_bin_3_test.cpp b/test/fail_histogram_dynamic_bin_3_test.cpp deleted file mode 100644 index 26a5f896..00000000 --- a/test/fail_histogram_dynamic_bin_3_test.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include -#include - -using namespace boost::histogram; -int main() { - auto h = - make_dynamic_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(std::vector({-2, 0})); -} diff --git a/test/fail_histogram_dynamic_bin_out_of_bounds_test.cpp b/test/fail_histogram_dynamic_bin_out_of_bounds_test.cpp deleted file mode 100644 index 19f27899..00000000 --- a/test/fail_histogram_dynamic_bin_out_of_bounds_test.cpp +++ /dev/null @@ -1,7 +0,0 @@ -#include - -using namespace boost::histogram; -int main() { - auto h = make_dynamic_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(0, -2); -} diff --git a/test/fail_histogram_dynamic_bin_tuple_out_of_bounds_test.cpp b/test/fail_histogram_dynamic_bin_tuple_out_of_bounds_test.cpp deleted file mode 100644 index af06b291..00000000 --- a/test/fail_histogram_dynamic_bin_tuple_out_of_bounds_test.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include -#include - -using namespace boost::histogram; -int main() { - auto h = make_dynamic_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(std::make_tuple(0, -2)); -} diff --git a/test/fail_histogram_dynamic_bin_vector_out_of_bounds_test.cpp b/test/fail_histogram_dynamic_bin_vector_out_of_bounds_test.cpp deleted file mode 100644 index b527cdc1..00000000 --- a/test/fail_histogram_dynamic_bin_vector_out_of_bounds_test.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include -#include - -using namespace boost::histogram; -int main() { - auto h = make_dynamic_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(std::vector({0, -2})); -} diff --git a/test/fail_histogram_static_bin_wrong_size_test.cpp b/test/fail_histogram_static_at_vector_wrong_dimension_test.cpp similarity index 100% rename from test/fail_histogram_static_bin_wrong_size_test.cpp rename to test/fail_histogram_static_at_vector_wrong_dimension_test.cpp diff --git a/test/fail_histogram_static_bin_out_of_bounds_test.cpp b/test/fail_histogram_static_bin_out_of_bounds_test.cpp deleted file mode 100644 index ec10b4cc..00000000 --- a/test/fail_histogram_static_bin_out_of_bounds_test.cpp +++ /dev/null @@ -1,7 +0,0 @@ -#include - -using namespace boost::histogram; -int main() { - auto h = make_static_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(0, -2); -} diff --git a/test/fail_histogram_static_bin_tuple_out_of_bounds_test.cpp b/test/fail_histogram_static_bin_tuple_out_of_bounds_test.cpp deleted file mode 100644 index 2688272a..00000000 --- a/test/fail_histogram_static_bin_tuple_out_of_bounds_test.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include -#include - -using namespace boost::histogram; -int main() { - auto h = make_static_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(std::make_tuple(0, -2)); -} diff --git a/test/fail_histogram_static_bin_vector_out_of_bounds_test.cpp b/test/fail_histogram_static_bin_vector_out_of_bounds_test.cpp deleted file mode 100644 index 082ac181..00000000 --- a/test/fail_histogram_static_bin_vector_out_of_bounds_test.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include -#include - -using namespace boost::histogram; -int main() { - auto h = make_static_histogram(axis::integer<>(0, 2), axis::integer<>(0, 2)); - h.at(std::vector(-2, 0)); -} diff --git a/test/histogram_test.cpp b/test/histogram_test.cpp index c6051195..dfa7be61 100644 --- a/test/histogram_test.cpp +++ b/test/histogram_test.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -810,6 +811,14 @@ void run_tests() { BOOST_TEST_EQ(h[i].variance(), 5); } + // bad bin access + { + auto h = make(Tag(), axis::integer<>(0, 1), axis::integer<>(0, 1)); + BOOST_TEST_THROWS(h.at(0, 2), std::out_of_range); + BOOST_TEST_THROWS(h.at(std::make_pair(2, 0)), std::out_of_range); + BOOST_TEST_THROWS(h.at(std::vector({0, 2})), std::out_of_range); + } + // pass histogram to function { auto h = make(Tag(), axis::integer<>(0, 3));