diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index ef3da3bdd..ea06d0bec 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -922,8 +922,9 @@ is the revert that established what the other one costs. every `Snapshot` marker the expression carries first, because a freshly rebuilt document cannot be a node any earlier binding was frozen from — the one fact a `StandardJson` can always establish, where a positive witness cannot (an empty container or a scalar carries - no cursor to recover a node id from). The generic funnels, which already prove their - markers against a live cursor, take the non-demoting `eval_each_owned_bridged` so the proof + no cursor to recover a node id from). The generic funnels hand the re-entry the witness + of the live cursor they bridge from (`Reentry::Against`, #3122) — only a caller that has + already run that same demotion for this document passes `Reentry::Proven` — so the proof is kept (and a re-entry whose expression holds no assignment, builtin or call skips the rebuild, since only a resolver invocation can read a demotion). The two classes #2642 deliberately excluded are closed with it: the owned-identity diff --git a/docs/plan/jq-bind-origin-frame.md b/docs/plan/jq-bind-origin-frame.md index 37cc849f3..f4131270d 100644 --- a/docs/plan/jq-bind-origin-frame.md +++ b/docs/plan/jq-bind-origin-frame.md @@ -229,8 +229,9 @@ Three findings, in order of what they cost: fabrication where the bind and the rebuild both run inside `eval.rs` (the input-queue bridge, a fold's UPDATE, a `|=` right-hand side, `with_entries`, a `catch` handler) — no funnel ran there. Closed at every owned→document re-entry in `eval.rs` (`eval_each_owned` - and its siblings demote every `Snapshot` marker; the generic funnels take the - non-demoting `eval_each_owned_bridged`), plus `OwnedIdentity::exact`/`root`/`root_witness` + and its siblings demote every `Snapshot` marker; the generic funnels pass their cursor's + witness as `Reentry::Against` since #3122, and only a caller that already ran that + demotion passes `Reentry::Proven`), plus `OwnedIdentity::exact`/`root`/`root_witness` for the owned-identity route and `try_payload_root` for `catch`. See `limitations.md`'s #3036 paragraph for the refuse-only flips. - **Value-mode bindings — the root case closed by @@ -245,6 +246,23 @@ Three findings, in order of what they cost: a non-root register position, which needs a document-absolute bind path reachable from a value-mode cursor (the `Origin::At` machinery) plus `needs_path_context` routing to decide when to pay for it — scoped separately. +- **Closed by [#3122](https://github.com/rust-works/succinctly/issues/3122).** The + re-entry contract is one type. Every owned re-entry in `eval.rs` (`eval_each_owned`, + `eval_owned_input`, `eval_owned_expr_fork`, `eval_owned_multi_first`, + `update_root_with_filter`, `each_recurse_walk`) and `eval_generic.rs`'s `eval_on_owned` takes + a `Reentry`: `Against(RootWitness)` to reroot the markers against a named root at the + re-entry (#2642's demotion and #3037's promotion, one walk), `Proven` when a funnel, an + enclosing re-entry or a once-per-fold/loop hoist already did. It replaced + the three booleans that used to encode the same fact (`trackable` at the resolver sinks, + `ambient` in `|=`'s root leaf and `until`/`while`, `resolver_free` in the folds) and the + thirteen "plain vs `_bridged`" twins, and `reroot_for_reentry`/`demote_for_reentry` -- + #3036's fused single-walk precheck (a rewritable marker *and* a resolver-reaching node, else + borrowed) over `reroot_markers`' rewrite -- is the one derivation on both sides of the bridge. + `trackable` remains a resolver-internal register-provenance flag (it also drives path-shape + decisions); `Reentry::at_register` is its one conversion. The resolver holds no cursor and no + `RootWitness` (`Frame` is an invocation id and a path), so `Proven` is the honest encoding of + "the funnel that built this document already demoted for it, and the register is a navigated + node of it" -- not a witness the resolver could mint itself. - [#2646](https://github.com/rust-works/succinctly/issues/2646) — `first`/`last`/`add` navigating inside their own jq-level definitions against a *constructed* value inside `path()` never raise, found by `scripts/jq-bind-origin-fuzz.py`'s differential fuzz and diff --git a/src/jq/eval.rs b/src/jq/eval.rs index ff95ff888..1a43b0504 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -4695,62 +4695,18 @@ where /// `Many`/`ManyOwned`/`Partial` intact instead of folding — this just /// unpacks its `QueryResult` into the `(outputs, trailing control)` shape /// `reduce`/`foreach`/`while`/`until` build their fold/fork logic on top of. +#[inline(always)] fn eval_owned_expr_fork( expr: &Expr, input: &OwnedValue, optional: bool, + reentry: Reentry, ) -> (Vec, Option) { - fork_outputs(eval_owned_input::, S>(expr, input, optional)) -} - -/// [`eval_owned_expr_fork`] over the caller's own ambient value, unrebuilt -/// -- see [`eval_owned_input_bridged`] (#3036). -fn eval_owned_expr_fork_bridged( - expr: &Expr, - input: &OwnedValue, - optional: bool, -) -> (Vec, Option) { - fork_outputs(eval_owned_input_bridged::, S>( - expr, input, optional, + fork_outputs(eval_owned_input::, S>( + expr, input, optional, reentry, )) } -/// [`eval_owned_expr_fork_bridged`] over `expr` or its pre-demoted twin -/// `demoted`, chosen by whether `input` is the caller's own ambient value -/// still (#3036) -- the loop shape `until`/`while` take, where the first -/// round runs on the input and every later round on a value `update` -/// computed. `demoted` is [`demote_for_owned_reentry`] of `expr`, computed -/// once by the caller rather than re-walked every round: `cond`/`update` -/// are the same static expression at every step, so re-demoting them per -/// step was a full `any_subexpr` scan per iteration, not per loop. -fn eval_owned_expr_fork_from( - expr: &Expr, - demoted: &Expr, - input: &OwnedValue, - optional: bool, - ambient: bool, -) -> (Vec, Option) { - let expr = if ambient { expr } else { demoted }; - eval_owned_expr_fork_bridged::(expr, input, optional) -} - -/// [`eval_owned_expr_fork`] or its bridged twin, chosen by whether `value` -/// is a node the resolver is tracking (#3036) -- [`eval_each_owned_at`]'s -/// fork-shaped twin, for a bind source evaluated collected (by value) -/// rather than through a sink. -fn eval_owned_expr_fork_at( - expr: &Expr, - value: &OwnedValue, - trackable: bool, - optional: bool, -) -> (Vec, Option) { - if trackable { - eval_owned_expr_fork_bridged::(expr, value, optional) - } else { - eval_owned_expr_fork::(expr, value, optional) - } -} - /// [`eval_owned_expr_fork`]'s unpacking of an owned `QueryResult`. fn fork_outputs(result: QueryResult<'_, Vec>) -> (Vec, Option) { match result { @@ -6103,19 +6059,25 @@ fn each_try<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Flow::Escaped(Control::Error(e)) => match catch { Some(catch_expr) => { // #3036: see `try_payload_root`. - let catch_expr = reroot_markers::(catch_expr, &try_payload_root(expr)); - eval_each_owned_bridged::(&catch_expr, &e.payload(), optional, &mut |o| { - sink(Item::Owned(o)) - }) + let root = try_payload_root(expr); + eval_each_owned::( + catch_expr, + &e.payload(), + optional, + Reentry::Against(root), + &mut |o| sink(Item::Owned(o)), + ) } None => Flow::Exhausted, }, Flow::Escaped(Control::Break(_)) => match catch { - Some(catch_expr) => { - eval_each_owned::(catch_expr, &OwnedValue::Null, optional, &mut |o| { - sink(Item::Owned(o)) - }) - } + Some(catch_expr) => eval_each_owned::( + catch_expr, + &OwnedValue::Null, + optional, + Reentry::REBUILT, + &mut |o| sink(Item::Owned(o)), + ), None => Flow::Exhausted, }, // Halt is never caught (`Control`'s own guarantee); other terminal @@ -6983,23 +6945,19 @@ fn each_any_all_gen_cond<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // #3036: `owned` is this arm's own input, unrebuilt -- bridged. Each // element `gen` yields is a computed value, so `cond` runs on it through // the demoting entry. - let flow = - eval_each_owned_bridged::( - gen, - &owned, - optional, - &mut |elem| match any_all_probe_element::(cond, &elem, target_truthy) { - Ok(true) => { - probe_escape = None; - if sink(Item::Owned(OwnedValue::Bool(target_truthy))) == Demand::Stop { - outer_stopped = true; - } - Demand::Stop + let flow = eval_each_owned::(gen, &owned, optional, Reentry::Proven, &mut |elem| { + match any_all_probe_element::(cond, &elem, target_truthy) { + Ok(true) => { + probe_escape = None; + if sink(Item::Owned(OwnedValue::Bool(target_truthy))) == Demand::Stop { + outer_stopped = true; } - Ok(false) => Demand::Continue, - Err(control) => stop_with_escape(&mut probe_escape, control), - }, - ); + Demand::Stop + } + Ok(false) => Demand::Continue, + Err(control) => stop_with_escape(&mut probe_escape, control), + } + }); let effective_flow = if matches!(flow, Flow::Stopped { .. }) && probe_escape.is_some() { resume_from_escape(probe_escape, flow) @@ -7034,7 +6992,7 @@ fn each_upper_in<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( let mut outer_stopped = false; // #3036: `current` is this arm's own input, unrebuilt -- bridged. - let flow = eval_each_owned_bridged::(s, ¤t, optional, &mut |candidate| { + let flow = eval_each_owned::(s, ¤t, optional, Reentry::Proven, &mut |candidate| { if owned_value_eq::(&candidate, ¤t) { if sink(Item::Owned(OwnedValue::Bool(true))) == Demand::Stop { outer_stopped = true; @@ -7781,7 +7739,9 @@ fn eval_each_pipe<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // downstream sink never asked for). Item::Owned(v) => { let rest_pipe = rest_pipe.get_or_insert_with(|| Expr::Pipe(rest.to_vec())); - eval_each_owned::(rest_pipe, &v, optional, &mut |o| sink(Item::Owned(o))) + eval_each_owned::(rest_pipe, &v, optional, Reentry::REBUILT, &mut |o| { + sink(Item::Owned(o)) + }) } }; match flow { @@ -8021,81 +7981,6 @@ fn each_take_nth<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( (wanted, flow) } -/// Owned-input twin of [`eval_each`], mirroring `eval_owned_input`. -/// -/// The sink takes `OwnedValue`, not `Item`: values produced against the -/// locally-built index cannot outlive this call, which is the same reason -/// `eval_owned_input` normalizes `One`/`Many` to `Owned`/`ManyOwned`. This is -/// what lets one primitive serve the owned-surface consumers (`any`/`all`, -/// `IN`) as well as the cursor ones. -/// -/// #3036: `input` is an owned value this evaluator is about to re-index into -/// a throwaway document, so no `Snapshot`-origin `TrackedVar` marker `expr` -/// carries can name a node of it -- every such marker was frozen from some -/// earlier document, and the rebuilt copy is a different node however equal -/// its value. They are demoted here, at the re-entry, for the same reason -/// the generic evaluator's funnels demote before bridging (#2642): once the -/// marker reaches `Frame::certifies` there is nothing left to compare. A -/// caller that has *already* proven its markers against a live cursor -/// (`RootWitness::of` + [`demote_rebuilt_markers`]) takes -/// [`eval_each_owned_bridged`] instead, so the proof is not thrown away. -pub(crate) fn eval_each_owned( - expr: &Expr, - input: &OwnedValue, - optional: bool, - sink: &mut dyn FnMut(OwnedValue) -> Demand, -) -> Flow { - if let Some(flow) = eval_each_owned_fast_path::(expr, input, optional, sink) { - return flow; - } - // After the fast path on purpose: it never reaches a resolver, and - // demoting rebuilds `expr` whenever it holds a marker at all. - let expr = demote_for_owned_reentry(expr); - eval_each_owned_reindexed::(&expr, input, optional, sink) -} - -/// [`eval_each_owned`] for a reindex bridge whose markers were already -/// checked against the node being bridged (#2642's funnel sites in -/// `eval_generic.rs`): a `Snapshot` marker that survived that check *is* -/// the root of the document built here, and demoting it again would refuse -/// `. as $x | isempty(($x.a = 9))`-shaped programs jq accepts. Reach for -/// the demoting twin unless the call is preceded by exactly that check. -pub(crate) fn eval_each_owned_bridged( - expr: &Expr, - input: &OwnedValue, - optional: bool, - sink: &mut dyn FnMut(OwnedValue) -> Demand, -) -> Flow { - if let Some(flow) = eval_each_owned_fast_path::(expr, input, optional, sink) { - return flow; - } - eval_each_owned_reindexed::(expr, input, optional, sink) -} - -/// [`eval_each_owned`] or [`eval_each_owned_bridged`], chosen by whether -/// `value` is a node the resolver is *tracking* (#3036): inside a resolver -/// invocation, `trackable` means the ambient is the register -- a node -/// reached from the invocation's input by navigation alone -- so a marker -/// proven against that input is proven against this node too, and the -/// leaf, condition, computed key or bind source evaluated here must not -/// demote it (`path(select(($x.a = 9) | true))`, `.[($x.a = 9 | "a")] = -/// 5`). Once the register has moved off a node (a literal, a constructed -/// value), the ambient is a value that may be a rebuilt copy, and the -/// demoting entry applies. -fn eval_each_owned_at( - expr: &Expr, - value: &OwnedValue, - trackable: bool, - optional: bool, - sink: &mut dyn FnMut(OwnedValue) -> Demand, -) -> Flow { - if trackable { - eval_each_owned_bridged::(expr, value, optional, sink) - } else { - eval_each_owned::(expr, value, optional, sink) - } -} - /// [`eval_owned_fast_path`] delivered through `sink`: `Some` when the fast /// path answered, `None` when the caller has to reindex. fn eval_each_owned_fast_path( @@ -8115,13 +8000,47 @@ fn eval_each_owned_fast_path( }) } -/// The reindex half of [`eval_each_owned`]/[`eval_each_owned_bridged`]. -fn eval_each_owned_reindexed( +/// Owned-input twin of [`eval_each`], mirroring `eval_owned_input`. +/// +/// The sink takes `OwnedValue`, not `Item`: values produced against the +/// locally-built index cannot outlive this call, which is the same reason +/// `eval_owned_input` normalizes `One`/`Many` to `Owned`/`ManyOwned`. This is +/// what lets one primitive serve the owned-surface consumers (`any`/`all`, +/// `IN`) as well as the cursor ones. +/// +/// #3036: `input` is an owned value this evaluator is about to re-index into +/// a throwaway document, so no `Snapshot`-origin `TrackedVar` marker `expr` +/// carries can name a node of it -- every such marker was frozen from some +/// earlier document, and the rebuilt copy is a different node however equal +/// its value. Whether they are demoted here, and against what, is what +/// `reentry` says (#3122): a caller that has *already* proven its markers +/// against the document `input` belongs to -- a generic funnel that ran +/// [`reroot_for_reentry`] against `RootWitness::of` its cursor, a resolver +/// leaf whose register is still a navigated node of that document +/// (`path(select(($x.a = 9) | true))`, `.[($x.a = 9 | "a")] = 5`), a fold +/// or loop that demoted its static operand once -- passes +/// [`Reentry::Proven`], so the proof is not thrown away: a `Snapshot` marker +/// that survived it *is* the root of the document built here, and demoting +/// it again would refuse `. as $x | isempty(($x.a = 9))`-shaped programs jq +/// accepts. Every other caller names the value's root +/// ([`Reentry::Against`]; [`Reentry::REBUILT`] for a value no document node +/// backs), and the markers are demoted at the re-entry, for the same reason +/// the generic evaluator's funnels demote before bridging (#2642): once the +/// marker reaches `Frame::certifies` there is nothing left to compare. +pub(crate) fn eval_each_owned( expr: &Expr, input: &OwnedValue, optional: bool, + reentry: Reentry, sink: &mut dyn FnMut(OwnedValue) -> Demand, ) -> Flow { + if let Some(flow) = eval_each_owned_fast_path::(expr, input, optional, sink) { + return flow; + } + // After the fast path on purpose: it never reaches a resolver, and + // demoting rebuilds `expr` whenever it holds a marker at all. + let expr = reentry.reroot::(expr); + let expr = expr.as_ref(); // Same round trip, and the same `to_json_for_reindex` reasoning (#561), as // `eval_owned_input`. let json_str = input.to_json_for_reindex::(); @@ -10309,10 +10228,12 @@ fn eval_try<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // `try error("boom") catch .` yields "boom". QueryResult::Error(e) => match catch { // #3036: see `try_payload_root`. - Some(catch_expr) => { - let catch_expr = reroot_markers::(catch_expr, &try_payload_root(expr)); - eval_owned_input_bridged::(&catch_expr, &e.payload(), optional) - } + Some(catch_expr) => eval_owned_input::( + catch_expr, + &e.payload(), + optional, + Reentry::Against(try_payload_root(expr)), + ), None => QueryResult::None, }, // jq's `catch` catches a `break` the same way it catches a raised @@ -10321,7 +10242,9 @@ fn eval_try<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // marker — an implementation detail not worth replicating — so bind // `null` instead. QueryResult::Break(_) => match catch { - Some(catch_expr) => eval_owned_input::(catch_expr, &OwnedValue::Null, optional), + Some(catch_expr) => { + eval_owned_input::(catch_expr, &OwnedValue::Null, optional, Reentry::REBUILT) + } None => QueryResult::None, }, // Same #1620 decode-failure exclusion as the bare `Error` arm above @@ -10337,10 +10260,12 @@ fn eval_try<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( QueryResult::Partial(prefix, Control::Error(e)) => { let handled = match catch { // #3036: see `try_payload_root`. - Some(catch_expr) => { - let catch_expr = reroot_markers::(catch_expr, &try_payload_root(expr)); - eval_owned_input_bridged::(&catch_expr, &e.payload(), optional) - } + Some(catch_expr) => eval_owned_input::( + catch_expr, + &e.payload(), + optional, + Reentry::Against(try_payload_root(expr)), + ), None => QueryResult::None, }; prepend::<_, S>(prefix, handled) @@ -10350,9 +10275,12 @@ fn eval_try<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // handler bound to `null` and splice its result in after them. QueryResult::Partial(prefix, Control::Break(_)) => { let handled = match catch { - Some(catch_expr) => { - eval_owned_input::(catch_expr, &OwnedValue::Null, optional) - } + Some(catch_expr) => eval_owned_input::( + catch_expr, + &OwnedValue::Null, + optional, + Reentry::REBUILT, + ), None => QueryResult::None, }; prepend::<_, S>(prefix, handled) @@ -11504,7 +11432,7 @@ fn builtin_upper_in<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // .; .)` terms, and that sibling already threads the real ambient // `optional` into its own `gen` evaluation for exactly this reason. // #3036: `current` is this arm's own input, unrebuilt -- bridged. - let flow = eval_each_owned_bridged::(s, ¤t, optional, &mut |candidate| { + let flow = eval_each_owned::(s, ¤t, optional, Reentry::Proven, &mut |candidate| { if owned_value_eq::(&candidate, ¤t) { found += 1; Demand::Stop @@ -12043,7 +11971,7 @@ fn any_all_probe_element( target_truthy: bool, ) -> Result { let mut decided = false; - let flow = eval_each_owned::(cond, elem, false, &mut |out| { + let flow = eval_each_owned::(cond, elem, false, Reentry::REBUILT, &mut |out| { if out.is_truthy() == target_truthy { decided = true; Demand::Stop @@ -12222,21 +12150,17 @@ fn any_all_gen_cond<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // #3036: `owned` is this arm's own input, unrebuilt -- bridged. Each // element `gen` yields is a computed value, so `cond` runs on it through // the demoting entry. - let flow = - eval_each_owned_bridged::( - gen, - &owned, - optional, - &mut |elem| match any_all_probe_element::(cond, &elem, target_truthy) { - Ok(true) => { - matches += 1; - probe_escape = None; - Demand::Stop - } - Ok(false) => Demand::Continue, - Err(control) => stop_with_escape(&mut probe_escape, control), - }, - ); + let flow = eval_each_owned::(gen, &owned, optional, Reentry::Proven, &mut |elem| { + match any_all_probe_element::(cond, &elem, target_truthy) { + Ok(true) => { + matches += 1; + probe_escape = None; + Demand::Stop + } + Ok(false) => Demand::Continue, + Err(control) => stop_with_escape(&mut probe_escape, control), + } + }); // #1519: `probe_escape` is folded into `flow` here, and only here -- // not consulted unconditionally, which is what let a stale escape from @@ -14421,7 +14345,7 @@ fn builtin_with_entries<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // into its own throwaway document, so no `Snapshot` marker in `f` can // name a node of it -- see `eval_each_owned`. Demoted once, not per // entry. - let f = demote_for_owned_reentry(f); + let f = demote_for_reentry(f, &RootWitness::Owned); let f: &Expr = &f; let mut transformed: Vec = Vec::new(); for entry in entries { @@ -19170,7 +19094,8 @@ fn eval_sub_replacement<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // `test_sub_replacement_via_closure_param_*` in `tests/jq_cli_tests.rs` // for the pinning tests. let materialized = - eval_owned_input::(replacement_expr, &captures, optional).materialize_cursor(); + eval_owned_input::(replacement_expr, &captures, optional, Reentry::REBUILT) + .materialize_cursor(); let (values, trailing) = stream_outputs_lossy::<_, S>(materialized); if let Some(control) = trailing { return Err(partial(Vec::new(), control)); @@ -20738,7 +20663,7 @@ fn eval_pipe<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( QueryResult::Partial(vs, control) => QueryResult::Partial(vs, control), // #2182: was a wildcard `_ =>` -- verified by tracing // `eval_owned_pipe`'s full call chain (`eval_owned_input` -> - // `eval_owned_fast_path`/`eval_owned_input_reindexed` -> + // `eval_owned_fast_path`/`eval_owned_input_bridge` -> // `detach_from_temp_document`, the last already an // exhaustive per-variant match): every path resolves to // `Owned`/`None`/`Error`/`ManyOwned`/`Break`/`Halt`/ @@ -20854,7 +20779,7 @@ fn eval_owned_pipe<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // an owned stage answered `[1,2]` instead of `1` then `2`. // `reduce`/`foreach`/path-tracking still want that single-value collapse, so // only the pipe continuation is widened here. - eval_owned_input::(&rest_expr, &value, optional) + eval_owned_input::(&rest_expr, &value, optional, Reentry::REBUILT) } /// Find a field in an object by name. @@ -23684,7 +23609,7 @@ pub fn eval_owned_with_file_index<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics> file_origin: &[usize], ) -> QueryResult<'a, W> { if !needs_path_context(expr) { - return eval_owned_input::(expr, input, false); + return eval_owned_input::(expr, input, false, Reentry::REBUILT); } // Spine 2416 step 5: through the owned door like every other path-context // entry, instead of straight into the eager evaluator (the audit's @@ -23770,7 +23695,7 @@ pub fn eval_documents_together<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( use super::eval_generic::{reindex_bridge_is_identity, NodeOrigin, YqDocument}; use crate::json::JsonIndex; - // Same throwaway document `eval_owned_input_reindexed` builds, and the + // Same throwaway document `eval_owned_input_bridge` builds, and the // same `to_json_for_reindex` (not `to_json`) for the same reason (#561) -- // one per input document rather than one for a combined array. let texts: Vec = documents @@ -24406,8 +24331,13 @@ fn eval_update_impl<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // `|=` was called on, untouched -- see the eager loop below. let ambient = core::mem::replace(&mut untouched, false) && is_root_path(path); if ambient { - return update_root_with_filter::(result, filter_expr, pos.as_ref(), true) - .map(|_wrote| ()); + return update_root_with_filter::( + result, + filter_expr, + pos.as_ref(), + Reentry::Proven, + ) + .map(|_wrote| ()); } // `false` for `scalar_noop`, as the eager route computes it in jq // mode. @@ -24546,7 +24476,7 @@ fn eval_update_impl<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // very document `|=` was called on, untouched -- jq's filter then // sees `$x`'s own node. let outcome = if i == 0 && is_root_path(path) { - update_root_with_filter::(&mut result, filter_expr, pos.as_ref(), true) + update_root_with_filter::(&mut result, filter_expr, pos.as_ref(), Reentry::Proven) } else { update_path::( &mut result, @@ -24611,7 +24541,7 @@ fn collect_rhs_outputs<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // cursor route too, so nothing regresses either way; see // `docs/compliance/yq/limitations.md`.) let evaluated = match vivified { - Some(doc) => eval_owned_input::(value_expr, doc, optional), + Some(doc) => eval_owned_input::(value_expr, doc, optional, Reentry::REBUILT), None => eval_single::(value_expr, input.clone(), optional), }; match evaluated.materialize_cursor() { @@ -27358,27 +27288,21 @@ fn is_root_path(path: &Expr) -> bool { /// back -- [`update_path`]'s `Expr::Identity` arm, shared with /// [`eval_update_impl`]'s lone-root-path case. /// -/// `ambient` says `root` is still the document `|=` was called on, untouched -/// (#3036): the filter then runs through the non-demoting bridge, so -/// `. as $x | . |= ($x.a = 9)` keeps writing, as it does in jq (the filter's -/// input *is* `$x`'s node). Every other leaf -- a sub-path, or a root a -/// previous path of the same update already wrote -- is a value jq's own -/// `setpath` has copied, and takes the demoting bridge. +/// `reentry` is [`Reentry::Proven`] when `root` is still the document `|=` +/// was called on, untouched (#3036): the filter then runs without demoting, +/// so `. as $x | . |= ($x.a = 9)` keeps writing, as it does in jq (the +/// filter's input *is* `$x`'s node). Every other leaf -- a sub-path, or a +/// root a previous path of the same update already wrote -- is a value jq's +/// own `setpath` has copied, and passes [`Reentry::REBUILT`]. fn update_root_with_filter( root: &mut OwnedValue, filter_expr: &Expr, pos: Option<&UpdatePos<'_>>, - ambient: bool, + reentry: Reentry, ) -> Result { let positioned = position_update_filter::(filter_expr, pos)?; let filter_expr = positioned.as_ref().unwrap_or(filter_expr); - let run = || { - if ambient { - eval_owned_multi_first_bridged::(filter_expr, root) - } else { - eval_owned_multi_first::(filter_expr, root) - } - }; + let run = || eval_owned_multi_first::(filter_expr, root, reentry); let outputs = match pos { Some(pos) => super::eval_generic::with_absolute_path_base(pos.path.clone(), run)?, None => run()?, @@ -27467,7 +27391,7 @@ fn update_path( // nested assignment inside the filter (`.a | .b |= (.c |= path)`), // which reaches `eval_update` through the ordinary dispatch with // no parameter to ride on. - update_root_with_filter::(root, filter_expr, pos, false) + update_root_with_filter::(root, filter_expr, pos, Reentry::REBUILT) } Expr::Field(name) => { let root_was_null = matches!(root, OwnedValue::Null); @@ -28576,17 +28500,9 @@ fn eval_owned_multi( fn eval_owned_multi_first( expr: &Expr, input: &OwnedValue, + reentry: Reentry, ) -> Result, EvalEscape> { - first_outputs::(eval_owned_input::, S>(expr, input, false)) -} - -/// [`eval_owned_multi_first`] over the update's own untouched input -- see -/// [`update_root_with_filter`] (#3036). -fn eval_owned_multi_first_bridged( - expr: &Expr, - input: &OwnedValue, -) -> Result, EvalEscape> { - first_outputs::(eval_owned_input_bridged::, S>(expr, input, false)) + first_outputs::(eval_owned_input::, S>(expr, input, false, reentry)) } /// [`eval_owned_multi_first`]'s first-output rule over an owned result. @@ -28678,7 +28594,7 @@ fn eval_owned_multi_keep_partial( expr: &Expr, input: &OwnedValue, ) -> (Vec, Option) { - let result = eval_owned_input::, S>(expr, input, false); + let result = eval_owned_input::, S>(expr, input, false, Reentry::REBUILT); let mut out = Vec::new(); let escape = push_owned_values_lossy::<_, S>(result, &mut out).map(EvalEscape::from); (out, escape) @@ -29098,6 +29014,61 @@ impl RootWitness { } } +/// What an owned re-entry -- a call that builds a throwaway document out of +/// an `OwnedValue` and evaluates an expression over it -- may assume about +/// the `Snapshot` markers in that expression (#2642/#3036/#3037, #3122): +/// whether they were already rerooted for this value's document, or must be +/// rerooted against a named [`RootWitness`] first. Every such entry takes one of +/// these instead of offering a demoting and a non-demoting twin, so a new +/// re-entry has to say what its value's root is rather than guess a name. +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) enum Reentry { + /// Already rerooted for this value's document upstream: the value is + /// that document's root or a node reached from it by navigation alone (a + /// generic funnel or an enclosing re-entry rerooted the markers against + /// its root), or the caller ran [`reroot_for_reentry`] (or its + /// demote-only twin [`demote_for_reentry`], for an `Owned` root) on this + /// same expression itself (a fold or loop hoisting a static operand out + /// of its per-element path). Nothing left to rewrite. + Proven, + /// The value may be a rebuilt copy of whatever the markers name, or the + /// very node a value-mode bind was frozen from: reroot against this + /// witness before bridging -- #2642's demotion and, in jq mode, #3037's + /// promotion, in one walk ([`reroot_for_reentry`]). + Against(RootWitness), +} + +impl Reentry { + /// No document node backs the value (an accumulator, a produced child, + /// a literal, a `catch`-less `null`): every `Snapshot` marker is a copy. + pub(crate) const REBUILT: Self = Self::Against(RootWitness::Owned); + + /// A resolver sink's own conversion of its register provenance (#3036): + /// `trackable` means the register is a node reached from the + /// invocation's input by navigation alone, and that input's markers were + /// demoted by whichever funnel or re-entry built the document it is the + /// root of -- so a marker proven against the input is proven here too. + /// Once the register has moved off a node (a literal, a constructed + /// value), the ambient may be a rebuilt copy and is treated as one. + fn at_register(trackable: bool) -> Self { + if trackable { + Self::Proven + } else { + Self::REBUILT + } + } + + /// The expression to bridge: `expr` itself under [`Reentry::Proven`], + /// its [`reroot_for_reentry`] twin under [`Reentry::Against`]. + #[inline(always)] + pub(crate) fn reroot(self, expr: &Expr) -> Cow<'_, Expr> { + match self { + Self::Proven => Cow::Borrowed(expr), + Self::Against(root) => reroot_for_reentry::(expr, &root), + } + } +} + /// What a `catch` handler's `Snapshot` markers are checked against before /// they cross into `eval.rs` (#3036): the payload of `try BODY` is an owned /// value with no node identity of its own, so it is `Owned` -- which demotes @@ -29319,15 +29290,20 @@ pub(crate) fn reroot_markers<'e, S: EvalSemantics>( expr: &'e Expr, root: &RootWitness, ) -> Cow<'e, Expr> { - rewrite_markers(expr, &|marker| { - if marker_needs_demotion(marker, root) { - Some(Origin::Untracked) - } else if S::TAG == EvalTag::Jq && marker_is_root(marker, root) { - Some(Origin::Snapshot) - } else { - None - } - }) + rewrite_markers(expr, &|marker| reroot_rewrite::(marker, root)) +} + +/// [`reroot_markers`]' per-marker rule -- demote, promote, or leave -- +/// shared with [`reroot_for_reentry`]'s precheck (#3122) so the two cannot +/// disagree about which marker a re-entry rewrites. +fn reroot_rewrite(marker: &Tracked, root: &RootWitness) -> Option { + if marker_needs_demotion(marker, root) { + Some(Origin::Untracked) + } else if S::TAG == EvalTag::Jq && marker_is_root(marker, root) { + Some(Origin::Snapshot) + } else { + None + } } /// Rebuild `expr` with every [`Expr::TrackedVar`] marker for which `rewrite` @@ -29413,11 +29389,15 @@ fn has_demotable_marker(expr: &Expr, root: &RootWitness) -> bool { ) } -/// [`demote_rebuilt_markers`] against [`RootWitness::Owned`] for an -/// owned-value re-entry into this evaluator (#3036), skipping the rebuild -/// when nothing in `expr` could observe it. +/// [`reroot_markers`] against `root` for an owned-value re-entry into this +/// evaluator, skipping the rebuild when nothing in `expr` could observe it +/// (#3036's precheck over #3037's rewrite) -- the one derivation every +/// [`Reentry::Against`] runs, funnel witnesses included (#3122). +/// [`demote_for_reentry`] is its demote-only twin for a root that is +/// always [`RootWitness::Owned`], where the promotion is a no-op and the +/// name says what happens. /// -/// A demotion is read in exactly one place, `Frame::certifies`, and a +/// A demotion or promotion is read in exactly one place, `Frame::certifies`, and a /// `Frame` exists only inside a resolver invocation -- which starts from an /// assignment (`Expr::Assign`/`Update`/`CompoundAssign`/`AlternativeAssign`/ /// `MetaAssign`), a builtin (`del`, `path`, `sort_keys`, `pick`, every @@ -29430,26 +29410,49 @@ fn has_demotable_marker(expr: &Expr, root: &RootWitness) -> bool { /// rebuilding it would cost a clone of every bound value per re-entry /// (measured +5% on that shape, both architectures) for no observable /// change. -fn demote_for_owned_reentry(expr: &Expr) -> Cow<'_, Expr> { +pub(crate) fn reroot_for_reentry<'e, S: EvalSemantics>( + expr: &'e Expr, + root: &RootWitness, +) -> Cow<'e, Expr> { + if !reentry_can_observe(expr, &|marker| reroot_rewrite::(marker, root)) { + return Cow::Borrowed(expr); + } + reroot_markers::(expr, root) +} + +/// [`reroot_for_reentry`] without the promotion: [`demote_rebuilt_markers`] +/// behind the same precheck, for the hoists whose root is always +/// [`RootWitness::Owned`] (a fold's UPDATE, a loop's operands, +/// `with_entries`' `f`, every level of a recurse walk below the first). +pub(crate) fn demote_for_reentry<'e>(expr: &'e Expr, root: &RootWitness) -> Cow<'e, Expr> { + if !reentry_can_observe(expr, &|marker| { + marker_needs_demotion(marker, root).then_some(Origin::Untracked) + }) { + return Cow::Borrowed(expr); + } + demote_rebuilt_markers(expr, root) +} + +/// The precheck [`reroot_for_reentry`] and [`demote_for_reentry`] share: +/// whether `expr` holds a marker `rewrite` would change *and* a node that +/// can start a resolver invocation -- the only reader of the change. When +/// either is missing the re-entry hands `expr` back borrowed. +fn reentry_can_observe(expr: &Expr, rewrite: &dyn Fn(&Tracked) -> Option) -> bool { // One walk for both questions: this runs once per re-entry, which on a // per-element shape is once per element, so a second pass over a // marker-bearing body was measurable (about +1% on a 7950X). - let mut demotable = false; + let mut rewritable = false; let mut resolver = false; any_subexpr(expr, &mut |e| { - demotable |= matches!(e, Expr::TrackedVar(marker) - if marker_needs_demotion(marker, &RootWitness::Owned)); + rewritable |= matches!(e, Expr::TrackedVar(marker) if rewrite(marker).is_some()); resolver |= may_enter_resolver_node(e); - demotable && resolver + rewritable && resolver }); - if !(demotable && resolver) { - return Cow::Borrowed(expr); - } - demote_rebuilt_markers(expr, &RootWitness::Owned) + rewritable && resolver } /// Whether evaluating `node` itself can start a resolver invocation -- see -/// [`demote_for_owned_reentry`], which asks this of every node in the +/// [`reentry_can_observe`], which asks this of every node in the /// expression. Conservative: every builtin and every call counts (a `def` /// body, a resolved `DefCall` and a `Shared` argument are walked too). fn may_enter_resolver_node(node: &Expr) -> bool { @@ -30339,15 +30342,19 @@ fn resolve_cond_fork_stream( mut dispatch: impl FnMut(bool) -> ResolveFlow, ) -> ResolveFlow { let mut dispatched: Option = None; - let flow = eval_each_owned_at::(cond, value, trackable, false, &mut |c| match dispatch( - c.is_truthy(), - ) { - ResolveFlow::Exhausted => Demand::Continue, - other => { - dispatched = Some(other); - Demand::Stop - } - }); + let flow = eval_each_owned::( + cond, + value, + false, + Reentry::at_register(trackable), + &mut |c| match dispatch(c.is_truthy()) { + ResolveFlow::Exhausted => Demand::Continue, + other => { + dispatched = Some(other); + Demand::Stop + } + }, + ); resolve_stream_flow(flow, dispatched) } @@ -31255,10 +31262,16 @@ fn resolve_node_sink<'a, S: EvalSemantics>( // keep" reasoning `resolve_index_expr`'s own key/target evaluation // escapes already document). Expr::Builtin(Builtin::DebugMsg(msg)) => { - let flow = eval_each_owned_at::(msg, value, trackable, false, &mut |msg_value| { - write_debug_line::(&msg_value); - Demand::Continue - }); + let flow = eval_each_owned::( + msg, + value, + false, + Reentry::at_register(trackable), + &mut |msg_value| { + write_debug_line::(&msg_value); + Demand::Continue + }, + ); if let Flow::Escaped(control) = flow { return ResolveFlow::Escaped(EvalEscape::from(control)); } @@ -32351,30 +32364,36 @@ fn resolve_leaf_sink<'a, S: EvalSemantics>( let mut delivered = 0usize; let mut stopped_by_sink = false; - let flow = eval_each_owned_at::(expr, value, trackable, false, &mut |v| { - delivered += 1; - let branch = PathBranch::untracked(Cow::Owned(v)) - .with_register(trackable.then(|| Cow::Borrowed(value))); - // `untracked_branches`' own rule, applied one value at a time -- - // see its doc comment for why the register is recorded here. - let demand = sink(branch); - if demand == Demand::Stop { - stopped_by_sink = true; - } - // Two independent reasons to stop, folded into one answer: the - // sink asked, or this leaf has delivered its own `keep` bound. - // Only the first is reachable with today's bounded consumers -- - // `resolve_bounded_sink` (limit/first) and the nth arm both answer - // `Demand::Stop` from `sink(branch)` on the very branch that - // reaches the count they narrowed `keep` to. The second is kept - // because honouring `keep` is this function's own contract with - // #1872, not something to inherit from whoever is downstream. - if demand == Demand::Stop || delivered >= limit { - Demand::Stop - } else { - Demand::Continue - } - }); + let flow = eval_each_owned::( + expr, + value, + false, + Reentry::at_register(trackable), + &mut |v| { + delivered += 1; + let branch = PathBranch::untracked(Cow::Owned(v)) + .with_register(trackable.then(|| Cow::Borrowed(value))); + // `untracked_branches`' own rule, applied one value at a time -- + // see its doc comment for why the register is recorded here. + let demand = sink(branch); + if demand == Demand::Stop { + stopped_by_sink = true; + } + // Two independent reasons to stop, folded into one answer: the + // sink asked, or this leaf has delivered its own `keep` bound. + // Only the first is reachable with today's bounded consumers -- + // `resolve_bounded_sink` (limit/first) and the nth arm both answer + // `Demand::Stop` from `sink(branch)` on the very branch that + // reaches the count they narrowed `keep` to. The second is kept + // because honouring `keep` is this function's own contract with + // #1872, not something to inherit from whoever is downstream. + if demand == Demand::Stop || delivered >= limit { + Demand::Stop + } else { + Demand::Continue + } + }, + ); // Halt first, exactly as the collecting form checks `trailing` first: an // already-triggered halt must never be downgraded into a catchable path @@ -32481,10 +32500,16 @@ fn resolve_leaf_bounded<'a, S: EvalSemantics>( // answers `Stop` (`drain_result`'s own doc comment states that // invariant). let mut values = Vec::new(); - let flow = eval_each_owned_at::(expr, value, trackable, false, &mut |v| { - values.push(v); - Demand::Continue - }); + let flow = eval_each_owned::( + expr, + value, + false, + Reentry::at_register(trackable), + &mut |v| { + values.push(v); + Demand::Continue + }, + ); // `Flow::Stopped` cannot actually occur here — this sink never // answers `Stop` — but folded to "no trailing control" rather than // `unreachable!()`, mirroring `any_all_gen_cond`'s own "a defensible @@ -32622,14 +32647,20 @@ fn resolve_leaf<'a, S: EvalSemantics>( // generator for every value — see [`drive_fold_source`]. let limit = keep.limit(); let mut values: Vec = Vec::new(); - let flow = eval_each_owned_at::(expr, value, trackable, false, &mut |v| { - values.push(v); - if values.len() >= limit { - Demand::Stop - } else { - Demand::Continue - } - }); + let flow = eval_each_owned::( + expr, + value, + false, + Reentry::at_register(trackable), + &mut |v| { + values.push(v); + if values.len() >= limit { + Demand::Stop + } else { + Demand::Continue + } + }, + ); // Halt-first, exactly as the eager version checked `trailing` first — // an already-triggered halt (whether it escaped bare, or is `pending` @@ -34073,12 +34104,18 @@ fn drive_fold_source_by_value( trackable: bool, step: &mut dyn FnMut(FoldSourceValue) -> Demand, ) -> Option { - match eval_each_owned_at::(source, value, trackable, false, &mut |value| { - step(FoldSourceValue { - value, - register_path: None, - }) - }) { + match eval_each_owned::( + source, + value, + false, + Reentry::at_register(trackable), + &mut |value| { + step(FoldSourceValue { + value, + register_path: None, + }) + }, + ) { Flow::Exhausted | Flow::Stopped { .. } => None, Flow::Escaped(control) => Some(control), } @@ -34290,15 +34327,19 @@ fn resolve_bind_source_sink( } let mut stashed: Option = None; - let flow = eval_each_owned_at::(source, value, trackable, false, &mut |bound| match bind( - bound, None, - ) { - ResolveFlow::Exhausted => Demand::Continue, - other => { - stashed = Some(other); - Demand::Stop - } - }); + let flow = eval_each_owned::( + source, + value, + false, + Reentry::at_register(trackable), + &mut |bound| match bind(bound, None) { + ResolveFlow::Exhausted => Demand::Continue, + other => { + stashed = Some(other); + Demand::Stop + } + }, + ); resolve_stream_flow(flow, stashed) } @@ -34606,7 +34647,8 @@ fn resolve_as_pattern<'a, S: EvalSemantics>( // does (#3036 review): while `value` is the register's own node, `source` // must not have its own `Snapshot` markers demoted just because this // arm evaluates by value instead of through a sink. - let (sources, trailing) = eval_owned_expr_fork_at::(source, value, trackable, false); + let (sources, trailing) = + eval_owned_expr_fork::(source, value, false, Reentry::at_register(trackable)); let head = unwrap_paren(source); let all_names = pattern_alternatives_var_names(patterns); let last_idx = patterns.len() - 1; @@ -36758,16 +36800,26 @@ pub(crate) struct RecurseWalkEnd { /// levels (fully native), +18% at 40 (the boundary), +13% at 100, +7% at /// 200 (mostly queued past the budget), the ratio falling as the queued /// (unchanged) portion of the walk grows. +/// +/// `reentry` is what `f` may assume about `root` (#3122): the level-0 node +/// is the caller's own input, and `f` runs on it as `reentry` says -- +/// [`Reentry::Proven`] from `eval.rs`'s own arms, whose input the enclosing +/// re-entry already demoted for, [`Reentry::Against`] the cursor's witness +/// from the generic funnel. Every deeper node is a value `f` produced, so +/// `f`/`cond` run there through their `Owned`-demoted twins, computed once +/// here rather than per visited node (#3036 review). pub(crate) fn each_recurse_walk( f: &Expr, cond: Option<&Expr>, root: OwnedValue, + reentry: Reentry, sink: &mut dyn FnMut(OwnedValue) -> Demand, ) -> RecurseWalkEnd { - let demoted_f = demote_for_owned_reentry(f); - let demoted_cond = cond.map(demote_for_owned_reentry); + let f = reentry.reroot::(f); + let demoted_f = demote_for_reentry(&f, &RootWitness::Owned); + let demoted_cond = cond.map(|c| demote_for_reentry(c, &RootWitness::Owned)); let mut walk = ValueRecurseWalk:: { - f, + f: &f, cond, demoted_f: demoted_f.as_ref(), demoted_cond: demoted_cond.as_deref(), @@ -37100,7 +37152,7 @@ mod recurse_native_levels_override { struct ValueRecurseWalk<'e, 'd, 's, S> { f: &'e Expr, cond: Option<&'e Expr>, - /// [`demote_for_owned_reentry`] of `f`/`cond`, computed once by + /// [`demote_for_reentry`] (against `Owned`) of `f`/`cond`, computed once by /// [`each_recurse_walk`] instead of per visited node (#3036 review): /// `f`/`cond` are the same static expression at every level, so /// re-walking either's AST on every native-recursion `expand`/`gate` @@ -37151,9 +37203,9 @@ impl ValueRecurseWalk<'_, '_, '_, S> { stop_on_abort(&mut abort, end) }; let flow = if level == 0 { - eval_each_owned_bridged::(f, &node, false, &mut run) + eval_each_owned::(f, &node, false, Reentry::Proven, &mut run) } else { - eval_each_owned_bridged::(demoted_f, &node, false, &mut run) + eval_each_owned::(demoted_f, &node, false, Reentry::Proven, &mut run) }; native_recurse_end(abort, flow) } @@ -37173,7 +37225,7 @@ impl ValueRecurseWalk<'_, '_, '_, S> { fn gate(&mut self, cond: &Expr, child: OwnedValue, level: u32) -> Option { let _scope = self.budget.scope(); let mut abort = None; - let flow = eval_each_owned_bridged::(cond, &child, false, &mut |verdict| { + let flow = eval_each_owned::(cond, &child, false, Reentry::Proven, &mut |verdict| { if verdict.is_truthy() { stop_on_abort(&mut abort, self.visit(child.clone(), level)) } else { @@ -37284,7 +37336,9 @@ fn each_recurse<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Ok(v) => v, Err(e) => return Flow::Escaped(Control::Error(e)), }; - let end = each_recurse_walk::(f, cond, root, &mut |v| sink(Item::Owned(v))); + let end = each_recurse_walk::(f, cond, root, Reentry::Proven, &mut |v| { + sink(Item::Owned(v)) + }); recurse_walk_flow(end) } @@ -37579,13 +37633,19 @@ impl<'a, S: EvalSemantics> PathRecurseWalk<'_, 'a, '_, S> { ) -> Option { let _scope = self.budget.scope(); let mut abort = None; - let flow = eval_each_owned::(cond, &child.value, false, &mut |verdict| { - if open && verdict.is_truthy() { - stop_on_abort(&mut abort, self.visit(Self::delivered(&child), level)) - } else { - Demand::Continue - } - }); + let flow = eval_each_owned::( + cond, + &child.value, + false, + Reentry::REBUILT, + &mut |verdict| { + if open && verdict.is_truthy() { + stop_on_abort(&mut abort, self.visit(Self::delivered(&child), level)) + } else { + Demand::Continue + } + }, + ); native_recurse_end(abort, flow) } @@ -37959,7 +38019,7 @@ fn drive_index_key( trackable: bool, sink: &mut dyn FnMut(OwnedValue) -> Demand, ) -> Option { - match eval_each_owned_at::(key, value, trackable, false, sink) { + match eval_each_owned::(key, value, false, Reentry::at_register(trackable), sink) { Flow::Exhausted => None, // `pending` is dropped, with every other Stage-2 consumer (see // [`Flow::Stopped`]): the caller stops only because `target` already @@ -38899,9 +38959,13 @@ fn drive_slice_bound( // same lossy materialization -- and falls back to the eager evaluator // for any `Expr` with no native lazy arm, which simply reproduces the // old ordering for that shape rather than changing anything. - let flow = eval_each_owned_at::(expr, value, trackable, false, &mut |raw| { - sink(PathSliceBound::classify(raw, round)) - }); + let flow = eval_each_owned::( + expr, + value, + false, + Reentry::at_register(trackable), + &mut |raw| sink(PathSliceBound::classify(raw, round)), + ); match flow { Flow::Exhausted => Ok(None), // `pending` is dropped, with every other Stage-2 consumer (see @@ -41501,7 +41565,6 @@ fn try_reduce_step_alternatives( acc_input: OwnedValue, optional: bool, budget: &mut usize, - resolver_free: bool, ) -> (OwnedValue, Option) { let last_idx = patterns.len() - 1; let invert_dedup = patterns.len() > 1; @@ -41545,16 +41608,10 @@ fn try_reduce_step_alternatives( // `update_vals.into_iter().last()` read off the collected `Vec`. let mut last_val: Option = None; let step_state = core::mem::replace(&mut state, OwnedValue::Null); - let flow = fold_step_each::( - &substituted, - step_state, - optional, - resolver_free, - &mut |v| { - last_val = Some(v); - Demand::Continue - }, - ); + let flow = fold_step_each::(&substituted, step_state, optional, &mut |v| { + last_val = Some(v); + Demand::Continue + }); // Unconditional, mirroring the pre-#1365 single-pattern fold's own // "acc = update_vals.into_iter().last()" -- run even when `flow` is // `Escaped`, since a retried alternative resumes from exactly this @@ -41950,7 +42007,6 @@ fn fold_step_each( expr: &Expr, state: OwnedValue, optional: bool, - resolver_free: bool, on_update: &mut dyn FnMut(OwnedValue) -> Demand, ) -> Flow { match owned_arith_accumulator_shape(expr) { @@ -41962,25 +42018,22 @@ fn fold_step_each( Err(_) if optional => Flow::Exhausted, Err(e) => Flow::Escaped(Control::Error(e)), }, - // #3036: `resolver_free` is [`fold_update_is_resolver_free`] of the - // fold's own UPDATE, decided once per fold rather than once per - // element -- a re-entry's demotion is unobservable without a - // resolver, and the walk that decides it was +3% on a tight - // `reduce` over 24k elements (7950X). - None if resolver_free => eval_each_owned_bridged::(expr, &state, optional, on_update), - None => eval_each_owned::(expr, &state, optional, on_update), + // #3036/#3122: `Proven` because `reduce_forks`/`foreach_forks` + // ran [`demote_for_reentry`] on the fold's own UPDATE once, against + // `Owned` (the accumulator is never a document node), before any + // element was substituted into it -- `substitute_foreach_steps` + // substitutes the loop variable through `substitute_vars`, which + // replaces `Expr::Var` with the value's own literal (`owned_to_expr`) + // and never with a marker (a node-less `Snapshot` marker *would* be + // demotable; the tracked substitution belongs to the resolver's own + // folds, which never come through here) nor with one of + // `may_enter_resolver_node`'s shapes, so the per-element copy needs + // no walk of its own (that walk was +3% on a tight `reduce` over 24k + // elements, 7950X). + None => eval_each_owned::(expr, &state, optional, Reentry::Proven, on_update), } } -/// Whether a fold's UPDATE (or EXTRACT) can never start a resolver -/// invocation, so the per-element re-entry may skip its demotion (#3036). -/// Static per fold: substituting the loop variable replaces `Expr::Var` -/// with a marker or a literal and never introduces one of -/// [`may_enter_resolver_node`]'s shapes. -fn fold_update_is_resolver_free(update: &Expr) -> bool { - !any_subexpr(update, &mut may_enter_resolver_node) -} - /// The `Identity`/`Field`/`Index` arms of [`eval_owned_fast_path`], factored /// out (#2201 review follow-up to #2048) so [`eval_owned_pure`]'s own /// navigation arm can call them directly instead of round-tripping through @@ -42244,7 +42297,7 @@ fn produces_fresh_value(expr: &Expr) -> bool { /// operator ([`apply_compare_op`], [`literal_to_owned`], [`owned_type_name`]'s /// mapping, `is_truthy`), rather than restating the rule. The pinning test is /// `eval_owned_pure_agrees_with_the_reindex_bridge`, which runs both this and -/// the reindex bridge (via [`eval_owned_input_reindexed`]) over a matrix of +/// the reindex bridge (via [`eval_owned_input_bridge`]) over a matrix of /// expressions x values and asserts they agree. /// /// Returns `None` for anything outside its own grammar, and for a @@ -42521,7 +42574,7 @@ fn eval_owned_expr_full( // `Snapshot` marker can name -- see `eval_each_owned`. After the fast // path on purpose: it never reaches a resolver, and demoting is a // rebuild of `expr` whenever a marker is present. - let expr = demote_for_owned_reentry(expr); + let expr = Reentry::REBUILT.reroot::(expr); // Create a synthetic JSON from the owned value // For simplicity, we'll serialize and reparse @@ -42672,10 +42725,17 @@ fn eval_owned_expr_opt( /// The returned result borrows nothing from the temporary document — every /// variant it produces is owned — so it is free to satisfy any caller's `'a` /// and `W`, the same way [`eval_owned_pipe`] does. +/// +/// `reentry` says whether the `Snapshot` markers in `expr` are already +/// demoted for `input`'s document or must be demoted against a named root +/// first -- see [`eval_each_owned`], this function's sink-shaped twin, for +/// the rule every caller follows (#3036, #3122). +#[inline(always)] fn eval_owned_input<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( expr: &Expr, input: &OwnedValue, optional: bool, + reentry: Reentry, ) -> QueryResult<'a, W> { if let Some(result) = eval_owned_fast_path::(expr, input, optional) { return match result { @@ -42684,57 +42744,20 @@ fn eval_owned_input<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Err(e) => e.into(), }; } - - eval_owned_input_reindexed::(expr, input, optional) -} - -/// [`eval_owned_input`] for a caller whose `input` is its own ambient value -/// materialized as-is (#3036) -- `to_owned` of the node the enclosing arm -/// was handed, with no stage between: the node a `Snapshot` marker was -/// frozen from or proven against is still the node it certifies against, -/// so nothing here may demote it. See [`eval_each_owned_bridged`]; a caller -/// handing over a *computed* value (an accumulator, a stage output, a -/// payload) takes [`eval_owned_input`]. -fn eval_owned_input_bridged<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( - expr: &Expr, - input: &OwnedValue, - optional: bool, -) -> QueryResult<'a, W> { - if let Some(result) = eval_owned_fast_path::(expr, input, optional) { - return match result { - Ok(Some(v)) => QueryResult::Owned(v), - Ok(None) => QueryResult::None, - Err(e) => e.into(), - }; - } - - eval_owned_input_reindexed_bridged::(expr, input, optional) + // After the fast path on purpose: it never reaches a resolver, and + // demoting rebuilds `expr` whenever it holds a marker at all. + eval_owned_input_bridge::(&reentry.reroot::(expr), input, optional) } -/// [`eval_owned_input`]'s reindex bridge, with no fast-path pre-check: build a -/// throwaway JSON document out of `input` and run the ordinary cursor -/// evaluator against it. -/// -/// Split out of [`eval_owned_input`] (#2048) so a test can force the bridge +/// [`eval_owned_input`]'s bridge alone, no fast path and no demotion: build +/// a throwaway JSON document out of `input` and run the ordinary cursor +/// evaluator against it. Split out (#2048) so a test can force the bridge /// for a shape [`eval_owned_fast_path`] now answers directly, and assert the /// two agree — the same pinning `eval_owned_fast_path_agrees_with_index_ /// object_by_name_and_index_array_by_position` does for #491's arms via its /// own `via_cursor`, but for arbitrary expressions rather than three hardcoded -/// ones. Behaviour is unchanged: this is the identical body, relocated. -fn eval_owned_input_reindexed<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( - expr: &Expr, - input: &OwnedValue, - optional: bool, -) -> QueryResult<'a, W> { - // #3036: an owned value re-indexed here is a fresh document no - // `Snapshot` marker can name -- see `eval_each_owned`. - let expr = demote_for_owned_reentry(expr); - eval_owned_input_reindexed_bridged::(&expr, input, optional) -} - -/// [`eval_owned_input_reindexed`] without the demotion -- see -/// [`eval_each_owned_bridged`] for when that is right. -fn eval_owned_input_reindexed_bridged<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( +/// ones. +fn eval_owned_input_bridge<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( expr: &Expr, input: &OwnedValue, optional: bool, @@ -42757,7 +42780,7 @@ fn eval_owned_input_reindexed_bridged<'a, W: Clone + AsRef<[u64]>, S: EvalSemant /// Own every value in `result` so it stops borrowing the throwaway document /// it was evaluated against, and can satisfy any caller's `'a`/`W`. /// -/// One definition shared by [`eval_owned_input_reindexed`] and +/// One definition shared by [`eval_owned_input_bridge`] and /// [`eval_path_context_pipe_owned`] (spine 2416, step 5): both build a /// temporary `JsonIndex` over `to_json_for_reindex` output, so both have to /// detach before returning, and a second hand-written copy of this match is @@ -43211,7 +43234,6 @@ fn try_foreach_step_alternatives( state_input: OwnedValue, optional: bool, budget: &mut usize, - resolver_free: bool, sink: &mut dyn FnMut(OwnedValue) -> Demand, ) -> (OwnedValue, Flow) { let last_idx = patterns.len() - 1; @@ -43327,11 +43349,13 @@ fn try_foreach_step_alternatives( // before reading `ext_control` used to encode // positionally (#494): here they are simply already // pushed by the time the `Flow` is inspected. - if resolver_free { - eval_each_owned_bridged::(ext_expr, &update_val, optional, sink) - } else { - eval_each_owned::(ext_expr, &update_val, optional, sink) - } + eval_each_owned::( + ext_expr, + &update_val, + optional, + Reentry::Proven, + sink, + ) } // EXTRACT omitted is EXTRACT `.` (jq desugars `foreach f as // $x (init; update)` to `(init; update; .)`), so the push @@ -43407,13 +43431,8 @@ fn try_foreach_step_alternatives( }; let step_state = core::mem::replace(&mut state, OwnedValue::Null); - let update_flow = fold_step_each::( - &substituted_update, - step_state, - optional, - resolver_free, - on_update, - ); + let update_flow = + fold_step_each::(&substituted_update, step_state, optional, on_update); match step_outcome { Some(StepOutcome::Retry(update_val)) => { @@ -43768,9 +43787,14 @@ pub(crate) fn foreach_forks( drive_source: ForeachSourceDrive<'_>, sink: &mut dyn FnMut(OwnedValue) -> Demand, ) -> Flow { - // #3036: decided once per fold, see `fold_update_is_resolver_free`. - let resolver_free = - fold_update_is_resolver_free(update) && extract.map_or(true, fold_update_is_resolver_free); + // #3036/#3122: UPDATE/EXTRACT rerun against the fold's own accumulator, + // never a document node, so their `Snapshot` markers are demoted against + // `Owned` -- once per fold, here, rather than once per element (see + // `fold_step_each`); the fold's own callers pass them in as written. + let update = demote_for_reentry(update, &RootWitness::Owned); + let update: &Expr = &update; + let extract = extract.map(|e| demote_for_reentry(e, &RootWitness::Owned)); + let extract: Option<&Expr> = extract.as_deref(); // Lazily computed on the first fork, then reused for every later one -- // `#2440`'s zero-INIT-outputs short-circuit is no longer a separate // early return this could sit above (see this function's own doc @@ -43817,7 +43841,6 @@ pub(crate) fn foreach_forks( step_state, optional, &mut budget, - resolver_free, sink, ); state = new_state; @@ -43908,8 +43931,12 @@ pub(crate) fn reduce_forks( drive_source: ForeachSourceDrive<'_>, sink: &mut dyn FnMut(OwnedValue) -> Demand, ) -> Flow { - // #3036: decided once per fold, see `fold_update_is_resolver_free`. - let resolver_free = fold_update_is_resolver_free(update); + // #3036/#3122: UPDATE reruns against the fold's own accumulator, never a + // document node, so its `Snapshot` markers are demoted against `Owned` + // -- once per fold, here, rather than once per element (see + // `fold_step_each`); the fold's own callers pass it in as written. + let update = demote_for_reentry(update, &RootWitness::Owned); + let update: &Expr = &update; // Lazily computed on the first fork and reused, for the same reason // `foreach_forks` defers it: a zero-output INIT (`reduce halt_error as // $x (empty; .)`, which exits 0) must not pay for a `Vec` build it never @@ -43942,7 +43969,6 @@ pub(crate) fn reduce_forks( step_acc, optional, &mut budget, - resolver_free, ); acc = new_acc; match step_control { @@ -44457,8 +44483,8 @@ fn eval_until<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Ok(v) => v, Err(e) => return suppress_or_raise(e, optional), }; - let demoted_cond = demote_for_owned_reentry(cond); - let demoted_update = demote_for_owned_reentry(update); + let demoted_cond = demote_for_reentry(cond, &RootWitness::Owned); + let demoted_update = demote_for_reentry(update, &RootWitness::Owned); let cond = LoopOperand::new(cond, &demoted_cond); let update = LoopOperand::new(update, &demoted_update); let mut outputs: Vec = Vec::new(); @@ -44476,7 +44502,7 @@ fn eval_until<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } /// A static `until`/`while` operand (`cond`/`update`) alongside its -/// [`demote_for_owned_reentry`] twin, computed once by `eval_until`/ +/// [`demote_for_reentry`] (against `Owned`) twin, computed once by `eval_until`/ /// `eval_while` (#3036 review) instead of re-walked every step -- bundled /// so threading both through `until_step`/`while_step`'s recursion doesn't /// push either past `clippy::too_many_arguments`. @@ -44491,14 +44517,20 @@ impl<'e> LoopOperand<'e> { Self { expr, demoted } } - /// [`eval_owned_expr_fork_from`] over this operand. + /// [`eval_owned_expr_fork`] over this operand: `expr` itself while + /// `ambient` (the loop's first round, on the caller's own input, which + /// the enclosing re-entry already demoted for) and the pre-demoted twin + /// on every later round, whose state is a value `update` computed. Both + /// are therefore [`Reentry::Proven`] here -- the demotion happened once, + /// in `eval_until`/`eval_while`, not once per step. fn fork( &self, input: &OwnedValue, optional: bool, ambient: bool, ) -> (Vec, Option) { - eval_owned_expr_fork_from::(self.expr, self.demoted, input, optional, ambient) + let expr = if ambient { self.expr } else { self.demoted }; + eval_owned_expr_fork::(expr, input, optional, Reentry::Proven) } } @@ -44609,8 +44641,8 @@ fn eval_while<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Ok(v) => v, Err(e) => return suppress_or_raise(e, optional), }; - let demoted_cond = demote_for_owned_reentry(cond); - let demoted_update = demote_for_owned_reentry(update); + let demoted_cond = demote_for_reentry(cond, &RootWitness::Owned); + let demoted_update = demote_for_reentry(update, &RootWitness::Owned); let cond = LoopOperand::new(cond, &demoted_cond); let update = LoopOperand::new(update, &demoted_update); let mut outputs: Vec = Vec::new(); @@ -44775,7 +44807,7 @@ fn each_repeat<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( let mut empty_rounds = 0usize; loop { // #3036: `owned` is this arm's own input, unrebuilt -- bridged. - let (vals, control) = eval_owned_expr_fork_bridged::(expr, &owned, optional); + let (vals, control) = eval_owned_expr_fork::(expr, &owned, optional, Reentry::Proven); if vals.is_empty() && control.is_none() { empty_rounds += 1; if empty_rounds >= MAX_EMPTY_REPEAT_ROUNDS { @@ -44851,7 +44883,7 @@ fn eval_repeat<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // .+100)))]` on `0` is `[1,100,1,100,1,100]`, not // `[[1,100],[1,100],[1,100]]`. // #3036: `owned` is this arm's own input, unrebuilt -- bridged. - let (vals, control) = eval_owned_expr_fork_bridged::(expr, &owned, optional); + let (vals, control) = eval_owned_expr_fork::(expr, &owned, optional, Reentry::Proven); for val in vals { if let Some(control) = charge_budget(&mut budget, "repeat") { return finish_fork(outputs, Some(control), optional); @@ -45317,7 +45349,7 @@ fn builtin_recurse_f<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // never stops, so the walk is exactly what it always was: every node // visited, `f` run at each, up to `RECURSE_MAX_ITEMS`. let mut outputs: Vec = Vec::new(); - let end = each_recurse_walk::(f, None, root, &mut |v| { + let end = each_recurse_walk::(f, None, root, Reentry::Proven, &mut |v| { outputs.push(v); Demand::Continue }); @@ -45374,7 +45406,7 @@ fn builtin_recurse_cond<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // supplied; an always-`Continue` sink keeps this the collecting walk it // has always been. let mut outputs: Vec = Vec::new(); - let end = each_recurse_walk::(f, Some(cond), root, &mut |v| { + let end = each_recurse_walk::(f, Some(cond), root, Reentry::Proven, &mut |v| { outputs.push(v); Demand::Continue }); @@ -45491,7 +45523,7 @@ fn walk_impl_at_depth( // Then apply f to the processed value, correctly threading Control // (#855) at every level -- the same fork building block every other // fork construct in this file uses. - eval_owned_expr_fork::(f, &processed, optional) + eval_owned_expr_fork::(f, &processed, optional, Reentry::REBUILT) } /// Builtin: isvalid(expr) - check if expr succeeds without errors. @@ -45631,7 +45663,7 @@ fn builtin_isvalid<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( /// has no cursor to give it, only a `StandardJson` that may be a sub-value /// of a larger document. Serializing `owned` into a throwaway document and /// taking *its* root cursor is that position expressed as a node: the same -/// round trip `eval_owned_input_reindexed` and `eval_generic::eval_on_owned` +/// round trip `eval_owned_input_bridge` and `eval_generic::eval_on_owned` /// already make. Handing the pipe over with no cursor instead was tried and /// is wrong -- `key` then falls through `eval_builtin`'s cursor-guarded arm /// to the `null` stub, and `.x = [.a[] | key]` prints `[null,null]` rather @@ -45676,7 +45708,7 @@ fn eval_path_context_pipe_owned<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( .then(|| { exprs .iter() - .map(|e| demote_for_owned_reentry(e).into_owned()) + .map(|e| demote_for_reentry(e, &RootWitness::Owned).into_owned()) .collect() }); let exprs: &[Expr] = demoted.as_deref().unwrap_or(exprs); @@ -45688,7 +45720,7 @@ fn eval_path_context_pipe_owned<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } } - // Same throwaway document `eval_owned_input_reindexed` builds, and the + // Same throwaway document `eval_owned_input_bridge` builds, and the // same `to_json_for_reindex` (not `to_json`) for the same reason (#561). let json_str = owned.to_json_for_reindex::(); let json_bytes = json_str.as_bytes(); @@ -46651,13 +46683,14 @@ fn each_paths_filter<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( let Some(val_at_path) = get_value_at_path(&owned, path_arr) else { continue; }; - let flow = eval_each_owned::(filter, &val_at_path, optional, &mut |v| { - if v.is_truthy() { - sink(Item::Owned(path.clone())) - } else { - Demand::Continue - } - }); + let flow = + eval_each_owned::(filter, &val_at_path, optional, Reentry::REBUILT, &mut |v| { + if v.is_truthy() { + sink(Item::Owned(path.clone())) + } else { + Demand::Continue + } + }); match flow { Flow::Exhausted => {} stopped_or_escaped => return stopped_or_escaped, @@ -54928,7 +54961,7 @@ fn builtin_debug_msg<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // raise-on-decode-failure rule and the `to_owned_or_suppress!` swap. let owned = to_owned_or_suppress!(&value, optional); // #3036: `owned` is this arm's own input, unrebuilt -- bridged. - let flow = eval_each_owned_bridged::(msg, &owned, false, &mut |msg_value| { + let flow = eval_each_owned::(msg, &owned, false, Reentry::Proven, &mut |msg_value| { write_debug_line::(&msg_value); Demand::Continue }); @@ -56429,7 +56462,7 @@ trait PatternMode { /// while the container's first step is still from the register's own /// node. A key evaluated on a tracked node must not have its own /// markers demoted just because it resolves by value -- see - /// [`eval_each_owned_at`]. + /// [`Reentry::at_register`]. fn key_input_tracked(&self, reg: &Self::Reg, first: bool) -> bool; /// The name a binding carries, for the duplicate-name rule. @@ -56666,15 +56699,21 @@ fn walk_object_entries( ObjectKey::Expr(key_expr) => { let mut ended: Option = None; let tracked = mode.key_input_tracked(®, first); - let key_flow = eval_each_owned_at::(key_expr, input, tracked, false, &mut |key| { - match per_key(PatternKey::Computed(&key), out) { - Flow::Exhausted => Demand::Continue, - // Classified on the way out (`stop_with_downstream`), so - // a `?//` *inside the key expression* never retries past - // a halt or an uncatchable error the walk below raised. - other => stop_with_downstream(&mut ended, other), - } - }); + let key_flow = eval_each_owned::( + key_expr, + input, + false, + Reentry::at_register(tracked), + &mut |key| { + match per_key(PatternKey::Computed(&key), out) { + Flow::Exhausted => Demand::Continue, + // Classified on the way out (`stop_with_downstream`), so + // a `?//` *inside the key expression* never retries past + // a halt or an uncatchable error the walk below raised. + other => stop_with_downstream(&mut ended, other), + } + }, + ); match ended { Some(flow) => flow, // The key generator's own verdict: exhausted, or its own @@ -66142,7 +66181,9 @@ mod tests { // behavior: jq's `_modify` only ever observes the update filter's // first output, so a trailing error must not surface here. let expr = parse(r#"1, error("boom")"#).unwrap(); - let values = eval_owned_multi_first::(&expr, &OwnedValue::Null).unwrap(); + let values = + eval_owned_multi_first::(&expr, &OwnedValue::Null, Reentry::REBUILT) + .unwrap(); assert_eq!(values, vec![OwnedValue::Int(1)]); } @@ -66158,7 +66199,8 @@ mod tests { // below, which `eval_owned_multi_first` deliberately keeps dropping // the trailing control from (jq's `_modify` never observes it). let expr = parse("break $foo").unwrap(); - let err = eval_owned_multi_first::(&expr, &OwnedValue::Null).unwrap_err(); + let err = eval_owned_multi_first::(&expr, &OwnedValue::Null, Reentry::REBUILT) + .unwrap_err(); assert_eq!(err, EvalEscape::Break("foo".to_string())); } @@ -66172,7 +66214,9 @@ mod tests { // confirmed live, `.a |= (2, break $out)` is `{"a":2}` in real jq, // not an error and not an unwind to `$out`. let expr = parse("1, break $foo").unwrap(); - let values = eval_owned_multi_first::(&expr, &OwnedValue::Null).unwrap(); + let values = + eval_owned_multi_first::(&expr, &OwnedValue::Null, Reentry::REBUILT) + .unwrap(); assert_eq!(values, vec![OwnedValue::Int(1)]); } @@ -66509,7 +66553,7 @@ mod tests { /// This pins the two together for the new arms: run every expression in /// [`pure_expr_matrix`] against every value in [`pure_value_matrix`] /// through both `eval_owned_input` (now fast-pathed) and - /// [`eval_owned_input_reindexed`] (the bridge, forced), and assert the + /// [`eval_owned_input_bridge`] (the bridge, forced), and assert the /// values *and* the terminating control/error text agree exactly. Both /// semantics are covered: `S` reaches these arms through `apply_compare_op` /// (`STRICT_NUMERIC_EQUALITY`) and through the bridge's own float @@ -66538,9 +66582,12 @@ mod tests { ); for value in &values { let fast = debug_normalize(eval_owned_input::, JqSemantics>( - &expr, value, false, + &expr, + value, + false, + Reentry::REBUILT, )); - let bridge = debug_normalize(eval_owned_input_reindexed::, JqSemantics>( + let bridge = debug_normalize(eval_owned_input_bridge::, JqSemantics>( &expr, value, false, )); assert_eq!( @@ -66549,9 +66596,12 @@ mod tests { ); let fast = debug_normalize(eval_owned_input::, YqSemantics>( - &expr, value, false, + &expr, + value, + false, + Reentry::REBUILT, )); - let bridge = debug_normalize(eval_owned_input_reindexed::, YqSemantics>( + let bridge = debug_normalize(eval_owned_input_bridge::, YqSemantics>( &expr, value, false, )); assert_eq!( @@ -66618,9 +66668,12 @@ mod tests { ); for value in &values { let fast = debug_normalize(eval_owned_input::, JqSemantics>( - &expr, value, false, + &expr, + value, + false, + Reentry::REBUILT, )); - let bridge = debug_normalize(eval_owned_input_reindexed::< + let bridge = debug_normalize(eval_owned_input_bridge::< Vec, JqSemantics, >(&expr, value, false)); @@ -66629,9 +66682,12 @@ mod tests { "jq mode: {src:?} with $y := {bound:?} on {value:?} disagrees with the bridge" ); let fast = debug_normalize(eval_owned_input::, YqSemantics>( - &expr, value, false, + &expr, + value, + false, + Reentry::REBUILT, )); - let bridge = debug_normalize(eval_owned_input_reindexed::< + let bridge = debug_normalize(eval_owned_input_bridge::< Vec, YqSemantics, >(&expr, value, false)); @@ -66969,7 +67025,7 @@ mod tests { let direct = eval_owned_pure::(&expr, &value, ResultPosition::Operand) .unwrap() .unwrap(); - let bridge = debug_normalize(eval_owned_input_reindexed::, JqSemantics>( + let bridge = debug_normalize(eval_owned_input_bridge::, JqSemantics>( &expr, &value, false, )); assert_eq!(format!("{direct:?}"), "Int(2)"); @@ -66977,7 +67033,10 @@ mod tests { // The gate holds: what actually runs is the bridge's answer. assert_eq!( debug_normalize(eval_owned_input::, JqSemantics>( - &expr, &value, false + &expr, + &value, + false, + Reentry::REBUILT )), bridge ); @@ -67062,7 +67121,7 @@ mod tests { ); // The fast path must agree with the reindex bridge here too, not // just on the value it returns when both succeed. - match eval_owned_input_reindexed::, JqSemantics>(&cond, &value, false) { + match eval_owned_input_bridge::, JqSemantics>(&cond, &value, false) { QueryResult::Error(bridge_err) => assert_eq!(err.message, bridge_err.message), other => panic!("expected the bridge to also raise, got: {other:?}"), } @@ -78583,7 +78642,8 @@ mod tests { #[test] fn eval_owned_expr_fork_empty_update_yields_no_outputs_directly() { let expr = parse("empty").unwrap(); - let (vals, control) = eval_owned_expr_fork::(&expr, &OwnedValue::Null, false); + let (vals, control) = + eval_owned_expr_fork::(&expr, &OwnedValue::Null, false, Reentry::REBUILT); assert_eq!(vals, Vec::::new()); assert!(control.is_none()); } @@ -79166,11 +79226,12 @@ mod tests { right: Box::new(Expr::Literal(Literal::String("a".to_string()))), }; let mut on_update_calls = 0; - let flow = - fold_step_each::(&expr, OwnedValue::Int(1), true, false, &mut |_| { - on_update_calls += 1; - Demand::Continue - }); + // omni-dev: coverage tolerate reason="the closure is asserted never called below (on_update_calls stays 0) -- Err(_) with optional=true short-circuits fold_step_each before this sink runs (#3122)" + let flow = fold_step_each::(&expr, OwnedValue::Int(1), true, &mut |_| { + on_update_calls += 1; + Demand::Continue + }); + // omni-dev: coverage end assert_eq!(on_update_calls, 0); assert!(matches!(flow, Flow::Exhausted)); } @@ -88537,6 +88598,29 @@ mod tests { ); } + /// #3122: [`eval_path_context_pipe_owned`]'s own marker-demotion + /// precheck. `needs_path_context` deliberately does not recurse into a + /// `?//`-alternative chain's body (`patterns.len() > 1`, mirroring its + /// identical carve-out for `reduce`/`foreach`'s UPDATE, both documented + /// on that function), so a `key` sitting inside one reaches this + /// evaluator's own `eval_pipe` bridge with the routing decision already + /// made against `false` at the top level -- and a `Snapshot` marker + /// bound before the chain (`. as $m`, still a live document node here + /// since the eager evaluator's `eval_as` never re-entered a resolver in + /// between) reaches the bridge un-demoted, exactly the shape this + /// door's own `has_demotable_marker` precheck exists to catch before + /// the pipe crosses into a rebuilt document. + #[test] + fn test_hidden_pipe_inside_alternative_chain_demotes_a_live_marker_3122() { + assert_eq!( + outputs( + b"{\"a\":{\"b\":1}}", + ". as $m | (1 as $q ?// $q | .a | [key, $m])" + ), + vec![r#"["a",{"a":{"b":1}}]"#] + ); + } + /// Spine 2416 (the exit), door 2: a value the reindex bridge would /// re-spell must not take it. /// @@ -88559,7 +88643,7 @@ mod tests { /// /// One shape the detached route does *not* keep, and the reason it is /// not a row: a stage it hands to the ordinary owned evaluator - /// (`.[] | select(key == 1)`) goes through `eval_owned_input_reindexed`, + /// (`.[] | select(key == 1)`) goes through `eval_owned_input_bridge`, /// which applies the same formatter, so the 300-digit literal comes back /// as `1E+299` there. That is the owned evaluator's round trip, not this /// door's, and it is reachable only for a value class no document read @@ -88633,6 +88717,22 @@ mod tests { ); } + /// #3122: `eval_owned_with_file_index`'s own fast path -- a filter that + /// `needs_path_context` can see nothing in (no `file_index`/`key`/ + /// `parent`/`path`) skips the reindex-and-walk door entirely and goes + /// straight through the ordinary owned evaluator via `Reentry::REBUILT`, + /// the overwhelming-majority case the function's own doc comment + /// describes. Every other test in this module that reaches + /// `eval_owned_with_file_index` deliberately uses a path-context + /// filter to exercise the door itself, so none of them cover this arm. + #[test] + fn test_eval_all_fast_path_skips_path_context_door_3122() { + assert_eq!( + eval_all_outputs(b"[10,20]", &[7, 8], ".[] | . + 1"), + vec!["11", "21"] + ); + } + #[test] fn test_generic_builtin_continues_pipe_with_path_context() { // `tostring` (any builtin without its own dedicated path-context @@ -97897,6 +97997,51 @@ mod tests { } } + /// `rewrite_markers`' own precheck (#3122 coverage review): its + /// `any_subexpr` gate decides once, for the *whole* expression, whether + /// to walk at all -- so a marker that itself needs no change (a + /// `Snapshot` already naming `root`'s own node, neither demotable nor + /// promotable) is still visited and cloned back untouched whenever a + /// *sibling* marker in the same expression is the one that tripped the + /// gate. `reroot_markers_only_lifts_the_roots_own_node_3037` above pairs + /// a demotion with a promotion -- both change -- so it never exercises + /// this "walked but left alone" arm; this test pairs a demotion with a + /// marker that already matches `root` exactly. + #[test] + fn rewrite_markers_leaves_an_already_matching_sibling_untouched_3122() { + let value = OwnedValue::object_from([("b".to_string(), OwnedValue::Int(1))]); + let node = |node: usize, document: usize| BindOrigin::Node { node, document }; + let marker = |node: Option| { + Expr::TrackedVar(Rc::new(Tracked { + value: value.clone(), + origin: Origin::Snapshot, + node, + })) + }; + let origin_of = |e: &Expr| match e { + Expr::TrackedVar(m) => m.origin.clone(), + other => panic!("expected a marker, got {other:?}"), // omni-dev: coverage tolerate-line reason="unreachable in a passing suite by design -- every expression this closure receives is built by `marker` above (#3122)" + }; + let root = RootWitness::Node { + node: 3, + document: 9, + }; + // First marker names another node: demoted. Second already names + // `root` exactly: `marker_needs_demotion` refuses (its node matches) + // and `marker_is_root` refuses too (its origin is `Snapshot`, not + // `Untracked`), so `reroot_rewrite` answers `None` for it -- the + // walk still reaches it (the first marker already proved a rewrite + // is needed somewhere) and clones it back with the same origin. + let mixed = Expr::Comma(vec![marker(Some(node(4, 9))), marker(Some(node(3, 9)))]); + match reroot_markers::(&mixed, &root).as_ref() { + Expr::Comma(parts) => { + assert_eq!(origin_of(&parts[0]), Origin::Untracked); + assert_eq!(origin_of(&parts[1]), Origin::Snapshot); + } + other => panic!("expected the comma back, got {other:?}"), // omni-dev: coverage tolerate-line reason="unreachable in a passing suite by design -- `rewrite_markers` rebuilds the same node kind it was given (#3122)" + } + } + /// yq mode is untouched (#3037): real yq's assignment through a /// variable is a no-op that prints the document unchanged, where /// succinctly refuses loudly -- a pre-existing divergence in the safe @@ -97919,6 +98064,116 @@ mod tests { ); } + /// #3122: `reroot_for_reentry` is the one derivation every owned + /// re-entry runs, and its precheck is what keeps the rebuild off the + /// common path: against *any* witness, an expression that cannot start + /// a resolver invocation is handed back borrowed however many demotable + /// markers it holds, and one that can is rebuilt exactly when a marker + /// cannot name the root. `Reentry` is the only way in: `Proven` never + /// touches the expression, `REBUILT` is `Against(Owned)`. + #[test] + fn demote_for_reentry_rebuilds_only_a_resolver_reaching_marker_3122() { + let value = OwnedValue::object_from([("a".to_string(), OwnedValue::Int(1))]); + let marker = Expr::TrackedVar(Rc::new(Tracked { + value, + origin: Origin::Snapshot, + node: Some(BindOrigin::Node { + node: 3, + document: 7, + }), + })); + let own = RootWitness::Node { + node: 3, + document: 7, + }; + let other = RootWitness::Node { + node: 4, + document: 7, + }; + // Read as a plain value: never rebuilt, whatever the root. + let plain = Expr::Pipe(vec![marker.clone(), Expr::Identity]); + for root in [&own, &other, &RootWitness::Owned] { + assert!(matches!(demote_for_reentry(&plain, root), Cow::Borrowed(_))); + } + // Reaching a resolver: rebuilt exactly when the marker cannot name + // the root. + let resolving = Expr::Builtin(Builtin::Path(Box::new(marker.clone()))); + assert!(matches!( + demote_for_reentry(&resolving, &own), + Cow::Borrowed(_) + )); + for root in [&other, &RootWitness::Owned] { + let Cow::Owned(Expr::Builtin(Builtin::Path(inner))) = + demote_for_reentry(&resolving, root) + else { + // omni-dev: coverage tolerate-line reason="unreachable in a passing suite by design -- fires only if demote_for_reentry's own let-else assertion condition is false (#3122)" + panic!("a mismatching root must rebuild the resolver-reaching expression"); + }; + let Expr::TrackedVar(demoted) = inner.as_ref() else { + panic!("the rebuilt expression keeps its shape"); // omni-dev: coverage tolerate-line reason="unreachable in a passing suite by design -- fires only if demote_for_reentry's own let-else assertion condition is false (#3122)" + }; + assert_eq!(demoted.origin, Origin::Untracked); + } + assert!(matches!( + Reentry::Proven.reroot::(&resolving), + Cow::Borrowed(_) + )); + assert!(matches!( + Reentry::REBUILT.reroot::(&resolving), + Cow::Owned(_) + )); + assert!(matches!( + Reentry::Against(own).reroot::(&resolving), + Cow::Borrowed(_) + )); + assert_eq!(Reentry::at_register(true), Reentry::Proven); + assert_eq!(Reentry::at_register(false), Reentry::REBUILT); + + // #3037's promotion rides the same precheck: an `Untracked` marker + // whose node *is* the root is lifted to a `Snapshot` only where a + // resolver could read it, only in jq mode, and never through + // `Proven`, which trusts the caller. + let untracked = Expr::TrackedVar(Rc::new(Tracked { + value: OwnedValue::object_from([("a".to_string(), OwnedValue::Int(1))]), + origin: Origin::Untracked, + node: Some(BindOrigin::Node { + node: 3, + document: 7, + }), + })); + let plain = Expr::Pipe(vec![untracked.clone(), Expr::Identity]); + assert!(matches!( + Reentry::Against(own).reroot::(&plain), + Cow::Borrowed(_) + )); + let resolving = Expr::Builtin(Builtin::Path(Box::new(untracked))); + let Cow::Owned(Expr::Builtin(Builtin::Path(inner))) = + Reentry::Against(own).reroot::(&resolving) + else { + // omni-dev: coverage tolerate-line reason="unreachable in a passing suite by design -- fires only if reroot's own let-else assertion condition is false (#3122)" + panic!("the root's own untracked marker must be promoted where a resolver reads it"); + }; + let Expr::TrackedVar(promoted) = inner.as_ref() else { + // omni-dev: coverage tolerate-line reason="unreachable in a passing suite by design -- fires only if reroot's own let-else assertion condition is false (#3122)" + panic!("the rebuilt expression keeps its shape"); + }; + assert_eq!(promoted.origin, Origin::Snapshot); + for (label, reentry) in [ + ("another node", Reentry::Against(other)), + ("no node", Reentry::REBUILT), + ("proven", Reentry::Proven), + ] { + assert!( + matches!(reentry.reroot::(&resolving), Cow::Borrowed(_)), + "{label}" + ); + } + assert!(matches!( + Reentry::Against(own).reroot::(&resolving), + Cow::Borrowed(_) + )); + } + /// #3036: `try BODY catch HANDLER`'s handler is checked against the /// payload's own node, which only `error($x)`/`$x | error` on a marker /// that knows its node can name -- every other body, including a diff --git a/src/jq/eval_generic.rs b/src/jq/eval_generic.rs index 188e02a30..23f29f5ca 100644 --- a/src/jq/eval_generic.rs +++ b/src/jq/eval_generic.rs @@ -56,28 +56,28 @@ use super::eval::{ boolean_fanout_bools, boolean_fanout_each, cache_shared_chain_value, cached_shared_chain_value, cannot_reserve_cross_product, classify_limit_n, classify_nth_n, classify_parent_n, classify_skip_n, clear_nonretryable_stop, collapse_vec, collect_pattern_var_names, - compare_key_arrays, compare_values, debug_assert_materialization_error, demote_rebuilt_markers, + compare_key_arrays, compare_values, debug_assert_materialization_error, demote_for_reentry, each_path_on_owned, each_pattern_binding_set, each_recurse_walk, enter_def_call_frame, - entries_to_object, eval_each_owned, eval_each_owned_bridged, eval_full as full_eval, - finish_fork_flow, finish_fork_from_flow, finish_short_circuit, fold_escaped_generator_prefix, - foreach_forks, format_owned, has_type_mismatch_is_permissive, index_component_value, - index_in_array_bounds, index_one_owned as index_owned_by_key, is_dollar_safe_chain_key, - is_pure_chain_link, is_retryable_control, is_retryable_stop, key_arrays_eq, literal_to_owned, + entries_to_object, eval_each_owned, eval_full as full_eval, finish_fork_flow, + finish_fork_from_flow, finish_short_circuit, fold_escaped_generator_prefix, foreach_forks, + format_owned, has_type_mismatch_is_permissive, index_component_value, index_in_array_bounds, + index_one_owned as index_owned_by_key, is_dollar_safe_chain_key, is_pure_chain_link, + is_retryable_control, is_retryable_stop, key_arrays_eq, literal_to_owned, mark_nonretryable_escape, needs_path_context, numeric_key_to_array_index, numeric_key_to_index, numeric_length_owned, owned_bound_to_i64, owned_to_expr, owned_to_string, pattern_alternatives_var_names, prefer_pending_control, range_max_exceeded_error, range_num, - range_values_f64, range_values_int, recurse_walk_flow, reduce_forks, reroot_markers, - resolve_computed_slice_bounds, resume_from_escape, reverse_length_is_empty, select_emits, - slice_component_value, slice_object_as_yq_children, slice_owned_value_read_computed, - stop_with_downstream, stop_with_error, stop_with_escape, stop_with_escape_cell, - streams_escaped_generator_prefix, streams_unbounded, substitute_bound_var_from, - substitute_vars, suppresses, tonumber_from_str, try_payload_root, vec_with_capacity, - yq_absent_key_read_is_empty, yq_assign_rhs_document, yq_empty_operand_output, - yq_field_index_on_scalar_is_empty, yq_negative_index_check, yq_numeric_index_on_object_is_null, - yq_object_key_stringify, yq_read_only_context, yq_scalar_text, BinaryFanoutRules, - ComputedSliceBound, Control, Demand, EmptyOperandOp, EvalError, EvalSemantics, EvalTag, Flow, - JqSemantics, LimitN, PathTrail, QueryResult, RangeNum, RootWitness, SliceTargetKind, - YqSemantics, WHILE_UNTIL_MAX_STEPS, + range_values_f64, range_values_int, recurse_walk_flow, reduce_forks, reroot_for_reentry, + reroot_markers, resolve_computed_slice_bounds, resume_from_escape, reverse_length_is_empty, + select_emits, slice_component_value, slice_object_as_yq_children, + slice_owned_value_read_computed, stop_with_downstream, stop_with_error, stop_with_escape, + stop_with_escape_cell, streams_escaped_generator_prefix, streams_unbounded, + substitute_bound_var_from, substitute_vars, suppresses, tonumber_from_str, try_payload_root, + vec_with_capacity, yq_absent_key_read_is_empty, yq_assign_rhs_document, + yq_empty_operand_output, yq_field_index_on_scalar_is_empty, yq_negative_index_check, + yq_numeric_index_on_object_is_null, yq_object_key_stringify, yq_read_only_context, + yq_scalar_text, BinaryFanoutRules, ComputedSliceBound, Control, Demand, EmptyOperandOp, + EvalError, EvalSemantics, EvalTag, Flow, JqSemantics, LimitN, PathTrail, QueryResult, RangeNum, + Reentry, RootWitness, SliceTargetKind, YqSemantics, WHILE_UNTIL_MAX_STEPS, }; #[cfg(test)] use super::expr::FuncDefBound; @@ -2376,12 +2376,10 @@ impl LazySeq { // `RootWitness::Owned` demotes any `Snapshot` marker `instr.f` // carries, since it cannot be proven to be the same node as `o`. (LazyElem::Owned(o), EvalTag::Jq) => { - let f = demote_rebuilt_markers(&instr.f, &RootWitness::Owned); - eval_on_owned::(&f, o, false) + eval_on_owned::(&instr.f, o, false, Reentry::REBUILT) } (LazyElem::Owned(o), EvalTag::Yq) => { - let f = demote_rebuilt_markers(&instr.f, &RootWitness::Owned); - eval_on_owned::(&f, o, false) + eval_on_owned::(&instr.f, o, false, Reentry::REBUILT) } } } @@ -2715,10 +2713,15 @@ fn format_result( /// /// This converts the OwnedValue to JSON, evaluates using the full evaluator, /// and converts the result back to GenericResult. +/// +/// `reentry` says whether the `Snapshot` markers in `expr` are already +/// demoted for `owned`'s document or must be demoted against a named root +/// first (#2642/#3122) -- see `eval::eval_each_owned` for the rule. fn eval_on_owned( expr: &Expr, owned: OwnedValue, optional: bool, + reentry: Reentry, ) -> GenericResult { // Formats need neither an index nor a cursor, so the round-trip below is // pure overhead for them (#124). No non-finite-float guard is needed here @@ -2749,6 +2752,9 @@ fn eval_on_owned( return GenericResult::Owned(OwnedValue::String(owned_to_string::(&owned))); } + // After the bypasses on purpose: neither reaches a resolver, and + // demoting rebuilds `expr` whenever it holds a marker at all. + let expr = reentry.reroot::(expr); let json_str = owned.to_json_for_reindex::(); let json_bytes = json_str.as_bytes(); let index = JsonIndex::build(json_bytes); @@ -2779,7 +2785,7 @@ fn eval_on_owned( // arms rather than `.unwrap()`/`.expect()` because the *type* (`Result`) // is what lets a real failure, if this invariant is ever violated by a // future change, surface as a normal `EvalError` instead of a panic. - query_result_to_generic::(full_eval::, S>(expr, cursor)) + query_result_to_generic::(full_eval::, S>(&expr, cursor)) } /// Materialize `value` (with its `cursor`, if any) and hand `expr` to the @@ -2830,12 +2836,8 @@ fn bridge_to_full_evaluator( // this call's own root first (a no-op, `Cow::Borrowed`, unless `expr` // actually contains one). let root = RootWitness::of(cursor.as_ref()); - // #3037: `reroot_markers`, not `demote_rebuilt_markers` -- this root is a - // live cursor, so an `Untracked` marker bound from this very node is - // promoted to a `Snapshot` for the call (jq mode; see its doc comment). - let expr = reroot_markers::(expr, &root); - match bridge_ambient_input::<_, S>(&expr, &value, cursor) { - Ok(owned) => eval_on_owned::(&expr, owned, optional), + match bridge_ambient_input::<_, S>(expr, &value, cursor) { + Ok(owned) => eval_on_owned::(expr, owned, optional, Reentry::Against(root)), Err(e) if suppresses(&e, optional) => GenericResult::None, Err(e) => GenericResult::Error(e), } @@ -2862,9 +2864,11 @@ fn bridge_to_full_evaluator_flow( // #2642: same rebuilt-root demotion as `bridge_to_full_evaluator`'s own // sibling fix -- see its comment. let root = RootWitness::of(cursor.as_ref()); - let expr = reroot_markers::(expr, &root); - match bridge_ambient_input::<_, S>(&expr, &value, cursor) { - Ok(owned) => drain_result_generic(eval_on_owned::(&expr, owned, optional), sink), + match bridge_ambient_input::<_, S>(expr, &value, cursor) { + Ok(owned) => drain_result_generic( + eval_on_owned::(expr, owned, optional, Reentry::Against(root)), + sink, + ), Err(e) if suppresses(&e, optional) => Flow::Exhausted, Err(e) => Flow::Escaped(Control::Error(e)), } @@ -2907,11 +2911,12 @@ fn bridge_to_each_owned_flow( // #2642: same rebuilt-root demotion as `bridge_to_full_evaluator`'s own // fix -- see its comment. let root = RootWitness::of(cursor.as_ref()); - let expr = reroot_markers::(expr, &root); - match bridge_ambient_input::<_, S>(&expr, &value, cursor) { - Ok(owned) => eval_each_owned_bridged::(&expr, &owned, optional, &mut |v| { - sink.push(GenericItem::Owned(v)) - }), + match bridge_ambient_input::<_, S>(expr, &value, cursor) { + Ok(owned) => { + eval_each_owned::(expr, &owned, optional, Reentry::Against(root), &mut |v| { + sink.push(GenericItem::Owned(v)) + }) + } Err(e) if suppresses(&e, optional) => Flow::Exhausted, Err(e) => Flow::Escaped(Control::Error(e)), } @@ -3144,9 +3149,10 @@ fn eval_each_owned_collect( expr: &Expr, input: &OwnedValue, optional: bool, + reentry: Reentry, ) -> GenericResult { let mut collected: Vec = Vec::new(); - let flow = eval_each_owned_bridged::(expr, input, optional, &mut |v| { + let flow = eval_each_owned::(expr, input, optional, reentry, &mut |v| { collected.push(v); Demand::Continue }); @@ -3908,11 +3914,11 @@ fn eval_on_many_owned( ) -> GenericResult { // #2642: `owned_values` come from a prior generator/`Partial` // accumulation, never a document node -- `RootWitness::Owned` demotes - // any `Snapshot` marker `expr` carries. - let expr = demote_rebuilt_markers(expr, &RootWitness::Owned); + // any `Snapshot` marker `expr` carries. Once, ahead of the loop (#3122). + let expr = demote_for_reentry(expr, &RootWitness::Owned); let mut results = Vec::new(); for owned in owned_values { - match eval_on_owned::(&expr, owned, optional) { + match eval_on_owned::(&expr, owned, optional, Reentry::Proven) { GenericResult::One(_) => unreachable!("eval_on_owned never returns One"), GenericResult::OneCursor(_) => unreachable!("eval_on_owned never returns OneCursor"), GenericResult::Many(_) => unreachable!("eval_on_owned never returns Many"), @@ -4859,8 +4865,7 @@ pub fn eval_using(expr: &Expr, value: V) -> let owned = owned_or_err!(to_owned_with_cursor::<_, S>(&value, None)); // #2642: the cursor is `None` here -- `Owned` demotes any `Snapshot` // marker `expr` carries. - let expr = demote_rebuilt_markers(expr, &RootWitness::Owned); - return eval_each_owned_collect::(&expr, &owned, false); + return eval_each_owned_collect::(expr, &owned, false, Reentry::REBUILT); } eval_single::(expr, value, false, None) } @@ -5268,8 +5273,7 @@ pub fn eval_with_cursor_using( // #2642: `cursor` is this call's own root -- demote any `Snapshot` // marker `expr` carries that isn't proven to be this document node. let root = RootWitness::of(Some(&cursor)); - let expr = reroot_markers::(expr, &root); - return eval_each_owned_collect::(&expr, &owned, false); + return eval_each_owned_collect::(expr, &owned, false, Reentry::Against(root)); } eval_single::(expr, cursor.value(), false, Some(cursor)) } @@ -5374,8 +5378,13 @@ pub fn eval_each_with_cursor_using( // #2642: `cursor` is this call's own root -- demote any `Snapshot` // marker `expr` carries that isn't proven to be this document node. let root = RootWitness::of(Some(&cursor)); - let expr = reroot_markers::(expr, &root); - return match eval_each_owned_bridged::(&expr, &owned, false, &mut owned_sink) { + return match eval_each_owned::( + expr, + &owned, + false, + Reentry::Against(root), + &mut owned_sink, + ) { Flow::Exhausted => None, // Kept for the same reason as the streaming branch below. Flow::Stopped { pending } => pending, @@ -5687,8 +5696,7 @@ fn fold_pipe_stages( // Continue piping from owned value via JSON round-trip. // #2642: `o` is a prior stage's computed value, never // document-backed. - let expr = demote_rebuilt_markers(expr, &RootWitness::Owned); - eval_on_owned::(&expr, o, optional) + eval_on_owned::(expr, o, optional, Reentry::REBUILT) } GenericResult::ManyOwned(os) => { // Continue piping from owned values via JSON round-trip @@ -5927,10 +5935,7 @@ fn fold_lazy_keys_stage( // collapse rule itself, through `effective_keys`. _ => match materialize_lazy_keys::(&fields, sorted, collapse) { // #2642: the synthesized keys array is never document-backed. - Ok(owned) => { - let expr = demote_rebuilt_markers(expr, &RootWitness::Owned); - eval_on_owned::(&expr, owned, optional) - } + Ok(owned) => eval_on_owned::(expr, owned, optional, Reentry::REBUILT), Err(e) => GenericResult::Error(e), }, } @@ -6000,10 +6005,12 @@ fn fold_lazy_index_range_stage( LazySeq::new(LazySource::IndexRange { next: 0, len }).push_map(f, S::TAG), )), // #2642: the synthesized index-range array is never document-backed. - _ => { - let expr = demote_rebuilt_markers(expr, &RootWitness::Owned); - eval_on_owned::(&expr, materialize_lazy_index_range(len), optional) - } + _ => eval_on_owned::( + expr, + materialize_lazy_index_range(len), + optional, + Reentry::REBUILT, + ), } } @@ -6237,10 +6244,7 @@ fn fold_lazy_seq_stage( // #2642: the atomic-materialized lazy-seq accumulator is never // document-backed. _ => match seq.materialize_atomic::() { - Ok(owned) => { - let expr = demote_rebuilt_markers(expr, &RootWitness::Owned); - eval_on_owned::(&expr, owned, optional) - } + Ok(owned) => eval_on_owned::(expr, owned, optional, Reentry::REBUILT), Err(Control::Error(e)) => GenericResult::Error(e), Err(Control::Break(label)) => GenericResult::Break(label), Err(Control::Halt(code)) => GenericResult::Halt(code), @@ -6684,7 +6688,7 @@ fn try_owned_format_or_tostring_bypass( ) -> Result { match remaining { [stage @ (Expr::Format(_) | Expr::Builtin(Builtin::ToString))] => Ok(drain_result_generic( - eval_on_owned::(stage, o, optional), + eval_on_owned::(stage, o, optional, Reentry::REBUILT), sink, )), _ => Err(o), @@ -6769,11 +6773,13 @@ fn fold_pipe_stages_sink( // #2642: `o` is a computed intermediate value, never // document-backed. let rest_pipe = Expr::Pipe(stages[j..].to_vec()); - let rest_pipe = - demote_rebuilt_markers(&rest_pipe, &RootWitness::Owned).into_owned(); - return eval_each_owned::(&rest_pipe, &o, optional, &mut |o| { - sink.push(GenericItem::Owned(o)) - }); + return eval_each_owned::( + &rest_pipe, + &o, + optional, + Reentry::REBUILT, + &mut |o| sink.push(GenericItem::Owned(o)), + ); } // Nothing further to fold; push (or don't) and stop, same as // `drain_result_generic`'s own handling of these. @@ -6960,8 +6966,13 @@ fn try_single_generic( let run_catch = |payload: &OwnedValue| -> GenericResult { match catch { Some(catch_expr) => { - let catch_expr = reroot_markers::(catch_expr, &try_payload_root(inner)); - eval_each_owned_collect::(&catch_expr, payload, optional) + let root = try_payload_root(inner); + eval_each_owned_collect::( + catch_expr, + payload, + optional, + Reentry::Against(root), + ) } None => GenericResult::None, } @@ -7421,10 +7432,8 @@ fn eval_single( // #2642: `cursor` is `None` in this arm, so the root has // no document node -- `Owned` demotes any `Snapshot` // marker `exprs` carries. - let pipe = - demote_rebuilt_markers(&Expr::Pipe(exprs.clone()), &RootWitness::Owned) - .into_owned(); - return eval_on_owned::(&pipe, owned, optional); + let pipe = Expr::Pipe(exprs.clone()); + return eval_on_owned::(&pipe, owned, optional, Reentry::REBUILT); } } @@ -7848,17 +7857,16 @@ fn eval_single( // and the source is driven per INIT fork, which is what jq does // (`[reduce ("s"|stderr) as $x ((0,1); .)]` writes `ss`). // - // #2642: `update` reruns against the fold's own `OwnedValue` + // #2642/#3122: `update` reruns against the fold's own `OwnedValue` // accumulator (INIT's own value, then each step's own result) -- // never the ambient cursor above, regardless of what `cursor` - // itself points at, so this is always `Owned`, not - // `RootWitness::of(cursor)` (which would wrongly compare against - // `.`'s own node instead of the accumulator's). - let update = demote_rebuilt_markers(update, &RootWitness::Owned); + // itself points at, so `reduce_forks` demotes it against `Owned` + // itself, not `RootWitness::of(cursor)` (which would wrongly + // compare against `.`'s own node instead of the accumulator's). let mut outputs: Vec = Vec::new(); let flow = reduce_forks::( patterns, - &update, + update, &mut |per_init| { drive_foreach_expr_generic::(init, &value, optional, cursor, per_init) }, @@ -7907,17 +7915,14 @@ fn eval_single( // `stream_owned_outputs_generic`, the same demand-forwarding // treatment the source already gets just below. // - // #2642: `update`/`extract` rerun against the fold's own - // `OwnedValue` accumulator, never the ambient cursor -- always - // `Owned`, same reasoning as the `Expr::Reduce` arm just above. - let update = demote_rebuilt_markers(update, &RootWitness::Owned); - let extract = extract - .as_deref() - .map(|e| demote_rebuilt_markers(e, &RootWitness::Owned)); + // #2642/#3122: `update`/`extract` rerun against the fold's own + // `OwnedValue` accumulator, never the ambient cursor -- + // `foreach_forks` demotes them against `Owned` itself, same + // reasoning as the `Expr::Reduce` arm just above. let mut outputs: Vec = Vec::new(); let flow = foreach_forks::( patterns, - &update, + update, extract.as_deref(), &mut |per_init| { drive_foreach_expr_generic::(init, &value, optional, cursor, per_init) @@ -8181,7 +8186,7 @@ fn eval_single( // does not name it and, in jq mode, promoting an `Untracked` // one that *is* it (`.a as $y | .a | ($y.b) = 9`, #3037). let root = RootWitness::of(cursor.as_ref()); - let expr = reroot_markers::(expr, &root); + let expr = reroot_for_reentry::(expr, &root); let expr = expr.as_ref(); let owned = owned_or_err!(bridge_ambient_input::<_, S>(expr, &value, cursor)); let json_str = owned.to_json_for_reindex::(); @@ -8624,11 +8629,16 @@ fn each_recurse_generic( // // #1755: still the raising form, not a lossy read -- an undecodable // root must raise rather than be visited as `""`. + // #2642/#3122: level 0 is this node's own value -- `f` runs on it + // against this call's own root, as at every other funnel. + let reentry = Reentry::Against(RootWitness::of(cursor.as_ref())); let root = match to_owned_with_cursor::<_, S>(&value, cursor) { Ok(v) => v, Err(e) => return Flow::Escaped(Control::Error(e)), }; - let end = each_recurse_walk::(f, cond, root, &mut |v| sink.push(GenericItem::Owned(v))); + let end = each_recurse_walk::(f, cond, root, reentry, &mut |v| { + sink.push(GenericItem::Owned(v)) + }); recurse_walk_flow(end) } @@ -9144,6 +9154,9 @@ fn eval_each_generic( // #2642: demote any marker not proven to be `cursor`'s own node // before resolving, exactly as the eager arm does. let root = RootWitness::of(cursor.as_ref()); + // `reroot_markers`, not `reroot_for_reentry`: `path_expr` is the + // resolver's own argument, so the resolver-reaching node the + // precheck looks for is the `path` builtin *around* it (#3122). let demoted = reroot_markers::(path_expr, &root); // #2280: `optional` suppresses a decode failure into no output. let owned = match to_owned_with_cursor::<_, S>(&value, cursor) { @@ -9154,13 +9167,17 @@ fn eval_each_generic( if !reindex_bridge_is_identity(&owned) { // Hand the bridge the document already materialized above, // the way the eager arm does -- re-entering `eval_single` - // would run `RootWitness::of` + `demote_rebuilt_markers` + + // would run `RootWitness::of` + `demote_for_reentry` + // `to_owned_with_cursor` a second time over the whole // document to rebuild exactly these values. let owned_builtin_expr = Expr::Builtin(Builtin::Path(path_expr.clone())); - let builtin_expr = reroot_markers::(&owned_builtin_expr, &root); return drain_result_generic( - eval_on_owned::(&builtin_expr, owned, optional), + eval_on_owned::( + &owned_builtin_expr, + owned, + optional, + Reentry::Against(root), + ), sink, ); } @@ -9248,12 +9265,12 @@ fn each_reduce_generic( cursor: Option, sink: &mut dyn Sink, ) -> Flow { - // #2642: same reasoning as the eager `Expr::Reduce` arm -- `update` - // reruns against the fold's own accumulator, never the ambient cursor. - let update = demote_rebuilt_markers(update, &RootWitness::Owned); + // #2642/#3122: same reasoning as the eager `Expr::Reduce` arm -- `update` + // reruns against the fold's own accumulator, never the ambient cursor, + // and `reduce_forks` demotes it against `Owned` itself. reduce_forks::( patterns, - &update, + update, &mut |per_init| { drive_foreach_expr_generic::(init, &value, optional, cursor, per_init) }, @@ -9294,15 +9311,13 @@ fn each_foreach_generic( // INIT first, source second (#2440), same as the eager arm above it -- // #2668: both now driven through `eval_each_generic`. // - // #2642: same reasoning as the eager `Expr::Foreach` arm above -- + // #2642/#3122: same reasoning as the eager `Expr::Foreach` arm above -- // `update`/`extract` rerun against the fold's own accumulator, never the - // ambient cursor, so this is always `Owned`. - let update = demote_rebuilt_markers(update, &RootWitness::Owned); - let extract = extract.map(|e| demote_rebuilt_markers(e, &RootWitness::Owned)); + // ambient cursor, and `foreach_forks` demotes them against `Owned` itself. foreach_forks::( patterns, - &update, - extract.as_deref(), + update, + extract, &mut |per_init| { drive_foreach_expr_generic::(init, &value, optional, cursor, per_init) }, @@ -9405,14 +9420,14 @@ fn each_repeat_generic( Err(e) if suppresses(&e, optional) => return Flow::Exhausted, Err(e) => return Flow::Escaped(Control::Error(e)), }; - let f = reroot_markers::(f, &root); + let f = reroot_for_reentry::(f, &root); let mut empty_rounds = 0usize; loop { let mut stopped = false; let mut produced_any = false; let mut budget_control = None; let mut budget = super::eval::REPEAT_WIDTH_BUDGET; - let flow = eval_each_owned_bridged::(&f, &owned, optional, &mut |v| { + let flow = eval_each_owned::(&f, &owned, optional, Reentry::Proven, &mut |v| { produced_any = true; if let Some(control) = super::eval::charge_budget(&mut budget, "repeat") { stopped = true; @@ -9539,8 +9554,7 @@ fn continue_pipe_element_generic( }; // #2642: `o` is a computed intermediate value, never // document-backed. - let rest_expr = demote_rebuilt_markers(rest.owned(), &RootWitness::Owned); - eval_each_owned::(&rest_expr, &o, optional, &mut |o| { + eval_each_owned::(rest.owned(), &o, optional, Reentry::REBUILT, &mut |o| { sink.push(GenericItem::Owned(o)) }) } @@ -9712,7 +9726,7 @@ fn run_try_handler_generic( sink: &mut dyn Sink, ) -> Flow { // #3036: see `try_payload_root`. Demoted ahead of both routes below. - let handler = reroot_markers::(handler, payload_root); + let handler = reroot_for_reentry::(handler, payload_root); let handler: &Expr = &handler; if let Some(c) = cursor { let stages = owned_identity_body_stages(handler); @@ -9735,7 +9749,7 @@ fn run_try_handler_generic( ); } } - eval_each_owned_bridged::(handler, &payload, optional, &mut |o| { + eval_each_owned::(handler, &payload, optional, Reentry::Proven, &mut |o| { sink.push(GenericItem::Owned(o)) }) } @@ -10597,7 +10611,9 @@ fn any_all_probe_item_generic( | GenericItem::LazyIndexRange(_) | GenericItem::LazySeq(_)) => { let elem = generic_item_into_owned::<_, S>(item)?; - eval_each_owned::(cond, &elem, false, &mut |o| probe(GenericItem::Owned(o))) + eval_each_owned::(cond, &elem, false, Reentry::REBUILT, &mut |o| { + probe(GenericItem::Owned(o)) + }) } }; if let Some(control) = escape { @@ -10957,9 +10973,9 @@ impl LoopState { Self::Document(v, cursor) => { eval_each_generic::(expr, v.clone(), optional, *cursor, sink) } - Self::Owned(o) => { - eval_each_owned::(expr, o, optional, &mut |o| sink.push(GenericItem::Owned(o))) - } + Self::Owned(o) => eval_each_owned::(expr, o, optional, Reentry::REBUILT, &mut |o| { + sink.push(GenericItem::Owned(o)) + }), } } @@ -18471,7 +18487,7 @@ fn path_context_component_each( PathNode::Owned(v) => v, _ => &OwnedValue::Null, }; - match eval_each_owned::(&expr, value, false, sink) { + match eval_each_owned::(&expr, value, false, Reentry::REBUILT, sink) { Flow::Escaped(control) => Some(control), Flow::Exhausted | Flow::Stopped { .. } => None, } @@ -20976,11 +20992,13 @@ fn try_path_context_absent_sink( // is by definition a value that is not one // (the array a slice built, a handler's output), // so nothing here can be a marker's own node. - Ok(resolved) => { - eval_each_owned::(&resolved, &owned, false, &mut |v| { - sink.push(GenericItem::Owned(v)) - }) - } + Ok(resolved) => eval_each_owned::( + &resolved, + &owned, + false, + Reentry::REBUILT, + &mut |v| sink.push(GenericItem::Owned(v)), + ), Err(e) => Flow::Escaped(Control::Error(e)), } } @@ -21922,7 +21940,7 @@ fn eval_builtin( let mapped = Expr::Builtin(Builtin::Map(f.clone())); let mut outputs: Vec = Vec::new(); if let Flow::Escaped(control) = - eval_each_owned::(&mapped, &entries, optional, &mut |v| { + eval_each_owned::(&mapped, &entries, optional, Reentry::REBUILT, &mut |v| { outputs.push(v); Demand::Continue }) @@ -22457,6 +22475,9 @@ fn eval_builtin( // is a different node). Demote any marker not proven to be // `cursor`'s own node before either resolution route below. let root = RootWitness::of(cursor.as_ref()); + // `reroot_markers`, not `reroot_for_reentry`: `path_expr` is the + // resolver's own argument, so the resolver-reaching node the + // precheck looks for is this builtin *around* it (#3122). let path_expr = reroot_markers::(path_expr, &root); let owned = owned_or_suppress!(to_owned_with_cursor::<_, S>(&value, cursor), optional); if reindex_bridge_is_identity(&owned) { @@ -22468,8 +22489,7 @@ fn eval_builtin( )); } let owned_builtin_expr = Expr::Builtin(builtin.clone()); - let builtin_expr = reroot_markers::(&owned_builtin_expr, &root); - eval_on_owned::(&builtin_expr, owned, optional) + eval_on_owned::(&owned_builtin_expr, owned, optional, Reentry::Against(root)) } // #2168: `getpath(P)` reads one node, and now costs one read. @@ -22857,12 +22877,12 @@ fn eval_builtin( // (e.g. `path(...)`/`del(...)` nested inside this builtin's own // argument), so it must be checked against this call's own root. let root = RootWitness::of(cursor.as_ref()); - let expr = reroot_markers::(&Expr::Builtin(builtin.clone()), &root).into_owned(); + let expr = Expr::Builtin(builtin.clone()); let owned = owned_or_suppress!( bridge_ambient_input::<_, S>(&expr, &value, cursor), optional ); - eval_on_owned::(&expr, owned, optional) + eval_on_owned::(&expr, owned, optional, Reentry::Against(root)) } } } @@ -24622,9 +24642,8 @@ fn owned_identity_values( optional: bool, root: &RootWitness, ) -> (Vec, Option) { - let expr = reroot_markers::(expr, root); let mut values = Vec::new(); - let flow = eval_each_owned_bridged::(&expr, value, optional, &mut |v| { + let flow = eval_each_owned::(expr, value, optional, Reentry::Against(*root), &mut |v| { values.push(v); Demand::Continue }); @@ -26484,10 +26503,13 @@ fn eval_owned_identity_stages( Expr::Error(_) | Expr::Builtin( Builtin::Empty | Builtin::Halt | Builtin::HaltError | Builtin::HaltErrorCode(_), - ) => { - let stage = reroot_markers::(stage, &id.root_witness()); - eval_each_owned_bridged::(&stage, &value, optional, &mut |_| Demand::Continue) - } + ) => eval_each_owned::( + stage, + &value, + optional, + Reentry::Against(id.root_witness()), + &mut |_| Demand::Continue, + ), _ => { let Some(rule) = owned_identity_rule(stage) else { unreachable!("owned_identity_pipe_supported admits ruled stages only") @@ -26556,8 +26578,6 @@ fn eval_owned_identity_stages( // for the node's own unrebuilt value, and demotes everything // otherwise -- rather than the blanket `Owned` witness the // #2642 review tried and reverted here. - let stage_expr = reroot_markers::(stage_expr, &id.root_witness()); - let stage_expr: &Expr = &stage_expr; let mut downstream: Option = None; let mut emit = |output: OwnedValue| -> Demand { let flow = match owned_identity_after_stage::( @@ -26618,12 +26638,24 @@ fn eval_owned_identity_stages( { match id.path() { Ok(path) => with_path_base(&path, || { - eval_each_owned_bridged::(stage_expr, &value, optional, &mut emit) + eval_each_owned::( + stage_expr, + &value, + optional, + Reentry::Against(id.root_witness()), + &mut emit, + ) }), Err(e) => Flow::Escaped(Control::Error(e)), } } - None => eval_each_owned_bridged::(stage_expr, &value, optional, &mut emit), + None => eval_each_owned::( + stage_expr, + &value, + optional, + Reentry::Against(id.root_witness()), + &mut emit, + ), }; match downstream { Some(flow) => flow, @@ -26746,6 +26778,28 @@ mod tests { assert_eq!(one_owned_json(b"[1]", "[path(.)]"), "[[]]"); } + /// #3122: `eval_single`'s own `Expr::Pipe` arm, `cursor.is_none()` branch + /// -- `eval_using` (the value-only entry point) handed a pipe with a + /// path-context stage (`[key]`). No cursor exists to answer `key` from + /// directly, so this arm materializes the value and hands the whole pipe + /// to `eval_on_owned` under `Reentry::REBUILT`, which reindexes it into + /// a throwaway document rather than losing the position outright + /// (`key` still answers `"a"`, not `null`). No existing test drives a + /// *multi-stage* pipe whose first stage alone needs path context through + /// this exact door. + #[test] + fn test_eval_using_pipe_with_path_context_stage_has_no_cursor_3122() { + let json = br#"{"a":{"b":1}}"#; + let index = JsonIndex::build(json); + let value = index.root(json).value(); + let expr = crate::jq::parse(".a | [key]").unwrap(); + let result = eval_using::(&expr, value); + assert_eq!( + result.collect_owned::().unwrap(), + vec![OwnedValue::Array(vec![OwnedValue::string("a")].into())] + ); + } + /// temporary JSON index is dropped, preserving the complete preorder. #[test] fn cursorless_recursive_descent_bridge_preserves_values_2661() { @@ -27100,6 +27154,29 @@ mod tests { } } + /// #3122: `eval_builtin`'s `Builtin::Path` arm, the fallback taken when + /// `path_expr` is not cursor-navigable (`first(.a)` -- a resolver call, + /// not a plain field/index/pipe chain `path_expr_is_cursor_navigable` + /// recognises) *and* the document holds a value + /// [`reindex_bridge_is_identity`] refuses -- a `NumberLiteral` past + /// `REINDEX_LITERAL_LEN_CAP` here. `builtin_path_on_owned`'s direct, + /// no-round-trip route is only sound when the bridge would be a no-op, + /// so this shape falls all the way through to `eval_on_owned` under + /// `Reentry::Against(root)` instead, the same round trip the pre-#2061 + /// evaluator always paid. + #[test] + fn test_path_non_navigable_falls_back_when_reindex_is_not_identity_3122() { + let long = "1".repeat(super::REINDEX_LITERAL_LEN_CAP + 1); + let json = alloc::format!(r#"{{"a":[1,2],"big":{long}}}"#); + let index = JsonIndex::build(json.as_bytes()); + let expr = crate::jq::parse("path(first(.a))").unwrap(); + let result = eval_with_cursor_using::(&expr, index.root(json.as_bytes())); + assert_eq!( + result.collect_owned::().unwrap(), + vec![OwnedValue::Array(vec![OwnedValue::string("a")].into())] + ); + } + /// The one thing [`reindex_bridge_is_identity`]'s `Int` arm rests on, and /// the one thing `round_trips_unchanged` cannot check because it /// normalizes `Int` before comparing: `to_json_for_reindex` spells a bare diff --git a/src/jq/expr.rs b/src/jq/expr.rs index f13ed82d7..094680753 100644 --- a/src/jq/expr.rs +++ b/src/jq/expr.rs @@ -127,10 +127,14 @@ pub struct Tracked { /// residual (#2889), not a new correctness gap. #3036 closed the same /// hole where the bind *and* the rebuild both run inside `eval.rs` (the /// input-queue bridge, a fold's UPDATE, a `|=` right-hand side): every -/// owned-value re-entry there (`eval_each_owned` and its siblings) demotes -/// every `Snapshot` marker, since a freshly re-indexed document cannot be -/// a node any earlier binding was frozen from, and the funnels above take -/// `eval_each_owned_bridged` so their own proof is kept. +/// owned-value re-entry there (`eval_each_owned` and its siblings) takes a +/// `Reentry` (#3122) saying what its value's root is: `Against(Owned)` for +/// a freshly re-indexed value no earlier binding could have been frozen +/// from, the funnel's own `RootWitness` at a funnel, `Proven` where an +/// enclosing funnel or re-entry already demoted for this document -- one +/// parameter and one derivation (`demote_for_reentry`) in place of the +/// demoting/non-demoting twins, so a new re-entry has to name its root +/// rather than pick a twin. /// - [`Origin::SnapshotAt`] -- a [`Origin::Snapshot`] that also knows /// *where* `.` was when it was frozen (#2978). Made only inside a /// resolver invocation, while the branch is trackable and the frame's diff --git a/tests/jq_cli_tests.rs b/tests/jq_cli_tests.rs index a14167179..1cd8feb99 100644 --- a/tests/jq_cli_tests.rs +++ b/tests/jq_cli_tests.rs @@ -63383,3 +63383,65 @@ fn test_any_all_cond_isvalid_loops_validate_only_what_they_read_2658() -> Result Ok(()) } + +/// #3122: `recurse(f)`'s level-0 run of `f` is a generic-funnel re-entry +/// like any other, and takes the cursor's own witness (`each_recurse_generic` +/// used to hand `f` to the walk without one). Pinned on both sides: a write +/// through `$x` from the node `$x` was bound from answers as jq does, and +/// one from a rebuilt copy or a different node refuses. Captured live against +/// jq 1.7.1. +#[test] +// jq filter literals like `{a:1}` are not formatting strings; clippy cannot +// tell the two apart from the brace shape alone. +#[allow(clippy::literal_string_with_formatting_args)] +fn test_tracked_var_recurse_level_zero_names_its_root_3122() -> Result<()> { + for (input, filter, want) in [ + ( + r#"{"a":1,"b":{"c":1}}"#, + ". as $x | [recurse(if .a == 1 then ($x.a = 9) else empty end)]", + r#"[{"a":1,"b":{"c":1}},{"a":9,"b":{"c":1}}]"#, + ), + ( + r#"{"a":1,"b":{"c":1}}"#, + ". as $x | [recurse(if .a == 1 then ($x.a = 9) else empty end; .a == 1)]", + r#"[{"a":1,"b":{"c":1}}]"#, + ), + ] { + let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(input))?; + assert_eq!( + (stdout.trim(), code), + (want, 0), + "#3122: `{filter}` must stay accepted (real jq accepts it); stderr={stderr:?}" + ); + } + for (input, filter) in [ + ( + r#"{"a":1,"b":{"c":1}}"#, + ". as $x | [.b | recurse(if .c == 1 then ($x.a = 9) else empty end)]", + ), + ( + r#"{"a":1}"#, + ". as $x | {a:1} | [recurse(if .a == 1 then ($x.a = 9) else empty end)]", + ), + ( + r#"{"a":1}"#, + ". as $x | {a:1} | [recurse(.b?; ($x.a = 9) | true)]", + ), + ( + r#"{"a":1}"#, + ". as $x | [.[]] | [recurse(if .[0]? == 1 then ($x.a = 9) else empty end)]", + ), + ] { + let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(input))?; + assert_eq!( + code, 5, + "#3122: `{filter}` must refuse (jq: Invalid path expression), got \ + stdout={stdout:?} stderr={stderr:?}" + ); + assert!( + stderr.contains("Invalid path expression"), + "#3122: `{filter}` -- stderr: {stderr:?}" + ); + } + Ok(()) +} diff --git a/tests/jq_eval_using_input_queue_gate_test.rs b/tests/jq_eval_using_input_queue_gate_test.rs index d166559cd..594414a03 100644 --- a/tests/jq_eval_using_input_queue_gate_test.rs +++ b/tests/jq_eval_using_input_queue_gate_test.rs @@ -1,10 +1,11 @@ //! #1504's `input_queue_is_active` + `uses_input_builtins` gate was added to //! *both* `eval_generic::eval_using` and `eval_generic::eval_with_cursor_using`, -//! but `succinctly jq`/`succinctly yq` only ever call the cursor-preserving -//! half (`eval_with_cursor_using`, via `jq_runner.rs`/`yq_runner.rs`) -- so no -//! CLI-driven integration test can reach `eval_using`'s own copy of the gate. -//! It is reachable only through the library's non-cursor `eval`/`eval_using` -//! entry point, which is what this file drives directly. +//! but `succinctly jq`/`succinctly yq`'s own routing (`jq_runner.rs`'s +//! `can_use_lazy_path`) excludes every program that uses `input`/`inputs` +//! from the lazy/generic path entirely, sending it to the eager evaluator +//! instead -- so no CLI-driven integration test can reach *either* copy of +//! the gate. Both are reachable only through the library's own generic +//! entry points, which is what this file drives directly. //! //! Seeding the input queue (`seed_remaining_inputs`) is thread-local and, //! once seeded, `input_queue_is_active()` stays true for the rest of that @@ -14,7 +15,7 @@ #![cfg(feature = "std")] -use succinctly::jq::eval_generic::eval_using; +use succinctly::jq::eval_generic::{eval_using, eval_with_cursor_using}; use succinctly::jq::{parse, seed_remaining_inputs, JqSemantics, OwnedValue}; use succinctly::json::JsonIndex; @@ -38,3 +39,27 @@ fn test_eval_using_interleaves_input_with_top_level_comma_1504() { assert_eq!(outputs, vec![r#"{"a":1}"#.to_string(), "42".to_string()]); } + +/// #3122: the cursor-preserving twin of the test above -- same gate, same +/// bridge shape (`Reentry::Against(RootWitness::of(Some(&cursor)))` rather +/// than `eval_using`'s cursor-less `Reentry::REBUILT`), reached only through +/// `eval_with_cursor_using` directly, for the same "the CLI never sends this +/// route an input/inputs program" reason the module doc above gives. +#[test] +fn test_eval_with_cursor_using_interleaves_input_with_top_level_comma_3122() { + let json = br#"{"a":1}"#; + let index = JsonIndex::build(json); + + seed_remaining_inputs(vec![(OwnedValue::Int(42), 0, 1)], None); + + let expr = parse("(., input)").expect("parse failed"); + let result = eval_with_cursor_using::(&expr, index.root(json)); + let outputs: Vec = result + .collect_owned::() + .unwrap() + .iter() + .map(OwnedValue::to_json) + .collect(); + + assert_eq!(outputs, vec![r#"{"a":1}"#.to_string(), "42".to_string()]); +}