fix for fill segfault

old code allowed to pass sequences of length 1 with sequence of length N > 1, which is invalid

include/boost/histogram/detail/fill_n.hpp

@@ -45,7 +45,7 @@ void fold(Ts&&...) noexcept {} // helper to enable operator folding
 template <class T>
 auto to_ptr_size(const T& x) {
   return static_if<std::is_scalar<T>>(
-      [](const auto& x) { return std::make_pair(&x, static_cast<std::size_t>(1)); },
+      [](const auto& x) { return std::make_pair(&x, static_cast<std::size_t>(0)); },
       [](const auto& x) { return std::make_pair(dtl::data(x), dtl::size(x)); }, x);
 }
 
@@ -96,7 +96,7 @@ struct index_visitor {
   template <class T>
   void call_1(std::false_type, const T& iterable) const {
     // T is iterable; fill N values
-    auto* tp = dtl::data(iterable) + start_;
+    const auto* tp = dtl::data(iterable) + start_;
     for (auto it = begin_; it != begin_ + size_; ++it) call_2(IsGrowing{}, it, *tp++);
   }
 
@@ -161,7 +161,7 @@ void fill_n_storage(S& s, const Index idx, Ts&&... p) noexcept {
     BOOST_ASSERT(idx < s.size());
     fill_storage_3(s[idx], *p.first...);
   }
-  fold((p.second > 1 ? ++p.first : 0)...);
+  fold((p.second ? ++p.first : 0)...);
 }
 
 template <class S, class Index, class T, class... Ts>
@@ -171,8 +171,8 @@ void fill_n_storage(S& s, const Index idx, weight_type<T>&& w, Ts&&... ps) noexc
     fill_storage_3(s[idx], weight_type<decltype(*w.value.first)>{*w.value.first},
                    *ps.first...);
   }
-  if (w.value.second > 1) ++w.value.first;
+  if (w.value.second) ++w.value.first;
-  fold((ps.second > 1 ? ++ps.first : 0)...);
+  fold((ps.second ? ++ps.first : 0)...);
 }
 
 // general Nd treatment
@@ -257,39 +257,48 @@ std::size_t get_total_size(const A& axes, const dtl::span<const T, N>& values) {
   // - span<CT, N>: for any histogram, N == rank
   // - span<V<T, CT>, N>: for any histogram, N == rank
   BOOST_ASSERT(axes_rank(axes) == values.size());
-  std::size_t size = 1u;
+  constexpr auto unset = static_cast<std::size_t>(-1);
+  std::size_t size = unset;
   for_each_axis(axes, [&size, vit = values.begin()](const auto& ax) mutable {
     using AV = axis::traits::value_type<std::decay_t<decltype(ax)>>;
-    auto vis = [&size](const auto& v) {
+    maybe_visit(
-      // v is either convertible to value or a sequence of values
+        [&size](const auto& v) {
-      using V = std::remove_const_t<std::remove_reference_t<decltype(v)>>;
+          // v is either convertible to value or a sequence of values
-      const std::size_t n = static_if_c<(std::is_convertible<decltype(v), AV>::value ||
+          using V = std::remove_const_t<std::remove_reference_t<decltype(v)>>;
-                                         !is_iterable<V>::value)>(
+          static_if_c<(std::is_convertible<decltype(v), AV>::value ||
-          [](const auto&) { return static_cast<std::size_t>(1); },
+                       !is_iterable<V>::value)>(
-          [](const auto& v) { return dtl::size(v); }, v);
+              [](const auto&) {},
-      if (size != 1u && n != 1u && size != n)
+              [&size](const auto& v) {
-        BOOST_THROW_EXCEPTION(
+                const auto n = dtl::size(v);
-            std::invalid_argument("spans must have compatible lengths"));
+                // must repeat this here for msvc :(
-      size = std::max(size, n);
+                constexpr auto unset = static_cast<std::size_t>(-1);
-    };
+                if (size == unset)
-    maybe_visit(vis, *vit++);
+                  size = dtl::size(v);
+                else if (size != n)
+                  BOOST_THROW_EXCEPTION(
+                      std::invalid_argument("spans must have compatible lengths"));
+              },
+              v);
+        },
+        *vit++);
   });
-  return size;
+  // if all arguments are not iterables, return size of 1
+  return size == unset ? 1 : size;
 }
 
 inline void fill_n_check_extra_args(std::size_t) noexcept {}
 
 template <class T, class... Ts>
-void fill_n_check_extra_args(std::size_t n, T&& x, Ts&&... ts) {
+void fill_n_check_extra_args(std::size_t size, T&& x, Ts&&... ts) {
-  // values of length 1 may not be combined with weights and samples of length > 1
+  // sequences must have same lengths, but sequences of length 0 are broadcast
-  if (x.second != 1 && n != x.second)
+  if (x.second != 0 && x.second != size)
     BOOST_THROW_EXCEPTION(std::invalid_argument("spans must have compatible lengths"));
-  fill_n_check_extra_args(n, std::forward<Ts>(ts)...);
+  fill_n_check_extra_args(size, std::forward<Ts>(ts)...);
 }
 
 template <class T, class... Ts>
-void fill_n_check_extra_args(std::size_t n, weight_type<T>&& w, Ts&&... ts) {
+void fill_n_check_extra_args(std::size_t size, weight_type<T>&& w, Ts&&... ts) {
-  fill_n_check_extra_args(n, w.value, std::forward<Ts>(ts)...);
+  fill_n_check_extra_args(size, w.value, std::forward<Ts>(ts)...);
 }
 
 template <class S, class A, class T, std::size_t N, class... Us>

include/boost/histogram/histogram.hpp

@@ -265,11 +265,11 @@ public:
     @param args iterable of values.
     @param samples single sample or an iterable of samples.
   */
-  template <class Iterable, class... Ts, class = detail::requires_iterable<Iterable>,
+  template <class Iterable, class... Ts, class = detail::requires_iterable<Iterable>>
-            class = mp11::mp_list<detail::requires_iterable<Ts>...>>
   void fill(const Iterable& args, const sample_type<std::tuple<Ts...>>& samples) {
     using acc_traits = detail::accumulator_traits<value_type>;
-    using sample_args_passed = std::tuple<decltype(*detail::data(std::declval<Ts>()))...>;
+    using sample_args_passed =
+        std::tuple<decltype(*detail::to_ptr_size(std::declval<Ts>()).first)...>;
     detail::sample_args_passed_vs_expected<sample_args_passed,
                                            typename acc_traits::args>();
     std::lock_guard<typename mutex_base::type> guard{mutex_base::get()};
@@ -294,20 +294,21 @@ public:
   }
 
   template <class Iterable, class T, class... Ts,
-            class = detail::requires_iterable<Iterable>,
+            class = detail::requires_iterable<Iterable>>
-            class = mp11::mp_list<detail::requires_iterable<Ts>...>>
   void fill(const Iterable& args, const weight_type<T>& weights,
             const sample_type<std::tuple<Ts...>>& samples) {
     using acc_traits = detail::accumulator_traits<value_type>;
-    using sample_args = std::tuple<decltype(*detail::data(std::declval<Ts>()))...>;
+    using sample_args_passed =
-    detail::sample_args_passed_vs_expected<sample_args, typename acc_traits::args>();
+        std::tuple<decltype(*detail::to_ptr_size(std::declval<Ts>()).first)...>;
+    detail::sample_args_passed_vs_expected<sample_args_passed,
+                                           typename acc_traits::args>();
     std::lock_guard<typename mutex_base::type> guard{mutex_base::get()};
     mp11::tuple_apply(
         [&](const auto&... sargs) {
           constexpr bool weight_valid = acc_traits::wsupport::value;
           static_assert(weight_valid, "error: accumulator does not support weights");
           constexpr bool sample_valid =
-              std::is_convertible<sample_args, typename acc_traits::args>::value;
+              std::is_convertible<sample_args_passed, typename acc_traits::args>::value;
           detail::fill_n(mp11::mp_bool<(weight_valid && sample_valid)>{}, offset_,
                          storage_, axes_, detail::make_span(args),
                          weight(detail::to_ptr_size(weights.value)),

test/histogram_fill_test.cpp

@@ -64,44 +64,92 @@ template <class Tag>
 void run_tests(const std::vector<int>& x, const std::vector<int>& y,
                const std::vector<double>& w) {
 
-  // 1D simple
+  // 1D simple A
   {
     auto h = make(Tag(), in{1, 3});
-    auto h3 = h;
     auto h2 = h;
-
     for (auto&& xi : x) h(xi);
-    h2.fill(x); // uses 1D specialization
+    // uses 1D specialization
-    const auto vx = {x};
+    h2.fill(x);
-    h3.fill(vx); // uses generic form
-
     BOOST_TEST_EQ(h, h2);
-    BOOST_TEST_EQ(h, h3);
+  }
+
+  // 1D simple B
+  {
+    auto h = make(Tag(), in{1, 3});
+    auto h2 = h;
+    for (auto&& xi : x) h(xi);
+    // uses generic form
+    const auto vx = {x};
+    h2.fill(vx);
+    BOOST_TEST_EQ(h, h2);
+  }
+
+  // 1D simple C
+  {
+    auto h = make(Tag(), in{1, 3});
+    auto h2 = h;
+    h(1);
+    for (auto&& xi : x) h(xi);
+    // uses variant
+    boost::variant2::variant<int, std::vector<int>, std::string> v[1];
+    v[0] = 1;
+    h2.fill(v);
+    v[0] = x;
+    h2.fill(v);
+    BOOST_TEST_EQ(h, h2);
+  }
+
+  // 1D bad arguments
+  {
+    auto h = make(Tag(), in{1, 3});
 
     int bad1[2][4];
     boost::ignore_unused(bad1);
+    BOOST_TEST_THROWS(h.fill(bad1), std::invalid_argument);
+
     std::vector<std::array<int, 4>> bad2;
     boost::ignore_unused(bad2);
-    BOOST_TEST_THROWS(h.fill(bad1), std::invalid_argument);
     BOOST_TEST_THROWS(h.fill(bad2), std::invalid_argument);
   }
 
   // 1D with category axis
   {
     auto h = make(Tag(), cs{"A", "B"});
+    auto h2 = h;
 
-    auto s = {"A", "B", "C"};
+    const auto s = {"A", "B", "C"};
-    h.fill(s);
+    for (auto&& si : s) h(si);
-    BOOST_TEST_EQ(h[0], 1);
+    h2.fill(s);
-    BOOST_TEST_EQ(h[1], 1);
-    BOOST_TEST_EQ(h[2], 1);
 
-    variant<std::string, std::vector<std::string>> v[1];
+    variant<int, std::string, std::vector<std::string>> v[1];
-    v[0] = "ABC";
+    h("B");
-    h.fill(v);
+    v[0] = "B";
-    BOOST_TEST_EQ(h[0], 1);
+    h2.fill(v);
-    BOOST_TEST_EQ(h[1], 1);
+
-    BOOST_TEST_EQ(h[2], 2);
+    v[0] = std::vector<std::string>(s.begin(), s.end());
+    for (auto&& si : s) h(si);
+    h2.fill(v);
+
+    BOOST_TEST_EQ(h, h2);
+  }
+
+  // 1D weight
+  {
+    auto h = make(Tag(), in{1, 3});
+    auto h2 = h;
+
+    for (auto&& xi : x) h(weight(2), xi);
+    h2.fill(weight(2), x);
+
+    for (unsigned i = 0; i < ndata; ++i) h(weight(w[i]), x[i]);
+    h2.fill(weight(w), x);
+
+    BOOST_TEST_EQ(h, h2);
+
+    auto w2 = {1};
+    boost::ignore_unused(w2);
+    BOOST_TEST_THROWS(h2.fill(x, weight(w2)), std::invalid_argument);
   }
 
   // 2D simple
@@ -119,49 +167,16 @@ void run_tests(const std::vector<int>& x, const std::vector<int>& y,
     BOOST_TEST_THROWS(h.fill(x), std::invalid_argument);
 
     // not rectangular
-    std::array<std::vector<int>, 2> bad = {{std::vector<int>(2), std::vector<int>(3)}};
+    std::array<std::vector<int>, 2> bad = {{std::vector<int>(1), std::vector<int>(2)}};
     boost::ignore_unused(bad);
     BOOST_TEST_THROWS(h2.fill(bad), std::invalid_argument);
   }
 
-  // 1D variant and weight
-  {
-    auto h1 = make(Tag(), in{1, 3});
-    auto h2 = h1;
-
-    h1(1);
-    for (auto&& xi : x) h1(xi);
-
-    using V = variant<int, std::vector<int>>;
-    std::vector<V> v(1);
-    v[0] = 1;
-    h2.fill(v);
-    v[0] = x;
-    h2.fill(v);
-
-    BOOST_TEST_EQ(h1, h2);
-
-    for (auto&& xi : x) h1(weight(2), xi);
-    h2.fill(weight(2), x);
-
-    BOOST_TEST_EQ(h1, h2);
-
-    for (unsigned i = 0; i < ndata; ++i) h1(weight(w[i]), x[i]);
-    h2.fill(weight(w), x);
-
-    BOOST_TEST_EQ(h1, h2);
-
-    auto w2 = w;
-    w2.resize(ndata - 1);
-    boost::ignore_unused(w2);
-    BOOST_TEST_THROWS(h2.fill(v, weight(w2)), std::invalid_argument);
-  }
-
   // 2D variant and weight
   {
     auto h = make(Tag(), in{1, 3}, in0{1, 5});
 
-    using V = variant<int, std::vector<int>>;
+    using V = variant<int, std::vector<int>, std::string>;
     V xy[2];
 
     {
@@ -249,9 +264,9 @@ void run_tests(const std::vector<int>& x, const std::vector<int>& y,
   {
     auto h = make(Tag(), ing(), ing());
     auto h2 = h;
-    for (unsigned i = 0; i < ndata; ++i) h(x[i], y[i], weight(w[i]));
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], y[i], weight(2));
     const auto xy = {x, y};
-    h2.fill(xy, weight(w));
+    h2.fill(xy, weight(2));
     BOOST_TEST_EQ(h, h2);
   }
 
@@ -259,24 +274,36 @@ void run_tests(const std::vector<int>& x, const std::vector<int>& y,
   {
     auto h = make(Tag(), csg{}, in{1, 2});
     auto h2 = h;
+
+    h("foo", 1);
+    h("foo", 2);
+
     using V = variant<std::string, std::vector<std::string>, int, std::vector<int>>;
     const auto xy = {V("foo"), V(std::vector<int>{1, 2})};
-    h.fill(xy);
+    h2.fill(xy);
-    h2("foo", 1);
+
-    h2("foo", 2);
     BOOST_TEST_EQ(h, h2);
+
+    const auto bad = {V(std::vector<std::string>(1, "foo")), V(std::vector<int>{1, 2})};
+    boost::ignore_unused(bad);
+    BOOST_TEST_THROWS(h.fill(bad), std::invalid_argument);
   }
 
   // 1D profile with samples
   {
     auto h = make_s(Tag(), profile_storage(), in(1, 3));
     auto h2 = h;
+
     for (unsigned i = 0; i < ndata; ++i) h(x[i], sample(w[i]));
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], sample(w[i]), weight(w[i]));
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], sample(2), weight(w[i]));
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], sample(w[i]), weight(2));
 
     h2.fill(x, sample(w));
-    BOOST_TEST_EQ(h, h2);
-    for (unsigned i = 0; i < ndata; ++i) h(x[i], sample(w[i]), weight(w[i]));
     h2.fill(x, sample(w), weight(w));
+    h2.fill(x, sample(2), weight(w));
+    h2.fill(x, sample(w), weight(2));
+
     BOOST_TEST_EQ(h, h2);
   }
 
@@ -285,13 +312,17 @@ void run_tests(const std::vector<int>& x, const std::vector<int>& y,
     auto h = make_s(Tag(), weighted_profile_storage(), in(1, 3), in0(1, 3));
     auto h2 = h;
 
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], 3, sample(w[i]), weight(w[i]));
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], 3, sample(2), weight(w[i]));
+    for (unsigned i = 0; i < ndata; ++i) h(x[i], 3, sample(w[i]), weight(2));
+
     using V = variant<int, std::vector<int>>;
     std::array<V, 2> xy;
     xy[0] = x;
     xy[1] = 3;
-
-    for (unsigned i = 0; i < ndata; ++i) h(x[i], 3, sample(w[i]), weight(w[i]));
     h2.fill(xy, sample(w), weight(w));
+    h2.fill(xy, sample(2), weight(w));
+    h2.fill(xy, sample(w), weight(2));
 
     BOOST_TEST_EQ(h, h2);
   }