From ec5796e1c7f63a97689efcce4c9ca2a464d0772f Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Fri, 9 Jan 2026 16:25:11 +0100 Subject: [PATCH] [hist] Assert on zero arguments to Fill ... and avoid follow-up errors from called functions or LastType by wrapping the remaining body into if constexpr. --- hist/histv7/inc/ROOT/RHist.hxx | 7 ++- hist/histv7/inc/ROOT/RHistEngine.hxx | 62 +++++++++++++---------- hist/histv7/inc/ROOT/RHistFillContext.hxx | 7 ++- hist/histv7/inc/ROOT/RHistStats.hxx | 27 +++++----- 4 files changed, 59 insertions(+), 44 deletions(-) diff --git a/hist/histv7/inc/ROOT/RHist.hxx b/hist/histv7/inc/ROOT/RHist.hxx index 12b22bad970c7..bb10298a4f0ee 100644 --- a/hist/histv7/inc/ROOT/RHist.hxx +++ b/hist/histv7/inc/ROOT/RHist.hxx @@ -294,8 +294,11 @@ public: template void Fill(const A &...args) { - fEngine.Fill(args...); - fStats.Fill(args...); + static_assert(sizeof...(A) >= 1, "need at least one argument to Fill"); + if constexpr (sizeof...(A) >= 1) { + fEngine.Fill(args...); + fStats.Fill(args...); + } } /// Scale all histogram bin contents and statistics. diff --git a/hist/histv7/inc/ROOT/RHistEngine.hxx b/hist/histv7/inc/ROOT/RHistEngine.hxx index 9a5e02f27e6ae..3a739795a0f77 100644 --- a/hist/histv7/inc/ROOT/RHistEngine.hxx +++ b/hist/histv7/inc/ROOT/RHistEngine.hxx @@ -365,21 +365,24 @@ public: template void Fill(const A &...args) { - auto t = std::forward_as_tuple(args...); - if constexpr (std::is_same_v::type, RWeight>) { - static_assert(SupportsWeightedFilling, "weighted filling is not supported for integral bin content types"); - static constexpr std::size_t N = sizeof...(A) - 1; - if (N != fAxes.GetNDimensions()) { - throw std::invalid_argument("invalid number of arguments to Fill"); + static_assert(sizeof...(A) >= 1, "need at least one argument to Fill"); + if constexpr (sizeof...(A) >= 1) { + auto t = std::forward_as_tuple(args...); + if constexpr (std::is_same_v::type, RWeight>) { + static_assert(SupportsWeightedFilling, "weighted filling is not supported for integral bin content types"); + static constexpr std::size_t N = sizeof...(A) - 1; + if (N != fAxes.GetNDimensions()) { + throw std::invalid_argument("invalid number of arguments to Fill"); + } + RWeight weight = std::get(t); + RLinearizedIndex index = fAxes.ComputeGlobalIndexImpl(t); + if (index.fValid) { + assert(index.fIndex < fBinContents.size()); + fBinContents[index.fIndex] += weight.fValue; + } + } else { + Fill(t); } - RWeight weight = std::get(t); - RLinearizedIndex index = fAxes.ComputeGlobalIndexImpl(t); - if (index.fValid) { - assert(index.fIndex < fBinContents.size()); - fBinContents[index.fIndex] += weight.fValue; - } - } else { - Fill(t); } } @@ -458,21 +461,24 @@ public: template void FillAtomic(const A &...args) { - auto t = std::forward_as_tuple(args...); - if constexpr (std::is_same_v::type, RWeight>) { - static_assert(SupportsWeightedFilling, "weighted filling is not supported for integral bin content types"); - static constexpr std::size_t N = sizeof...(A) - 1; - if (N != fAxes.GetNDimensions()) { - throw std::invalid_argument("invalid number of arguments to Fill"); - } - RWeight weight = std::get(t); - RLinearizedIndex index = fAxes.ComputeGlobalIndexImpl(t); - if (index.fValid) { - assert(index.fIndex < fBinContents.size()); - Internal::AtomicAdd(&fBinContents[index.fIndex], weight.fValue); + static_assert(sizeof...(A) >= 1, "need at least one argument to Fill"); + if constexpr (sizeof...(A) >= 1) { + auto t = std::forward_as_tuple(args...); + if constexpr (std::is_same_v::type, RWeight>) { + static_assert(SupportsWeightedFilling, "weighted filling is not supported for integral bin content types"); + static constexpr std::size_t N = sizeof...(A) - 1; + if (N != fAxes.GetNDimensions()) { + throw std::invalid_argument("invalid number of arguments to Fill"); + } + RWeight weight = std::get(t); + RLinearizedIndex index = fAxes.ComputeGlobalIndexImpl(t); + if (index.fValid) { + assert(index.fIndex < fBinContents.size()); + Internal::AtomicAdd(&fBinContents[index.fIndex], weight.fValue); + } + } else { + FillAtomic(t); } - } else { - FillAtomic(t); } } diff --git a/hist/histv7/inc/ROOT/RHistFillContext.hxx b/hist/histv7/inc/ROOT/RHistFillContext.hxx index 4ea0c286aa3a6..f86df47242884 100644 --- a/hist/histv7/inc/ROOT/RHistFillContext.hxx +++ b/hist/histv7/inc/ROOT/RHistFillContext.hxx @@ -101,8 +101,11 @@ public: template void Fill(const A &...args) { - fHist->fEngine.FillAtomic(args...); - fStats.Fill(args...); + static_assert(sizeof...(A) >= 1, "need at least one argument to Fill"); + if constexpr (sizeof...(A) >= 1) { + fHist->fEngine.FillAtomic(args...); + fStats.Fill(args...); + } } /// Flush locally accumulated entries to the histogram. diff --git a/hist/histv7/inc/ROOT/RHistStats.hxx b/hist/histv7/inc/ROOT/RHistStats.hxx index 07f7504ef24e7..ba51b6466d32c 100644 --- a/hist/histv7/inc/ROOT/RHistStats.hxx +++ b/hist/histv7/inc/ROOT/RHistStats.hxx @@ -415,19 +415,22 @@ public: template void Fill(const A &...args) { - auto t = std::forward_as_tuple(args...); - if constexpr (std::is_same_v::type, RWeight>) { - static constexpr std::size_t N = sizeof...(A) - 1; - if (N != fDimensionStats.size()) { - throw std::invalid_argument("invalid number of arguments to Fill"); + static_assert(sizeof...(A) >= 1, "need at least one argument to Fill"); + if constexpr (sizeof...(A) >= 1) { + auto t = std::forward_as_tuple(args...); + if constexpr (std::is_same_v::type, RWeight>) { + static constexpr std::size_t N = sizeof...(A) - 1; + if (N != fDimensionStats.size()) { + throw std::invalid_argument("invalid number of arguments to Fill"); + } + fNEntries++; + double w = std::get(t).fValue; + fSumW += w; + fSumW2 += w * w; + FillImpl<0, N>(t, w); + } else { + Fill(t); } - fNEntries++; - double w = std::get(t).fValue; - fSumW += w; - fSumW2 += w * w; - FillImpl<0, N>(t, w); - } else { - Fill(t); } }