Fix integer overflow in --parameter-scan near i32::MAX

[nikolauspschuetz]

Fixes #920.

RangeStep::next() advanced with an unconditional self.state += self.step.
When a --parameter-scan range ends at (or one step below) i32::MAX, the
final state + step overflows — panicking in debug/test builds and silently
wrapping to i32::MIN in release (which then generates ~4 billion commands).

The fix tracks completion with a finished flag and computes the step check as
end - state, which cannot overflow because state <= end. Behavior is
unchanged for all existing ranges (integer and Decimal); only the previously
overflowing tail is now handled correctly.

Verification

Added does_not_overflow_near_type_max, which collects
RangeStep::new(i32::MAX - 1, i32::MAX, 1). It panics on master
(attempt to add with overflow) and passes with this change. Full suite green;
cargo fmt --check and cargo clippy clean.

---

Disclosure: fix written with AI assistance; reviewed and verified by me.

[xhon-pelushi]

Checked this out and built it. The next() fix is right and the approach is the good one — deriving the stop condition from end - state instead of speculatively computing state + step is exactly the way to avoid the overflow. Your new test passes, and I confirmed the mirror case at the other end of the type works too (RangeStep::new(i32::MIN, i32::MIN + 1, 1) yields [-2147483648, -2147483647]).

Two things I ran into, one of which is introduced here.

1. size_hint() now disagrees with next() once finished is set

The finished flag short-circuits next() but size_hint() still derives its answer from state, which is deliberately left un-advanced. So a finished iterator claims one element remains:

hint start      = (2, Some(2))
hint after 1st  = (1, Some(1))
hint after last = (1, Some(1))   <- next() returns None

For comparison, the pre-existing exhaustion path stays consistent, which is what makes this stand out:

RangeStep::new(0, 10, 3), fully drained -> hint = (0, Some(0)), next() = None

Iterator::size_hint's contract is that the lower bound must not exceed the number of elements actually remaining, so a lower bound of 1 with zero left is incorrect, and collect() will over-allocate by one. An early return keeps it honest:

fn size_hint(&self) -> (usize, Option<usize>) {
    if self.finished {
        return (0, Some(0));
    }
    range_step_size_hint(self.state, self.end, self.step)
}

2. --parameter-scan still panics on a wide range — one call earlier

This is pre-existing rather than something you introduced, but it does mean the title case isn't fully closed. range_step_size_hint() computes end - start before anything else can reject the range:

let steps = (end - start + T::from(1)) / step;   // range_step.rs:92

and new() has to call it in order to apply the MAX_PARAMETERS cap. So the overflow happens while deciding whether the range is too large:

$ ./target/debug/hyperfine --parameter-scan x -2147483648 2147483647 'true'
thread 'main' panicked at src/parameter/range_step.rs:92:18:
attempt to subtract with overflow

At the unit level, with catch_unwind:

new(i32::MIN,     i32::MAX, 1) -> panicked
new(i32::MIN + 1, i32::MAX, 1) -> panicked
new(i32::MIN, i32::MIN + 1, 1) -> Ok([-2147483648, -2147483647])   (your fix, works)
new(i32::MAX - 1, i32::MAX, 1) -> Ok([2147483646, 2147483647])     (your test, works)

Worth noting this also qualifies the comment on the new code:

// `end - state` never overflows here (state <= end), unlike `state + step`.

state <= end on its own does not prevent the subtraction from overflowing for a signed type — line 92 is proof, since start <= end holds there too. What actually makes it safe inside next() is that new() already capped the span at MAX_PARAMETERS steps, so by the time you get there end - state is small. That is a sound invariant, just a different one than the comment states, and it's worth saying so since it's the reason the two subtractions behave differently.

Whether to fix #2 here or separately is your call — a checked_sub-style path would need a trait bound that Numeric doesn't currently have, so it's plausibly its own change. Just flagging that --parameter-scan with a full-width signed range still aborts after this lands.

Tested on e1813ea, rustc 1.97.1, linux x86_64, debug profile (overflow checks on).

[xhon-pelushi]

Checked this out and built it. The next() fix is right and the approach is the good one — deriving the stop condition from end - state instead of speculatively computing state + step is exactly the way to avoid the overflow. Your new test passes, and I confirmed the mirror case at the other end of the type works too (RangeStep::new(i32::MIN, i32::MIN + 1, 1) yields [-2147483648, -2147483647]).

Two things I ran into, one of which is introduced here.

1. size_hint() now disagrees with next() once finished is set

The finished flag short-circuits next() but size_hint() still derives its answer from state, which is deliberately left un-advanced. So a finished iterator claims one element remains:

hint start      = (2, Some(2))
hint after 1st  = (1, Some(1))
hint after last = (1, Some(1))   <- next() returns None

For comparison, the pre-existing exhaustion path stays consistent, which is what makes this stand out:

RangeStep::new(0, 10, 3), fully drained -> hint = (0, Some(0)), next() = None

Iterator::size_hint's contract is that the lower bound must not exceed the number of elements actually remaining, so a lower bound of 1 with zero left is incorrect, and collect() will over-allocate by one. An early return keeps it honest:

fn size_hint(&self) -> (usize, Option<usize>) {
    if self.finished {
        return (0, Some(0));
    }
    range_step_size_hint(self.state, self.end, self.step)
}

2. --parameter-scan still panics on a wide range — one call earlier

This is pre-existing rather than something you introduced, but it does mean the title case isn't fully closed. range_step_size_hint() computes end - start before anything else can reject the range:

let steps = (end - start + T::from(1)) / step;   // range_step.rs:92

and new() has to call it in order to apply the MAX_PARAMETERS cap. So the overflow happens while deciding whether the range is too large:

$ ./target/debug/hyperfine --parameter-scan x -2147483648 2147483647 'true'
thread 'main' panicked at src/parameter/range_step.rs:92:18:
attempt to subtract with overflow

At the unit level, with catch_unwind:

new(i32::MIN,     i32::MAX, 1) -> panicked
new(i32::MIN + 1, i32::MAX, 1) -> panicked
new(i32::MIN, i32::MIN + 1, 1) -> Ok([-2147483648, -2147483647])   (your fix, works)
new(i32::MAX - 1, i32::MAX, 1) -> Ok([2147483646, 2147483647])     (your test, works)

Worth noting this also qualifies the comment on the new code:

// `end - state` never overflows here (state <= end), unlike `state + step`.

state <= end on its own does not prevent the subtraction from overflowing for a signed type — line 92 is proof, since start <= end holds there too. What actually makes it safe inside next() is that new() already capped the span at MAX_PARAMETERS steps, so by the time you get there end - state is small. That is a sound invariant, just a different one than the comment states, and it's worth saying so since it's the reason the two subtractions behave differently.

Whether to fix #2 here or separately is your call — a checked_sub-style path would need a trait bound that Numeric doesn't currently have, so it's plausibly its own change. Just flagging that --parameter-scan with a full-width signed range still aborts after this lands.

Tested on e1813ea, rustc 1.97.1, linux x86_64, debug profile (overflow checks on).

[nikolauspschuetz]

Thanks for the thorough review — both catches are right.

1. size_hint(): fixed. It now returns (0, Some(0)) once finished is set, so it agrees with next() after exhaustion. Added a regression test (size_hint_is_zero_once_exhausted) that drives the iterator to the end and checks the hint.

2. The comment: corrected. You're right that state <= end isn't what keeps end - state from overflowing — as line 92 shows, start <= end holds there and it still overflows. The real invariant is that new() has already capped the span at MAX_PARAMETERS steps, so end - state is small by the time next() runs; the comment now says that.

3. The range_step_size_hint() overflow at new() time: agreed, it's a real pre-existing gap, and it does mean --parameter-scan with a full-width signed range still aborts. Since a clean fix needs checked/widening arithmetic that Numeric doesn't currently expose (as you noted), I'll do that as a separate follow-up rather than widen this PR — this one stays scoped to the next() overflow. Thanks for pinning the exact case.

[xhon-pelushi]

Re-checked 32b76b3. The finished guard fixes the size_hint() contract issue I flagged: cargo test range_step -- --nocapture passes, including size_hint_is_zero_once_exhausted, and the two-element max-bound CLI case (--parameter-scan x 2147483646 2147483647) runs cleanly.

I also reconfirmed the full-width signed range still panics at range_step_size_hint before construction, which matches your plan to handle that as a separate follow-up rather than widening this PR. With the PR-introduced issue fixed and the remaining scope called out, this looks good to me.