Two correctness bugs in chunk_size.hpp : swapped next_or_subrange args and mismatched add_ready_future_idx parameter order

[arpittkhandelwal]

While reviewing:

libs/core/algorithms/include/hpx/parallel/util/detail/chunk_size.hpp

I found two silent correctness bugs affecting bulk iteration shape handling.

Bug 1 — Swapped arguments in next_or_subrange (variable chunk path)

Location:
get_bulk_iteration_shape_variable (around line 320)

Signature:
constexpr auto next_or_subrange(IterOrR const& target,
std::size_t offset, std::size_t size)

Current code:

std::size_t chunk = (std::min)(chunk_size, count);
shape.emplace_back(it_or_r, chunk);
chunk = (std::min)(count, chunk);
count -= chunk;
it_or_r = next_or_subrange(it_or_r, count, chunk);  // BUG

Issue:
At this point count has already been decremented and represents the
remaining elements, not the advance distance.

The iterator should advance by chunk, but instead advances by the
remaining count, leading to incorrect range progression.

Correct form:

it_or_r = next_or_subrange(it_or_r, chunk, count);

This matches the correct usage in the other overload (line ~214).

Impact:

• Any algorithm using the variable-chunk-size path (std::true_type)
• Multi-chunk scenarios silently process incorrect ranges
• No compile-time warning

Proposed Fix:

• Swap the two arguments at line ~320
• Add a unit test ensuring multi-chunk iteration visits all elements exactly once

Bug 2 — Parameter order mismatch in add_ready_future_idx

Location:
Overloads for add_ready_future_idx (lines ~366–380)

Correct overload (future):

void add_ready_future_idx(std::vector<hpx::future<void>>& workitems,
    F&& f, FwdIter first, std::size_t base_idx, std::size_t count)

Incorrect overload (shared_future):

void add_ready_future_idx(
    std::vector<hpx::shared_future<void>>& workitems,
    F&& f, std::size_t base_idx, FwdIter first, std::size_t count)
       // base_idx and first swapped

Problem:
Call sites pass arguments in the order:

(first, base_idx, count)

This matches the future overload, but in the shared_future
case the parameters are reversed, potentially binding:

first → base_idx
base_idx → first

This leads to undefined behavior passed silently into the kernel.

Corrected version:

void add_ready_future_idx(
    std::vector<hpx::shared_future<void>>& workitems,
    F&& f, FwdIter first, std::size_t base_idx, std::size_t count)
{
    HPX_FORWARD(F, f)(first, count, base_idx);
    workitems.push_back(hpx::make_ready_future());
}

Impact:

• Silent UB when shared_future workitems are used
• Incorrect base_idx propagation in indexed iteration shapes

Proposed Fix:

• Reorder parameters of the shared_future overload
• Add test covering shared_future path via
  get_bulk_iteration_shape_idx
• Optionally document required parameter order to prevent recurrence

Recommendation

Both issues affect correctness and should be fixed before further
refactoring in this area. These are silent runtime errors without
diagnostics.

[arpittkhandelwal]

Hi @hkaiser sir,

While reviewing chunk_size.hpp I found two silent correctness issues I wanted to flag:

1. Swapped arguments to next_or_subrange (line ~320, get_bulk_iteration_shape_variable)

count and chunk are passed in the wrong order:

next_or_subrange(it_or_r, count, chunk);

At that point count is already decremented (remaining elements), so the iterator advances by the remaining count instead of the current chunk size. Every iteration after the first processes incorrect ranges silently.

Fix:
next_or_subrange(it_or_r, chunk, count);

2. Parameter order mismatch in add_ready_future_idx for shared_future (lines ~374–376)

base_idx and FwdIter first are swapped compared to the other overloads and the call sites. If this overload is selected, an iterator and a size_t are silently bound to each other's parameters, leading to undefined behavior.

Fix:
Reorder parameters to match the future overload.

Both issues are silent and produce no compile-time warnings.

If you’re okay with it, I can submit a PR that:

• Swaps count and chunk in get_bulk_iteration_shape_variable
• Fixes the parameter order in add_ready_future_idx (shared_future)
• Adds targeted unit tests to prevent regressions in both paths

Please let me know if you'd like me to go ahead.

[hkaiser]

Thanks for this analysis! I concur to your suggestions.

[OmkarTipugade]

@hkaiser, I would like to work on these bugs. Can you please assign this issue to me

[arpittkhandelwal]

@hkaiser, I would like to work on these bugs. Can you please assign this issue to me

Hi @OmkarTipugade,
Thanks for your interest in working on this.
I’ve already started working on the fixes and will be raising the PR soon with the corrections and accompanying unit tests.
Appreciate your willingness to contribute!