jq: three more path-context sites (.E[K], .E[S:T], if, limit) drain their argument generator eagerly, same bug class as #2259

[newhoggy]

Severity: Low

Summary

Found while implementing #2259's fix (getpath's path-context argument generator was
eagerly drained instead of pulled lazily, so a later path's side effects fired even when an
earlier path had already failed to navigate). #2259 converted exactly one call site
(path_context_step_getpath) to pull its argument generator lazily through the new
path_context_component_each sink. Three sibling call sites of
path_context_component_values (the pre-existing eager collector, src/jq/eval_generic.rs)
have the identical bug shape and were deliberately left unconverted, to keep #2259's fix
scoped to the literal issue reported rather than growing into a larger refactor.

Edit: the repro commands below were corrected -- the original version used | keys
(plural, a real jq builtin needing no path-context routing at all) instead of | key
(singular, succinctly's own extension, which is what actually forces path_context_step_*
to run). With | keys, none of these repro; with the correct | key, all four still do,
confirmed against the current main tip (which has since picked up unrelated path_context
laziness fixes for #2694's reduce/foreach SOURCE handling -- a different mechanism that
does not touch these four sites).

All four confirmed live against /usr/bin/jq 1.7.1 (using the bare filter, since jq has no
key/0 of its own) plus succinctly's own | key-forced path-context arm, using the same
debug("late")-in-a-later-comma-alternative repro shape #2259's own tests use:

1. path_context_step_computed_index (.E[K] | key) -- key stream

$ echo '{"a":5}' | jq -c '.a[(1,(debug("late")|2))]'
jq: error (at <stdin>:1): Cannot index number with number
$ echo '{"a":5}' | succinctly jq -c '.a[(1,(debug("late")|2))] | key'
["DEBUG:","late"]
jq: error (at <stdin>:1): Cannot index number with number

Note: this function is already named in #2259's own follow-up (#2914), but for a
different, deeper mechanism (the bracket's target is evaluated once instead of re-evaluated
per key output). This eager-drain angle on the key stream itself is a separate, narrower
issue from #2914's, not covered by it.

2. path_context_step_computed_slice (.E[S:T] | key) -- start/end bound streams

$ echo '{"a":5}' | jq -c '.a[(1,(debug("late")|2)):3]'
jq: error (at <stdin>:1): Cannot index number with object
$ echo '{"a":5}' | succinctly jq -c '.a[(1,(debug("late")|2)):3] | key'
["DEBUG:","late"]
jq: error (at <stdin>:1): Cannot index number with object

3. Expr::If's condition arm (path_context_step_generic)

$ echo '{}' | jq -c 'if (true, (debug("late")|false)) then error("boom") else empty end'
jq: error (at <stdin>:1): boom
$ echo '{}' | succinctly jq -c '(if (true, (debug("late")|false)) then error("boom") else empty end) | key'
["DEBUG:","late"]
jq: error (at <stdin>:1): boom

4. Expr::Limit's count argument arm (path_context_step_generic)

$ echo '{}' | jq -c 'limit((1,(debug("late")|"bad")); error("boom"))'
jq: error (at <stdin>:1): boom
$ echo '{}' | succinctly jq -c '(limit((1,(debug("late")|"bad")); error("boom"))) | key'
["DEBUG:","late"]
jq: error (at <stdin>:1): boom

Root cause / mechanism

All four sites call path_context_component_values (the eager collector) to get every
value of a generator-shaped sub-expression before running the per-value work that can fail
(.[K]'s navigation, a slice bound's use, an if's branch, limit's body). Real jq pulls
each such generator lazily, one output at a time, and stops asking for the next output once
a downstream failure occurs. #2259 already built the right primitive for this
(path_context_component_each, a Demand-driven sink over the same underlying generator);
what's left is rewiring each of these four call sites to walk their per-value work inside
the sink and answer Demand::Stop on failure, the same shape #2259's own
path_context_getpath_walk_one/path_context_step_getpath split demonstrates.

Why this needs its own issue rather than folding into #2259

Each site has its own per-value work shape (Expr::If's two-branch dispatch,
Expr::Limit's truncation-and-early-stop bookkeeping already present in
path_context_step_bounded, the slice's two-bound interaction, .E[K]'s existing
key-outer/target-inner ordering rule) needing its own careful oracle-verified conversion and
test matrix -- doing all four as a drive-by inside #2259 would have risked an unverified
regression in one of them. path_context_step_computed_index additionally interacts with
#2914's separate target-re-evaluation-order finding, so its full fix likely wants to land
together with (or after) that one.

Suggested fix direction

For each site, follow #2259's own pattern: split the "one value's worth of work" into its
own function (as path_context_getpath_walk_one did for getpath), then drive it from
path_context_component_each's sink instead of the eager
path_context_component_values+for loop, answering Demand::Stop on the first failure.

[newhoggy]

🔧 Claimed by agent Brian — branch issue-2916-path-context-eager-drain-sites