From 402a0ea9891801fb9eb961d3594dad8a30239b86 Mon Sep 17 00:00:00 2001 From: John Ky Date: Fri, 18 Sep 2026 16:26:24 +1000 Subject: [PATCH 1/2] fix(jq): carry the path register into nested pipes and catch handlers (#3133) A pipe nested under `try`/`if`/`,` is resolved by `resolve_node_sink`, which has no `PathBranch` to carry the register in, so it started register-less: a `$w` marker inside it could not re-establish, its navigation raised the resolver's own refusal, and `try` caught that as jq's `try` catches its own path errors -- but jq had no error to catch, `$w` being its register, and the write was silently discarded: `del(. as {a:$w} | try ($w | .b))` on `{"a":{"b":1}}` echoed the document where jq writes `{"a":{}}`. `Frame` gains the register's value beside its position (`register`, an `Rc` -- a refcount bump under #2999's sharing). An untracked stage sets it from `carried_register`; `resolve_seq_sink` seeds a nested pipe from it when the `Pipe` arm passes none; the `AsPattern` arm hands it to `resolve_as_pattern`; every constructor that moves or forgets `at` clears it. The outer stage then trusts a trackable step out of an untracked stage that carried a register (`step_may_reestablish`, generalising #3120's `step_certified`): from an untracked input only a re-establishment against that register can produce one. A `catch` handler gets the register jq restores at the `try`'s entry (its fork point saves and restores the path state; confirmed live, `null | path(try (.a | error(null)) catch .b)` is `["b"]`): a seeded pipe, driven through `resolve_seq_stage` directly so its untracked output still reaches the stages after the `try`. This closes the `catch-handler-var` row and the `$q[0]`-inside-`if` artefact the #2649 guard existed for. A pre-existing write-through the register would have widened is closed too: `. as $x` on an untracked stage binds a computed value, not a node, yet carried `Origin::Snapshot`, and its value rule certified a constructed copy -- `del(.a | {b:{c:1}} | . as $x | $x)` deleted `.a` where jq refuses. `identity_bind_position` answers `Untracked` for it; a marker source keeps its own origin, and a null/bool `.` is admitted by value as jq's is. Six REFUSE_ONLY sweep entries go; 23 rows and a widened fuzz alphabet (nested try/if/comma bodies, catch handlers) pin the change against jq 1.7.1, and two refuse-only residuals are recorded. --- docs/compliance/jq/limitations.md | 98 +++-- scripts/jq-bind-origin-fuzz.py | 19 +- scripts/jq-bind-origin-oracle-sweep.sh | 40 +- src/jq/eval.rs | 553 ++++++++++++++++++++----- tests/jq_cli_tests.rs | 30 +- 5 files changed, 583 insertions(+), 157 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 1fe30d1a2..73c59f221 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -882,7 +882,6 @@ is the revert that established what the other one costs. | `path(.a[0:3] as $y \| .a \| $y)` on `{"a":[1,2,3]}` | `["a"]` | jq's full slice *is* the array; the bind path ends in a slice component and `.a` does not | | `path(.a as $y \| (.c \| $y \| .b) as $w \| .a.b \| $w)` | `["a","b"]` | a marker is re-rooted only at the head of a source (`$y.b as $w`, `(($y \| .b) \| .c) as $w`); elsewhere it is certified against the ambient position | | `path(.a[1:] as $y \| .a[1:3] \| $y)` on `{"a":[1,2,3]}` | `["a",{"start":1,"end":3}]` | jq's `.a[1:]` and `.a[1:3]` of a 3-array are the same jv (same offset and length); the slice components differ, so the spelling never matches | - | `path(.a as $y \| .a \| try error("x") catch $y)` | `["a"]` | the handler resolves under an unknown frame and a raising `try` stage does not carry the register — pre-existing: `path(. as $x \| try error("x") catch $x)` refuses too (jq `[]`) | | `path(.a as $y \| .a \| 5 \| reduce (1) as $i (0; $y))` | `["a"]` | after a literal the register is only *carried*, and a fold whose INIT is untracked seeds its own register from the ambient literal — pre-existing: `path(. as $x \| 5 \| reduce (1) as $i (0; $x))` refuses too (jq `[]`) | Two related divergences were pre-existing and out of scope for #2042, tracked separately: @@ -1087,14 +1086,18 @@ is the revert that established what the other one costs. `test_as_pattern_arm_is_jq_mode_only_2649` pins that outcome as unaffected by this change. `?//` alternatives retry as they do in value mode, with one deliberate exception: the - **artefact guard**. This resolver raises refusals jq never raises — a nested `Pipe` carries - no register, so a `$q[0]` under an `if` or a `,` raises near-access even though the register - is standing right there — and retrying on one of those lands on the *wrong* alternative, + **artefact guard**. This resolver can raise refusals jq never raises — until + [#3133](https://github.com/rust-works/succinctly/issues/3133) a nested `Pipe` carried no + register, so a `$q[0]` under an `if` or a `,` raised near-access even though the register + was standing right there — and retrying on one of those lands on the *wrong* alternative, which a write then goes through. `path(. as {a:$q} ?// $z \| if $q then $q[0] else $z end)` is `["a",0]` in jq and would otherwise have answered `[]`, and the matching `del(. as {a:$q} ?// $z \| if $q then $q[0] else $z end)` on `{"a":[1,2,3]}` is `{"a":[2,3]}` in jq and would have deleted through that fabricated `[]`. So a body error of the resolver's - own two kinds (`UntrackedNavigation`/`InvalidPathExpression`) never retries. (On an + own two kinds (`UntrackedNavigation`/`InvalidPathExpression`) never retries. (Since #3133 + the untracked stage's `Frame` carries the register into that nested pipe, so both rows + answer as jq does — the guard is kept for the refusals that remain artefacts, such as a + register lost to an opaque stage.) (On an untracked stage the arm once kept the old opaque-leaf fall-through, since it could not see the register the first step is compared against; since [#3120](https://github.com/rust-works/succinctly/issues/3120) `resolve_seq_stage` hands it @@ -1156,9 +1159,10 @@ is the revert that established what the other one costs. Two refusals remain where jq might answer, and one refuses with different wording, all deliberate: the resolver's *own* refusals - never retry (a nested pipe carries no register, so `$q[0]` inside an `if` refuses here - where jq navigates, and retrying on that artefact would bind a different alternative than - jq and write through it), and a walk refusal retries only when it is jq's own verdict. + never retry (before #3133 a nested pipe carried no register, so `$q[0]` inside an `if` + refused here where jq navigates, and retrying on that artefact would have bound a different + alternative than jq and written through it; the rule stays for the artefacts that remain), + and a walk refusal retries only when it is jq's own verdict. For a source element that is not register-derived, the walk compares the element with the register by value where jq compares nodes; when the two are equal and not `null`/boolean, or the register is lost, the refusal is a guess and propagates instead: @@ -1213,50 +1217,56 @@ is the revert that established what the other one costs. re-establish against). Where no register can be handed in, the walk refuses unconditionally rather than guess. - Three refuse-only residuals, each pinned in `scripts/jq-bind-origin-oracle-sweep.sh`, and - one pre-existing discarded write: - - a `catch` handler runs under an untracked frame with no register (the `try` body may have - navigated before raising): `path(try error(null) catch (. as {a:$q} \| $q))` on a `null` - document is `["a"]` in jq and refuses here. It *agreed* before #3120 only by comparing - the `null` payload with the handler's own `null` ambient, and that same comparison wrote - `del(...)` → `{}` on `{"a":1}` where jq refuses — so the coincidence is not kept; - - a nested pipe (inside `if`/`try`, not a parenthesised group, which is flattened into the - enclosing pipe) carries no register, so a `null`/`bool` register behind a non-matching - literal cannot be recognised there: `path(.a \| 5 \| if true then (null as {b:$v} \| $v) - else . end)` on `{"a":null}` is `["a","b"]` in jq and refuses here (it refused before - #3120 too). When the literal *matches* the register (`.a \| null \| if ...` on - `{"a":null}`) the literal re-establishes it and the nested bind is trackable, so that - shape answers; + Two of the residuals #3120 recorded here — a `catch` handler with no register, and a pipe + nested under `if`/`try` with a `null`/`bool` register behind a non-matching literal — are + closed by [#3133](https://github.com/rust-works/succinctly/issues/3133) below. What + remains, each pinned in `scripts/jq-bind-origin-oracle-sweep.sh`: - an *opaque* stage (`reduce`, a `def` call, `first(..)`) drops the carried register (`cannot_move_register`, #1573), so after one the walk refuses without retrying: `del(reduce 1 as $i (null; null) \| . as [$v] ?// $v \| empty)` on `{"a":{"b":null}}` echoes the document in jq (the step is refused there too, and the retried `$v` body is `empty`) and exits 5 here. `main` echoed it by the ambient-`null` coincidence the fix removes. - - **Not refuse-only:** a pipe nested under `try`/`?` in the *body* carries no register, - so a `$w` marker there cannot re-establish, its navigation raises the resolver's own - refusal, and `try` catches it exactly as jq's `try` catches its own path errors — but jq - had no error to catch, because `$w` *is* its register. On a trackable stage this is - pre-existing (`del(. as {a:$w} \| try ($w \| .b))` on `{"a":{"b":1}}` is `{"a":{}}` in - jq and a silent no-op — exit 0, document echoed — on `main`). On an untracked stage the - same body used to refuse loudly only because the *walk* refused; now that the walk - answers, the body's discard is reached there too: `del(. as $x \| 5 \| $x as {a:$w} \| - try ($w \| .b))`, `del(... \| ($w \| .b)?)` and, via the bare alternative, `del(. as $x - \| 5 \| $x as [$w] ?// $z \| ($z \| .a)?)` (jq `{}`) all echo the document. The same - loss in the *source* position: `del(.a as $x \| .a \| 5 \| try ($x as {b:$q} \| $q))` - is `{"a":{}}` in jq and echoes here, as on `main`. The nested pipe is where the register - is lost, not the pattern arm; closing it means threading the carried register through - `resolve_node_sink`'s `Try`/`If`/`Comma` arms into their nested pipes — the "nested pipe - carries no register" limitation this section already records for `$q[0]` under `if`. - Filed as [#3133](https://github.com/rust-works/succinctly/issues/3133); the rows are - pinned as a characterization in - `test_nested_try_body_discards_the_write_characterization_3133` (`src/jq/eval.rs`) so - that fix flips them visibly. - a later-step refusal after a marker-certified first step does not retry `?//` (review): `refusal_is_exact` is decided per source (`bound != register`), and a marker that *is* the register is value-equal to it, so `path(.a as $x \| .a \| 5 \| $x as {b:[$q,$r]} ?// $w \| $w)` on `{"a":{"b":[1]}}` refuses at element `0` of `[1]` where jq retries onto `$w` and answers `["a"]`; the same shape on a trackable stage retries and agrees. + + **The register reaches a nested pipe and a `catch` handler** — + [#3133](https://github.com/rust-works/succinctly/issues/3133). A pipe nested under + `try`/`if`/`,` is resolved by `resolve_node_sink`, which has no `PathBranch` to carry the + register in, so it used to start register-less: a `$w` marker there could not + re-establish, its navigation raised the resolver's own refusal, and `try` caught it as jq's + `try` catches its own path errors — but jq had no error to catch, since `$w` *is* its + register, and the write was silently discarded: `del(. as {a:$w} \| try ($w \| .b))` on + `{"a":{"b":1}}` echoed the document where jq writes `{"a":{}}` (and `del(.a as $x \| .a \| + 5 \| try ($x as {b:$q} \| $q))` likewise, the same loss in the source position). An + untracked stage's `Frame` now carries the register's *value* beside its position, and a + nested pipe seeds itself from it; the outer stage trusts a trackable step out of such a + stage, since from an untracked input only a re-establishment against that very register + can produce one. A `catch` handler gets the register jq restores at the `try`'s entry (its + fork point saves and restores the path state — confirmed live, `null \| path(try (.a \| + error(null)) catch .b)` is `["b"]`, and with a non-null register the handler's `.b` + refuses): `path(.a as $y \| .a \| try error(1) catch $y)` is jq's `["a"]` (the + `catch-handler-var` row the #2042 table carried), and the `null`-document destructuring + row #3120 recorded as a residual answers again — for the right reason this time. The + handler's output is still handed on to the stages after the `try` rather than refused on + the spot (`[path(.a \| (try error(null) catch .) \| empty)]` is `[]`). + + The same change closes a pre-existing write-through: `. as $x` on an *untracked* stage + binds a value the stage computed, not a node, yet carried `Origin::Snapshot`, whose value + rule then certified a constructed copy against the register — `del(.a \| {b:{c:1}} \| . as + $x \| $x)` on `{"a":{"b":{"c":1}}}` deleted `.a` where jq refuses, and with the register + now reaching nested pipes `... \| $x \| .b \| .c` would have navigated through it too. + Such a bind is `Origin::Untracked` now (`identity_bind_position`); a marker source keeps + its own origin, and a `null`/`bool` `.` loses nothing since `jv_identical` admits those by + value. Two refuse-only residuals, both in the sweep: `path(.a \| try error(.) catch .)` — + `error(.)` raises the register node itself and jq answers `["a"]`, but a payload equal to + the register by value cannot be told from a rebuilt copy (`error({"a":1,"b":2})` refuses in + jq), so the handler stays untracked unless the payload is `null`/`bool`; and `(if true + then $x else . end) as $y` on an untracked stage, which binds `Untracked` because the + condition is not evaluated where jq evaluates it and binds the marker. 3. **jq's pointer-identity artifacts on `*`/`+` with an empty operand** — `path(. as $x \| reduce (1) as $i (0; $x + {}))` on `{"a":1}` is `[]` in jq; succinctly refuses (likewise `$x * {}` and `$x + null`). This is not a rule jq implements but an @@ -1765,7 +1775,11 @@ answers `["b"]` — and classified the two residuals appended below): - a `try` whose body holds an `if` whose condition happens *not* to raise — `path(.a | (try (if true then . else . end) catch 1) as $x | .k | $x | getpath(["k"]) | .b)`, `["a","k","b"]` in jq. The raise-free gate above is static and cannot tell it from the - raising twin, so the bind is a plain value; + raising twin, so the bind is a plain value. Note that when such a `$x` is then navigated + *inside* a `try` (`((try (if .a then . else . end) catch 1) as $v | try ($v | .b?)) |= .`), + the plain value's refusal is caught by that `try` exactly as jq's own path errors are, and + the write jq performs (`"b":null`) is silently skipped rather than refused — the same + static gate, surfacing through `try` instead of as an exit 5 (found by #3133's fuzz); - an `if` bind source whose arms sit at *different* positions — `path(. as $p | .a | (if true then $p else . end) as $x | $x | getpath(["a"]) | .b)`, `["a","b"]` in jq. `identity_bind_position` is static (the condition is not evaluated), diff --git a/scripts/jq-bind-origin-fuzz.py b/scripts/jq-bind-origin-fuzz.py index 135927f74..5b495841e 100755 --- a/scripts/jq-bind-origin-fuzz.py +++ b/scripts/jq-bind-origin-fuzz.py @@ -180,7 +180,24 @@ def doc(rng): # `["a","c"]`. The second form composes back onto the register. "(.a | ($v as $w | .c | $w | getpath([\"c\"]) | .b?))", "(.a | ($v as $w | .c | $w | getpath([\"a\",\"c\"]) | .b?))", - "(.x | ($v as $w | .a | $w | getpath([\"x\",\"a\"]) | .b?))"] + "(.x | ($v as $w | .a | $w | getpath([\"x\",\"a\"]) | .b?))", + # #3133: the register reaches a pipe nested under `try`/`if`/`,` and + # a `catch` handler (jq restores it to the `try`'s entry). A marker + # navigating inside such a pipe used to raise the resolver's own + # refusal, which `try`/`?` then swallowed into a discarded write -- + # so both halves are drawn: the marker that *is* the register + # (`$v` right after its own bind) and, through `SOURCES`' sibling + # copies, one that merely equals it. The catch shapes pair a + # payload that is the register (`error($v)`, `error(null)` on a + # null register) with one that is a rebuilt copy. + "try ($v | .b?)", "(try ($v | .b?) catch .)", "($v | .b?)?", + "(if true then ($v | .b?) else 1 end)", "(($v | .b?), 1)", + "try (($v | .[]?) | .b?)", "(try error($v) catch $v)", + "(try error($v) catch ($v | .b?))", "(try error(null) catch ($v | .b?))", + "(try error(1) catch $v)", "(5 | try error(1) catch $v)", + "(try error(.) catch .)", "(try error({b:1}) catch .b?)", + "(. as $q | 5 | $q as {a:$w} | try ($w | .b?))", + "(try (. as {a:$w} | $w | .b?) catch .)"] # #2978: an optional navigation *prefix* before the first bind, so `. as $v` # can be drawn below the invocation root. `program()` put every bind at the diff --git a/scripts/jq-bind-origin-oracle-sweep.sh b/scripts/jq-bind-origin-oracle-sweep.sh index d52f99c3e..7746c2550 100755 --- a/scripts/jq-bind-origin-oracle-sweep.sh +++ b/scripts/jq-bind-origin-oracle-sweep.sh @@ -45,6 +45,15 @@ # write operators, plus the two refusals kept on purpose (the reasons are in # REFUSE_ONLY below and in limitations.md). # +# The `nested-*`, `catch-*` and `computed-identity-bind-*` rows are #3133: an +# untracked stage's `Frame` carries the register's value into a pipe nested +# under `try`/`if`/`,` and into a `catch` handler (jq restores the register to +# the `try`'s entry when it catches), so a marker there re-establishes and a +# write through it lands -- `del(. as {a:$w} | try ($w | .b))` echoed the +# document where jq writes. The same change makes `. as $x` on an untracked +# stage an Untracked marker (a computed `.` is no node), closing a +# pre-existing write-through of a constructed copy. +# # The `untracked-*` rows are #3043/#3120: a destructuring bind on an # *untracked* stage checks its first pattern step against the register the # pipe carried in (`resolve_seq_stage` hands it to `resolve_as_pattern`), @@ -420,6 +429,29 @@ untracked-bare-alt-keeps-register-del {"a":1} del(. as $x | 5 | $x as [$v] ?// $ untracked-opaque-stage-lost-register {"a":{"b":null}} del(reduce 1 as $i (null; null) | . as [$v] ?// $v | empty) untracked-later-step-refusal-no-retry {"a":{"b":[1]}} path(.a as $x | .a | 5 | $x as {b:[$q,$r]} ?// $w | $w) untracked-later-step-refusal-trackable-twin {"a":{"b":[1]}} path(.a as $x | .a | $x as {b:[$q,$r]} ?// $w | $w) +nested-try-body-keeps-register {"a":{"b":1}} del(. as {a:$w} | try ($w | .b)) +nested-try-body-keeps-register-untracked {"a":{"b":1}} del(. as $x | 5 | $x as {a:$w} | try ($w | .b)) +nested-optional-body-keeps-register {"a":{"b":1}} del(. as $x | 5 | $x as {a:$w} | ($w | .b)?) +nested-optional-bare-alt-keeps-register {"a":1} del(. as $x | 5 | $x as [$w] ?// $z | ($z | .a)?) +nested-try-source-keeps-register {"a":{"b":1}} del(.a as $x | .a | 5 | try ($x as {b:$q} | $q)) +nested-try-sibling-copy-control {"a":{"b":1},"c":{"b":1}} del(.a as $y | .c | 5 | try ($y | .b)) +nested-pipe-null-register {"a":null} path(.a | 5 | (null | .b)) +nested-pipe-non-null-register {"a":{"b":1}} path(.a | 5 | (null | .b)) +catch-restores-entry-register null path(try (.a | error(null)) catch .b) +catch-restores-entry-register-refuses {"x":{"a":1,"b":2}} path(.x | try (.a | error(null)) catch .b) +catch-rebuilt-payload-refuses {"x":{"a":1,"b":2}} path(.x | try (.a | error({"a":1,"b":2})) catch .b) +catch-marker-reestablishes-untracked {"a":{"b":1}} path(.a as $y | .a | 5 | try error(1) catch $y) +catch-marker-reestablishes-del {"a":{"b":1}} del(.a as $y | .a | 5 | try error(1) catch $y) +catch-marker-navigates {"a":{"b":1}} path(.a as $y | .a | try error(1) catch ($y | .b)) +catch-payload-handed-on {"a":{"b":1}} [path(.a | (try error(null) catch .) | empty)] +catch-payload-handed-on-refuses {"a":{"b":1}} path(.a | (try error(null) catch .) | .b) +catch-payload-own-node-refuse-only {"a":{"b":1}} path(.a | try error(.) catch .) +computed-identity-bind-untracked {"a":{"b":{"c":1}}} path(.a | {b:{c:1}} | . as $x | $x) +computed-identity-bind-untracked-del {"a":{"b":{"c":1}}} del(.a | {b:{c:1}} | . as $x | $x) +computed-identity-bind-navigates {"a":{"b":{"c":1}}} path(.a | {b:{c:1}} | . as $x | $x | .b | .c) +computed-identity-bind-null-register {"a":null} path(.a | null | . as $x | $x | .b) +computed-identity-bind-marker-source {"a":{"b":{"c":1}}} path(. as $x | 5 | $x as $y | $y) +computed-identity-bind-mixed-if {"a":{"b":{"c":1}}} path(. as $x | 5 | (if true then $x else . end) as $y | $y) CASES_EOF # Known refuse-only rows (jq answers, succinctly refuses), each with the @@ -442,12 +474,8 @@ negative-index-spelling:a negative index is stored as written, so .[-2] never ma full-slice-is-the-array:jq's full slice is the array itself; the bind path ends in a slice component, .a does not marker-not-at-head:a marker is re-rooted only at the head of a source; elsewhere it is certified against the ambient position slice-spelling:jq's .a[1:] and .a[1:3] of a 3-array are the same jv; the slice components differ, so the spelling never matches (open-ended twin of full-slice-is-the-array) -catch-handler-var:the handler resolves under an unknown frame and a raising try stage does not carry the register; pre-existing, the root marker refuses too -literal-then-fold-untracked-init:after a literal the register is only carried, and a fold with an untracked INIT seeds its register from the ambient literal; pre-existing, the root marker refuses too destructure-bind-after-pattern:#2649 residue 1 -- a plain bind on the ambient input after a pattern moved the register: resolve_bind_source needs a trackable stage, and the pattern's body stage is not destructure-alt-navigation:#2649 residue 3 -- the body navigates the ambient input, which raises a near-access refusal the artefact guard cannot tell from an artefact, so the ?// does not retry -destructure-alt-artefact-guard:#2649 artefact guard -- MUST stay a refusal: retrying here would answer [] where jq answers ["a",0]; answering anything makes this row mismatch (fabrication assertion, not a bare allowlist entry) -destructure-alt-artefact-guard-del:#2649 artefact guard -- the write twin: a retry would delete the whole document where jq deletes .a[0] destructure-comma-marker-nav:#2649 residue 4 -- pre-existing comma shape: a nested Pipe gets no register, so $q[0] inside a comma raises near-access (limitations.md, #2042) carried-register-passthrough:pre-existing (#2042): once the register is only *carried* (an untracked stage), a select/label/first/getpath passthrough re-seeds it from the ambient value and the marker no longer re-establishes; if/try/`. as $q | .`/literals keep it. Twin of literal-then-fold-untracked-init, found by the #2649 fuzz destructure-passthrough-stage:the destructuring door onto carried-register-passthrough -- a pattern body starts on an untracked stage, so the same select/label/first/getpath passthroughs drop the register; the baseline binary refuses the plain-bind twin identically, so this is not #2649's @@ -466,10 +494,10 @@ navigated-bind-owned-root:#3037 residual -- a navigated bind on an owned-rooted navigated-bind-input-root:#3037 residual -- same as navigated-bind-owned-root, on the input-queue route identity-if-arms-differ:#2978 -- identity_bind_position is static: an if whose arms sit at different positions ($p at [], . at ["a"]) proves neither, so the bind stays a bare Snapshot and getpath has no position to compose from; jq evaluates the condition identity-try-if-nonraising:#2978 review -- a try body holding an if is not a passthrough (its condition may raise and bind the value of the handler); the gate is static, so an if whose condition happens not to raise pays a refusal. The raising twin (identity-trap-raising-try-*) is the write-side fabrication this prevents -untracked-catch-handler-null-document:#3120 -- a catch handler runs under an untracked frame with no register in hand, since the try body's navigation may have moved it before raising, so the walk refuses; jq answers only because this document is null. The same rule fabricated a del on any other document, see untracked-catch-handler-del, so this coincidence is not kept -untracked-nested-if-null-register:#3120 -- a nested pipe inside if/try carries no register, so with a null register behind a non-matching literal the walk cannot know jq's verdict, which is to accept here; refused before #3120 too untracked-opaque-stage-lost-register:#3120 review -- an opaque stage (reduce, a def call, first) drops the carried register, so the walk has none and refuses without retrying; jq refuses the step too and retries onto the bare alternative, whose empty body then writes nothing. main echoed the document by the ambient-null coincidence the fix removes untracked-later-step-refusal-no-retry:#3120 review -- refusal_is_exact is decided per source, and a marker that is the register is value-equal to it, so a later-step refusal after a certified first step is treated as a guess and does not retry; jq retries onto $w. The trackable twin retries and agrees +catch-payload-own-node-refuse-only:#3133 -- error(.) raises the register node itself and jq answers ["a"]; the payload equals the register by value but is not null/bool and carries no marker, so it cannot be told from a rebuilt copy (catch-rebuilt-payload-refuses) and the handler stays untracked +computed-identity-bind-mixed-if:#3133 -- an if source with one arm a computed `.` and the other a marker binds Untracked on an untracked stage (the condition is not evaluated); jq evaluates it and binds the marker REFUSE_EOF if [[ "${1:-}" == "--list-cases" ]]; then diff --git a/src/jq/eval.rs b/src/jq/eval.rs index 91ab5ec60..7f5ed716d 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -28906,10 +28906,30 @@ fn slice_bound_component_value(bound: Option, key: Option<&SliceBoundKey>) /// navigated position at all ([`Frame::enter`]'s syntactic gate), so the /// per-stage extension below is free for the ordinary write workloads /// (`.[] |= f`, `del(.[] | select(..))`) that never use one. +/// +/// `register` (#3133) is the register's *value* at `at`, carried only while +/// the branch this frame belongs to is untracked and the enclosing pipe +/// still holds the register (`resolve_seq_stage`'s `carried_register`) -- +/// `None` while trackable (the register is the ambient value itself) and +/// wherever `at` is not provable. It exists so a pipe nested under +/// `try`/`if`/`,`, and a `catch` handler, which `resolve_node_sink` reaches +/// with the same ambient value but no `PathBranch` to carry the register in, +/// can seed themselves with it instead of resolving register-less: `del(. as +/// {a:$w} | try ($w | .b))` discarded the write jq performs, because `$w` +/// had nothing to re-establish against inside the `try`. It is *value* +/// information and independent of `at`: the `null`/`bool` identity rule and +/// a `Snapshot` marker's value rule need no position (`null | path(try (.a | +/// error(null)) catch .b)` is `["b"]` in a program whose `at` the +/// [`may_bind_navigated`] gate left `None`), while an `Origin::At` marker +/// still certifies against `at` alone. It is cleared by every constructor +/// that moves or forgets the position, so the same one-directional +/// invariant covers it: a wrong `Some` would re-establish a value on a +/// register it is not identical to. #[derive(Debug, Clone)] pub(crate) struct Frame { invocation: u64, at: Option>, + register: Option>, } /// Next [`Frame::invocation`]. A plain atomic rather than a @@ -28923,7 +28943,11 @@ impl Frame { fn enter(expr: &Expr) -> Self { let invocation = NEXT_INVOCATION.fetch_add(1, core::sync::atomic::Ordering::Relaxed) as u64; let at = may_bind_navigated(expr).then(PathPrefix::root); - Self { invocation, at } + Self { + invocation, + at, + register: None, + } } /// `invocation`, at the absolute position `path` -- how a marker-headed @@ -28932,6 +28956,7 @@ impl Frame { Self { invocation, at: Some(path), + register: None, } } @@ -28940,9 +28965,27 @@ impl Frame { Self { invocation: self.invocation, at: None, + register: None, + } + } + + /// The same frame carrying `register` as the live register's value + /// (#3133), or none. + fn with_register(&self, register: Option<&OwnedValue>) -> Self { + Self { + invocation: self.invocation, + at: self.at.clone(), + // Structural sharing (#2999) makes this clone a refcount bump + // for a container, and the register is nearly always one. + register: register.map(|value| Rc::new(value.clone())), } } + /// The register value this frame carries, if any (#3133). + fn register(&self) -> Option<&OwnedValue> { + self.register.as_deref() + } + /// The same invocation, `path` further along from `at`. Free at the /// invocation root, where `path` already is the absolute path, and /// for an empty `path`; otherwise one node per component of `path`. @@ -28959,6 +29002,13 @@ impl Frame { Self { invocation: self.invocation, at, + // A moved position is a different node; only an empty `path` + // keeps the register (#3133). + register: if path.depth() == 0 { + self.register.clone() + } else { + None + }, } } @@ -30971,14 +31021,39 @@ fn resolve_node_sink<'a, S: EvalSemantics>( // message shape this resolver did not reproduce at all; `resolve_catch` // below now raises it correctly by marking the handler's payload // untracked, #843.) + // + // #3133: a caught error restores jq's register to where it was at + // the `try`'s entry (its fork point saves and restores the path + // state) -- confirmed live, `null | path(try (.a | error(null)) + // catch .b)` is `["b"]`, and with a non-null register the handler's + // `.b` refuses against it. That register is `value` itself while + // trackable, otherwise whatever this stage's frame carries; the + // handler is resolved with it in hand (`resolve_catch_sink`). Expr::Try { expr, catch } => { + let entry_register = if trackable { + Some(value) + } else { + frame.register() + }; match resolve_node_sink::(expr, value, trackable, snapshot, frame, keep, sink) { ResolveFlow::Escaped(EvalEscape::Error(e)) if !e.is_uncatchable() => { - resolve_catch_sink::(catch.as_deref(), e.payload(), frame, keep, sink) - } - ResolveFlow::Escaped(EvalEscape::Break(_)) => { - resolve_catch_sink::(catch.as_deref(), OwnedValue::Null, frame, keep, sink) + resolve_catch_sink::( + catch.as_deref(), + e.payload(), + frame, + entry_register, + keep, + sink, + ) } + ResolveFlow::Escaped(EvalEscape::Break(_)) => resolve_catch_sink::( + catch.as_deref(), + OwnedValue::Null, + frame, + entry_register, + keep, + sink, + ), other => other, } } @@ -31108,7 +31183,19 @@ fn resolve_node_sink<'a, S: EvalSemantics>( patterns, body, } if S::TAG == EvalTag::Jq => resolve_as_pattern::( - expr, patterns, body, value, trackable, snapshot, frame, keep, None, sink, + expr, + patterns, + body, + value, + trackable, + snapshot, + frame, + keep, + // #3133: nested under `try`/`if`/`,`, the frame carries the + // register the enclosing stage held; `resolve_seq_stage`'s own + // dispatch hands it in directly. + frame.register(), + sink, ), // #2234: `stderr`/`debug`/`debug(msg)` are true identity passthroughs // in jq -- their value-mode implementations (`builtin_stderr`/ @@ -36418,6 +36505,7 @@ fn resolve_catch_sink<'a, S: EvalSemantics>( catch: Option<&Expr>, payload: OwnedValue, frame: &Frame, + entry_register: Option<&OwnedValue>, keep: Keep, sink: &mut dyn FnMut(PathBranch<'a>) -> Demand, ) -> ResolveFlow { @@ -36426,15 +36514,50 @@ fn resolve_catch_sink<'a, S: EvalSemantics>( }; // A caught error/break payload is never a snapshot (#1591): jq's own // handler binding is unrelated to any `$x` frozen elsewhere in scope. - resolve_against_cow_sink::( - catch_expr, - Cow::Owned(payload), - false, - &Snapshot::No, - &frame.unknown(), - keep, - sink, - ) + // + // #3133: the handler runs against the payload with the register jq + // restored at the `try`'s entry -- at this frame's own position (`at` is + // the register's position, trackable or carried) and holding + // `entry_register`. It is resolved as a seeded pipe, exactly as a + // destructuring bind's body is: untracked with that register in hand, + // so a `$y` marker for it re-establishes (`path(.a as $y | .a | try + // error(1) catch $y)` is jq's `["a"]`; it refused under the old + // `frame.unknown()`), or trackable outright when the payload is a + // `null`/`bool` identical to the register by jq's `jv_identical` (`null + // | path(try (.a | error(null)) catch .b)` is `["b"]`). A frame whose + // position is not provable carries no register either (`with_register` + // keeps them tied), so nothing widens there: the seed is then a plain + // untracked branch, as before. + // + // Driven through `resolve_seq_stage` directly, every stage dynamic, + // rather than `resolve_seq_from_seed`: that function's static-tail fast + // path refuses an untracked seed on the spot, which is right for a + // pattern body (it ends the pipe) but not for a handler, whose untracked + // output must reach the stages after the `try` exactly as the direct + // route used to hand it on -- `[path(.a | (try error(null) catch .) | + // empty)]` is jq's `[]`, and an eager refusal here turned it into an + // exit 5. + let frame = frame.with_register(entry_register); + let seed = match frame.register() { + Some(register) if null_bool_identical(&payload, register) => { + PathBranch::new(PathPrefix::root(), Cow::Owned(payload), true) + } + Some(register) => { + PathBranch::passthrough(PathPrefix::root(), Cow::Owned(payload), false, Snapshot::No) + .with_register(Some(Cow::Owned(register.clone()))) + } + None => PathBranch::untracked(Cow::Owned(payload)), + }; + let mut flat = Vec::new(); + push_path_components(&mut flat, catch_expr); + let Some(last) = flat.len().checked_sub(1) else { + // A no-op handler (`catch .`): the payload itself, handed on. + return match sink(seed) { + Demand::Continue => ResolveFlow::Exhausted, + Demand::Stop => ResolveFlow::Stopped, + }; + }; + resolve_seq_stage::(&flat, last, 0, seed, &frame, keep, sink) } /// Shared "defer the escape, queue what's left" step for `recurse`'s two @@ -39473,7 +39596,12 @@ fn resolve_seq_sink<'a, S: EvalSemantics>( .with_register(if trackable { None } else { - register.map(Cow::Borrowed) + // #3133: a nested pipe (the `Pipe` arm passes `None`) inherits the + // register the enclosing stage's frame carries, so `try ($w | .b)` + // can re-establish `$w` exactly as the bare `$w | .b` stage does. + register + .map(Cow::Borrowed) + .or_else(|| frame.register().map(|reg| Cow::Owned(reg.clone()))) }); resolve_seq_from_seed::(exprs, seed, frame, keep, sink) } @@ -39721,7 +39849,14 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( // non-navigating stage has stepped off it (`prefix` does not advance // for those). What an `Origin::At` marker met in this stage, directly // or through a passthrough, is certified against. - let stage_frame = frame.extend(&prefix); + // #3133: an untracked stage's frame also carries the register's *value*, + // so a pipe nested in this stage (under `try`/`if`/`,`) or its `catch` + // handler can seed itself with it -- see `Frame::register`. + let stage_frame = frame.extend(&prefix).with_register(if branch_trackable { + None + } else { + carried_register.as_deref() + }); // Whether this stage can carry a live path register across itself // at all (#1573). `false` is the answer for every stage this // resolver cannot see inside, because jq's register moves on any @@ -39746,13 +39881,19 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( &stage_frame, ); let mut downstream: Option = None; - // `step_certified` (#3120): the step came from the untracked-stage - // `AsPattern` route below, which checked its pattern walk against the - // carried register itself and seeded its body with the register the walk - // moved to -- so a trackable step from it is a proven re-establishment - // at `prefix + components`, not something "untracked is absorbing" may - // demote. Every other route passes `false` and keeps that rule. - let mut place_step = |step: PathBranch<'a>, step_certified: bool| -> Demand { + // Whether a *trackable* step out of an untracked stage is a proven + // re-establishment rather than something "untracked is absorbing" must + // demote (#3120, generalised by #3133). From an untracked input, no route + // can produce a trackable branch except by re-establishing against a + // register -- `resolve_leaf` hands back untracked branches, the recurse + // family refuses -- and the only register any nested route has is the + // one this stage handed down (`stage_frame.register()`: the `AsPattern` + // route's walk and seeded body, a pipe under `try`/`if`/`,`, a `catch` + // handler). So when this stage carried one, the step's own verdict + // stands, at `prefix + components`; with none in hand the rule below is + // unchanged and a nested route cannot have re-established anything. + let step_may_reestablish = stage_frame.register().is_some(); + let mut place_step = |step: PathBranch<'a>| -> Demand { let PathBranch { path: components, value: resulting, @@ -39789,14 +39930,23 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( } else { PathPrefix::extend_many(&prefix, components.to_vec()) }; + let trackable = + reestablished || ((branch_trackable || step_may_reestablish) && step_trackable); let placed = PathBranch { - register: carry_register( - &facts, - reestablished, - trackable_step_eligible, - &carried_register, - step_register, - ), + // A trackable branch is its own register (`with_register`'s + // invariant); only an untracked one carries the live register + // forward. + register: if trackable { + None + } else { + carry_register( + &facts, + reestablished, + trackable_step_eligible, + &carried_register, + step_register, + ) + }, path, value: resulting, // Untracked is absorbing: nothing downstream can @@ -39814,7 +39964,7 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( // so the variable re-establishes the register's position // rather than reaching a new one. See // `reestablishes_register`. - trackable: reestablished || ((branch_trackable || step_certified) && step_trackable), + trackable, // Unlike `trackable`, this comes from the *step* alone: // the value leaving this stage is the step's own output, // so `.a | $x` still hands back the frozen snapshot @@ -39877,7 +40027,7 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( &stage_frame, stage_keep, carried_register.as_deref(), - &mut |step| place_step(step, true), + &mut place_step, ), _ => resolve_against_cow_sink::( element, @@ -39886,7 +40036,7 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( &branch_snapshot, &stage_frame, stage_keep, - &mut |step| place_step(step, false), + &mut place_step, ), }; match downstream { @@ -40739,21 +40889,30 @@ pub(crate) fn substitute_bound_var_from( substitute_bound_var_at(bind_expr, body, var_name, bound, None, None, node) } -/// The absolute position an identity-passthrough bind source freezes `.` -/// at, when provable (#2978) -- the [`Origin::SnapshotAt`] its marker -/// should carry instead of a bare [`Origin::Snapshot`], or `None` to keep -/// the bare one. Recurses over exactly [`is_identity_passthrough`]'s -/// grammar, so a source that predicate does not recognise answers `None` -/// here too (and never reaches [`substitute_bound_var_at`]'s identity -/// branch anyway). +/// The [`Origin`] an identity-passthrough bind source freezes `.` with, +/// inside a resolver invocation (#2978, #3133): the [`Origin::SnapshotAt`] +/// its marker should carry instead of a bare [`Origin::Snapshot`] when the +/// position is provable, the [`Origin::Untracked`] it must carry when `.` +/// is not a document node at all, or `None` to keep the bare `Snapshot`. +/// Recurses over exactly [`is_identity_passthrough`]'s grammar, so a source +/// that predicate does not recognise answers `None` here too (and never +/// reaches [`substitute_bound_var_at`]'s identity branch anyway). /// /// - `.`: while the branch is `trackable`, the register *is* the ambient /// `.`, and `Frame::at` is its absolute path by #2042's invariant (or /// `None` when not provable) -- so the position is exactly `frame`'s, /// including below the invocation root (`path(.x | . as $v | .a | $v | -/// getpath(["a"]) | .b)` is `["x","a","b"]`). Off the register the frame -/// names the *carried* register, not `.`, and nothing is minted: `path(.a -/// | {b:{c:1}} | . as $x | $x | getpath(["b"]) | .c)` must keep refusing. +/// getpath(["a"]) | .b)` is `["x","a","b"]`). Off the register, `.` is a +/// value the stage *computed* (a literal, a construction, an opaque +/// call), not the register's node nor any node's: the marker is +/// `Untracked` (#3133). `Snapshot`'s value rule is sound only for a +/// snapshot of a real node, and a constructed copy equal to the register +/// is exactly the rebuilt copy #2642 closed at the funnels -- `del(.a | +/// {b:{c:1}} | . as $x | $x)` on `{"a":{"b":{"c":1}}}` deleted `.a` +/// where jq refuses, and once a pipe nested in the body carries the +/// register (#3133) `... | $x | .b | .c` navigated through it too. A +/// `null`/`bool` `.` loses nothing: jq's `jv_identical` admits those by +/// value, and so does [`register_identical`], marker or not. /// - a marker source (`$x as $y`): **the marker's own** position, never the /// frame's -- `$x` bound at the root and rebound at `["a"]` is still the /// root's node, and `path(. as $x | .a | ($x as $y | .c | $y | @@ -40805,7 +40964,9 @@ fn identity_bind_position( invocation: frame.invocation, path: BindPath(Rc::clone(at)), }), - Expr::Identity => None, + Expr::Identity => Some(Origin::Untracked), + // A marker source rebinds the marker's own node: its origin + // travels with it, whatever stage the rebind happens on. Expr::TrackedVar(marker) => match &marker.origin { Origin::SnapshotAt { .. } => Some(marker.origin.clone()), Origin::Snapshot | Origin::At { .. } | Origin::Untracked => None, @@ -40815,9 +40976,20 @@ fn identity_bind_position( else_branch, .. } => { - let then_at = position(then_branch, trackable, frame)?; - let else_at = position(else_branch, trackable, frame)?; - (then_at == else_at).then_some(then_at) + let then_at = position(then_branch, trackable, frame); + let else_at = position(else_branch, trackable, frame); + match (then_at, else_at) { + // Both arms prove the same thing. + (a, b) if a == b => a, + // One arm is a computed `.`: the bind may be that, so + // it can only be certified as it would be (#3133). + (Some(Origin::Untracked), _) | (_, Some(Origin::Untracked)) => { + Some(Origin::Untracked) + } + // Two positions that differ, or a position beside a + // bare snapshot: the bare value rule. + _ => None, + } } Expr::Try { expr, .. } if is_raise_free_identity_passthrough(expr) => { position(expr, trackable, frame) @@ -40836,7 +41008,9 @@ fn identity_bind_position( /// what a navigated sibling binding would have gotten -- or, when the /// resolver call site could prove where `.` was frozen /// ([`identity_bind_position`], #2978), the `Origin::SnapshotAt` it -/// passes as `identity_at`, which is that same rule plus a position. +/// passes as `identity_at`, which is that same rule plus a position; or, +/// when that site knows `.` was a computed value rather than a node +/// (#3133), the `Origin::Untracked` it passes instead. /// Otherwise an explicit `origin` (the #2042 witness: a navigated source /// resolved *inside* a `path()`/`del()`/assignment invocation, from /// [`resolve_bind_source_witness`]) is kept as-is. With neither but a @@ -40856,8 +41030,11 @@ fn substitute_bound_var_at( ) -> Expr { let origin = if is_identity_passthrough(bind_expr) { debug_assert!( - matches!(identity_at, None | Some(Origin::SnapshotAt { .. })), - "identity_at is a SnapshotAt or nothing: {identity_at:?}" + matches!( + identity_at, + None | Some(Origin::SnapshotAt { .. } | Origin::Untracked) + ), + "identity_at is a SnapshotAt, an Untracked, or nothing: {identity_at:?}" ); identity_at.unwrap_or(Origin::Snapshot) } else if let Some(origin) = origin { @@ -96422,6 +96599,15 @@ mod tests { fn test_path_bind_origin_matrix_accepts_2042() { // (input, filter, jq 1.7.1's `-c '[FILTER]'`) let rows: &[(&[u8], &str, &str)] = &[ + // catch-handler-var, refuse-only until #3133: jq restores the + // register to the `try`'s entry when it catches, and the + // handler now runs with that register in hand, so `$y` -- which + // *is* it -- re-establishes + ( + br#"{"a":{"b":1}}"#, + r#"path(.a as $y | .a | try error("x") catch $y)"#, + r#"[["a"]]"#, + ), // destructure-stage (#2649): `[$q]` on an object is jq's own // "Cannot index object with number", the `?// $q` alternative // performs no step and restores the register, so `$y` is still @@ -96939,15 +97125,6 @@ mod tests { "path(.a[1:] as $y | .a[1:3] | $y)", r#"[["a",{"start":1,"end":3}]]"#, ), - // catch-handler-var: the handler resolves against the error - // payload under an unknown frame, and a raising `try` stage does - // not carry the register (pre-existing: the root-marker - // spelling refuses too) - ( - br#"{"a":{"b":1}}"#, - r#"path(.a as $y | .a | try error("x") catch $y)"#, - r#"[["a"]]"#, - ), ]; for (input, filter, jq_answer) in rows { match bind_origin_outputs(input, filter) { @@ -97174,29 +97351,35 @@ mod tests { /// comparing the `null` payload with the handler's own *ambient* `null` /// rather than with jq's register. The same rule fabricated on any other /// document: on `{"a":1}` jq refuses (the register is the root, and - /// `null` is not it) while `del(...)` wrote `{}` (#3043/#3120). A catch - /// handler runs under an untracked frame with no register in hand (the - /// try body may have navigated before raising), so the walk now refuses - /// unconditionally there: the fabrication is closed, and the `null` - /// document's coincidental agreement is a recorded refuse-only residual - /// (`limitations.md`). - #[test] - fn test_as_pattern_in_catch_handler_refuses_without_a_register_3120() { - for doc in [&b"null"[..], &br#"{"a":1}"#[..]] { - for filter in [ - r"path(try error(null) catch (. as {a:$q} | $q))", - r"del(try error(null) catch (. as {a:$q} | $q))", - ] { - query!(doc, filter, - QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => { - assert_eq!( - e.message, - r#"Invalid path expression near attempt to access element "a" of null"#, - "{filter}" - ); - } - ); - } + /// `null` is not it) while `del(...)` wrote `{}` (#3043/#3120). #3120 + /// made the handler refuse unconditionally (no register in hand); #3133 + /// hands it the register jq restores at the `try`'s entry, so the walk + /// is checked against the real one: `["a"]` on the `null` document (the + /// register *is* `null`), jq's refusal on `{"a":1}`, both read and + /// written. + #[test] + fn test_as_pattern_in_catch_handler_checks_the_entry_register_3133() { + assert_eq!( + outputs(b"null", r"path(try error(null) catch (. as {a:$q} | $q))"), + [r#"["a"]"#] + ); + assert_eq!( + outputs(b"null", r"del(try error(null) catch (. as {a:$q} | $q))"), + ["null"] + ); + for filter in [ + r"path(try error(null) catch (. as {a:$q} | $q))", + r"del(try error(null) catch (. as {a:$q} | $q))", + ] { + query!(br#"{"a":1}"#, filter, + QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => { + assert_eq!( + e.message, + r#"Invalid path expression near attempt to access element "a" of null"#, + "{filter}" + ); + } + ); } } @@ -98954,41 +99137,211 @@ mod tests { } } - /// #3133 characterization (found in #3120's review): a pipe nested under - /// `try`/`?` in a pattern body carries no register, so a `$w` marker - /// there cannot re-establish, its navigation raises the resolver's own - /// refusal, and `try` catches it -- where jq, whose register `$w` *is*, - /// writes. jq answers `{"a":{}}` (row 1-3) and `{}` (row 4); succinctly - /// echoes the document at exit 0. Row 1 does so on `main` too; rows 2-4 - /// used to refuse loudly only because the untracked-stage *walk* - /// refused, and reach the same discard now that the walk answers. This - /// pins the current wrong outputs on purpose so the #3133 fix flips a - /// test rather than a silent row -- see `limitations.md`. + /// #3133: a pipe nested under `try`/`?` in a pattern body used to carry + /// no register, so a `$w` marker there could not re-establish, its + /// navigation raised the resolver's own refusal, and `try` caught it -- + /// the write jq performs was silently discarded (exit 0, document + /// echoed; row 1 on `main`, rows 2-4 once #3120 let the walk answer). + /// The stage's frame now carries the register into the nested pipe, so + /// every row writes exactly what jq writes. #[test] - fn test_nested_try_body_discards_the_write_characterization_3133() { - for (doc, filter, echoed) in [ + fn test_nested_try_body_keeps_the_register_3133() { + for (doc, filter, want) in [ ( &br#"{"a":{"b":1}}"#[..], r"del(. as {a:$w} | try ($w | .b))", - r#"{"a":{"b":1}}"#, + r#"{"a":{}}"#, ), ( &br#"{"a":{"b":1}}"#[..], r"del(. as $x | 5 | $x as {a:$w} | try ($w | .b))", - r#"{"a":{"b":1}}"#, + r#"{"a":{}}"#, ), ( &br#"{"a":{"b":1}}"#[..], r"del(. as $x | 5 | $x as {a:$w} | ($w | .b)?)", - r#"{"a":{"b":1}}"#, + r#"{"a":{}}"#, ), ( &br#"{"a":1}"#[..], r"del(. as $x | 5 | $x as [$w] ?// $z | ($z | .a)?)", - r#"{"a":1}"#, + "{}", + ), + // The issue's own source-position twin, and its read. + ( + &br#"{"a":{"b":1}}"#[..], + r"del(.a as $x | .a | 5 | try ($x as {b:$q} | $q))", + r#"{"a":{}}"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"(.a as $x | .a | 5 | try ($x as {b:$q} | $q)) |= 9", + r#"{"a":{"b":9}}"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a as $x | .a | 5 | try ($x as {b:$q} | $q))", + r#"["a","b"]"#, + ), + // ... and the sibling-copy control stays exactly jq's: `$y` is + // not the register, `try` catches jq's own refusal, nothing is + // written. + ( + &br#"{"a":{"b":1},"c":{"b":1}}"#[..], + r"del(.a as $y | .c | 5 | try ($y | .b))", + r#"{"a":{"b":1},"c":{"b":1}}"#, + ), + ] { + assert_eq!(outputs(doc, filter), [want], "{filter}"); + } + } + + /// #3133: a `catch` handler runs with the register jq restores at the + /// `try`'s entry (confirmed live against jq 1.7.1 for every row), and a + /// `null`/`bool` payload identical to it seeds the handler trackable. + /// The refusal rows are the controls: a non-null register against a + /// `null` payload, a payload equal to the register but rebuilt, and the + /// handler's untracked output reaching -- not pre-empting -- the stages + /// after the `try`. + #[test] + fn test_catch_handler_runs_with_the_entry_register_3133() { + for (doc, filter, want) in [ + ( + &b"null"[..], + r"path(try (.a | error(null)) catch .b)", + r#"["b"]"#, + ), + ( + &br#"{"a":null}"#[..], + r"path(.a | (try error(null) catch .) | .b)", + r#"["a","b"]"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a as $y | .a | try error(1) catch $y)", + r#"["a"]"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a as $y | .a | try (.b | error(1)) catch $y)", + r#"["a"]"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a as $y | .a | try error(1) catch ($y | .b))", + r#"["a","b"]"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(. as $x | try error($x) catch $x)", + "[]", + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"[path(.a | (try error(null) catch .) | empty)]", + "[]", + ), + ] { + assert_eq!(outputs(doc, filter), [want], "{filter}"); + } + for (doc, filter, message) in [ + ( + &br#"{"x":{"a":1,"b":2}}"#[..], + r"path(.x | try (.a | error(null)) catch .b)", + r#"Invalid path expression near attempt to access element "b" of null"#, + ), + ( + &br#"{"x":{"a":1,"b":2}}"#[..], + r#"path(.x | try (.a | error({"a":1,"b":2})) catch .b)"#, + r#"Invalid path expression near attempt to access element "b" of {"a":1,"b":2}"#, + ), + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a | (try error(null) catch .) | .b)", + r#"Invalid path expression near attempt to access element "b" of null"#, + ), + // Refuse-only: `error(.)` raises the register's own node, which + // a value-equal payload cannot be told apart from a rebuilt copy. + ( + &br#"{"a":{"b":1}}"#[..], + r"path(.a | try error(.) catch .)", + r#"Invalid path expression with result {"b":1}"#, + ), + ] { + query!(doc, filter, + QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => { + assert_eq!(e.message, message, "{filter}"); + } + ); + } + } + + /// #3133: `. as $x` on an untracked stage binds a value the stage + /// computed, not a node, so its marker is `Untracked` -- before, it was + /// a `Snapshot`, and the value rule certified a constructed copy against + /// the register: `del(.a | {b:{c:1}} | . as $x | $x)` deleted `.a` on + /// `main` where jq refuses (and with the register reaching nested pipes, + /// `$x | .b | .c` would have navigated through it too). A marker source + /// keeps its own origin, and a `null`/`bool` `.` is admitted by value as + /// jq's `jv_identical` admits it. + #[test] + // jq filter literals like `{b:{c:1}}` are not formatting strings. + #[allow(clippy::literal_string_with_formatting_args)] + fn test_identity_bind_on_untracked_stage_is_untracked_3133() { + let doc = br#"{"a":{"b":{"c":1}}}"#; + for (filter, message) in [ + ( + r"path(.a | {b:{c:1}} | . as $x | $x)", + r#"Invalid path expression with result {"b":{"c":1}}"#, + ), + ( + r"del(.a | {b:{c:1}} | . as $x | $x)", + r#"Invalid path expression with result {"b":{"c":1}}"#, + ), + ( + r"path(.a | {b:{c:1}} | . as $x | $x | .b | .c)", + r#"Invalid path expression near attempt to access element "b" of {"b":{"c":1}}"#, + ), + ( + r#"path(.a | {b:{c:1}} | . as $x | $x | getpath(["b"]) | .c)"#, + r#"Invalid path expression near attempt to access element "c" of {"c":1}"#, + ), + ( + r"path(.a | 5 | (try . catch 1) as $x | $x)", + "Invalid path expression with result 5", + ), + // Refuse-only: the condition is not evaluated, so a mixed `if` + // binds Untracked where jq binds the marker. + ( + r"path(. as $x | 5 | (if true then $x else . end) as $y | $y)", + r#"Invalid path expression with result {"a":{"b":{"c":1}}}"#, + ), + ] { + query!(doc, filter, + QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => { + assert_eq!(e.message, message, "{filter}"); + } + ); + } + for (doc, filter, want) in [ + ( + &br#"{"a":null}"#[..], + r"path(.a | null | . as $x | $x | .b)", + r#"["a","b"]"#, + ), + ( + &br#"{"a":true}"#[..], + r"path(.a | true | . as $x | $x)", + r#"["a"]"#, + ), + (&doc[..], r"path(. as $x | 5 | $x as $y | $y)", "[]"), + ( + &doc[..], + r"path(.a | select(true) | . as $x | $x)", + r#"["a"]"#, ), ] { - assert_eq!(outputs(doc, filter), [echoed], "{filter} (see #3133)"); + assert_eq!(outputs(doc, filter), [want], "{filter}"); } } diff --git a/tests/jq_cli_tests.rs b/tests/jq_cli_tests.rs index 38e1f610f..a14167179 100644 --- a/tests/jq_cli_tests.rs +++ b/tests/jq_cli_tests.rs @@ -56554,24 +56554,38 @@ fn test_destructuring_moves_path_register_2649() -> Result<()> { // the only assertions; each row is otherwise a pure jq answer, not a // succinctly one. for (input, filter) in [ - // jq: ["a",0] + // jq: ["a"] + (d, "path(. as {a:$q} | .a as $z | $z)"), + // jq: ["a"] + (d, "path(. as {a:$q} ?// $z | .a)"), + ] { + let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(input))?; + assert_eq!(code, 5, "`{filter}`: stdout={stdout} stderr={stderr}"); + assert!(stdout.is_empty(), "`{filter}` must not print: {stdout}"); + } + // The artefact-guard pair this block once carried as "MUST stay a + // refusal" answers since #3133: the `if` body's nested pipe now carries + // the register the pattern moved to, so `$q[0]` navigates from it + // instead of raising the artefact the guard existed for -- jq's own + // `["a",0]`, and the `del` twin's `{"a":[2,3]}`, no retry involved. + for (input, filter, want) in [ ( r#"{"a":[1,2,3]}"#, "path(. as {a:$q} ?// $z | if $q then $q[0] else $z end)", + r#"["a",0]"#, ), - // jq: {"a":[2,3]} ( r#"{"a":[1,2,3]}"#, "del(. as {a:$q} ?// $z | if $q then $q[0] else $z end)", + r#"{"a":[2,3]}"#, ), - // jq: ["a"] - (d, "path(. as {a:$q} | .a as $z | $z)"), - // jq: ["a"] - (d, "path(. as {a:$q} ?// $z | .a)"), ] { let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(input))?; - assert_eq!(code, 5, "`{filter}`: stdout={stdout} stderr={stderr}"); - assert!(stdout.is_empty(), "`{filter}` must not print: {stdout}"); + assert_eq!( + (stdout.trim_end(), code), + (want, 0), + "`{filter}` (jq's answer since #3133): stderr={stderr}" + ); } // One former refuse-only pin of this block answers since #3120: the // marker-headed source on an untracked stage is checked against the From d20e12236a91d4d51f8bddd809465c61426430c0 Mon Sep 17 00:00:00 2001 From: John Ky Date: Fri, 18 Sep 2026 17:10:28 +1000 Subject: [PATCH 2/2] fix(jq): gate the catch register to jq mode; a caught break is never the register (#3133 review) Three findings from /code-review, plus one recording: - A caught `break`'s payload is jq's `{"__jq":N}` label object, modelled here as `null`, so the new null/bool seed made the handler trackable on a null register: `(.a | label $out | try (break $out) catch .b) = 1` on `{"a":null}` wrote where jq and main both refuse. The seed now takes a `payload_may_be_node` flag, false for the break arm. - The entry register was not gated to jq mode, so `(.a | try error(null) catch .b) = 1` wrote in yq mode, where real yq's lexer rejects `try` and there is no oracle. Both the `Try` arm and `resolve_seq_stage`'s own `with_register` are jq-only now, matching every other register admission in this file. - Comments claimed the register was tied to a provable `at`; it is not (the value rules need no position), and `resolve_catch_sink`'s header still said it streamed through `resolve_against_cow_sink`. Both corrected. - The same silent-discard class survives in a fold's UPDATE/EXTRACT body (`FoldRegister::resolve` resolves under a register-less frame) -- pre-existing, filed as #3145 and recorded in limitations.md. Rows pinned: the break pair (read and write), the marker that still re-establishes through a caught break, and the yq-mode refusals. --- docs/compliance/jq/limitations.md | 8 +- scripts/jq-bind-origin-oracle-sweep.sh | 2 + src/jq/eval.rs | 117 ++++++++++++++++++------- 3 files changed, 96 insertions(+), 31 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 73c59f221..ef3da3bdd 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -1261,7 +1261,13 @@ is the revert that established what the other one costs. now reaching nested pipes `... \| $x \| .b \| .c` would have navigated through it too. Such a bind is `Origin::Untracked` now (`identity_bind_position`); a marker source keeps its own origin, and a `null`/`bool` `.` loses nothing since `jv_identical` admits those by - value. Two refuse-only residuals, both in the sweep: `path(.a \| try error(.) catch .)` — + value. The same register does **not** yet reach a fold's UPDATE/EXTRACT body: that route + (`FoldRegister::resolve`) passes its register to `resolve_seq` explicitly under a frame that + carries none, so `del(foreach .a as $v (.; try ($v \| .b); .))` on `{"a":{"b":1}}` still + echoes the document where jq writes `{"a":{}}`, as does `del(.a as $y \| .a \| 5 \| foreach + range(1) as $i (0; .; try ($y \| .b)))` — pre-existing, filed as + [#3145](https://github.com/rust-works/succinctly/issues/3145). Two refuse-only residuals, + both in the sweep: `path(.a \| try error(.) catch .)` — `error(.)` raises the register node itself and jq answers `["a"]`, but a payload equal to the register by value cannot be told from a rebuilt copy (`error({"a":1,"b":2})` refuses in jq), so the handler stays untracked unless the payload is `null`/`bool`; and `(if true diff --git a/scripts/jq-bind-origin-oracle-sweep.sh b/scripts/jq-bind-origin-oracle-sweep.sh index 7746c2550..34c1bcdad 100755 --- a/scripts/jq-bind-origin-oracle-sweep.sh +++ b/scripts/jq-bind-origin-oracle-sweep.sh @@ -452,6 +452,8 @@ computed-identity-bind-navigates {"a":{"b":{"c":1}}} path(.a | {b:{c:1}} | . as computed-identity-bind-null-register {"a":null} path(.a | null | . as $x | $x | .b) computed-identity-bind-marker-source {"a":{"b":{"c":1}}} path(. as $x | 5 | $x as $y | $y) computed-identity-bind-mixed-if {"a":{"b":{"c":1}}} path(. as $x | 5 | (if true then $x else . end) as $y | $y) +catch-break-payload-not-register {"a":null} (.a | label $out | try (break $out) catch .b) = 1 +catch-break-restores-register {"a":null} path(.a as $y | .a | label $out | try (break $out) catch $y) CASES_EOF # Known refuse-only rows (jq answers, succinctly refuses), each with the diff --git a/src/jq/eval.rs b/src/jq/eval.rs index 7f5ed716d..ff95ff888 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -28907,11 +28907,11 @@ fn slice_bound_component_value(bound: Option, key: Option<&SliceBoundKey>) /// per-stage extension below is free for the ordinary write workloads /// (`.[] |= f`, `del(.[] | select(..))`) that never use one. /// -/// `register` (#3133) is the register's *value* at `at`, carried only while +/// `register` (#3133) is the live register's *value*, carried only while /// the branch this frame belongs to is untracked and the enclosing pipe -/// still holds the register (`resolve_seq_stage`'s `carried_register`) -- -/// `None` while trackable (the register is the ambient value itself) and -/// wherever `at` is not provable. It exists so a pipe nested under +/// still holds the register (`resolve_seq_stage`'s `carried_register`, jq +/// mode only) -- `None` while trackable (the register is the ambient value +/// itself). It exists so a pipe nested under /// `try`/`if`/`,`, and a `catch` handler, which `resolve_node_sink` reaches /// with the same ambient value but no `PathBranch` to carry the register in, /// can seed themselves with it instead of resolving register-less: `del(. as @@ -31030,10 +31030,14 @@ fn resolve_node_sink<'a, S: EvalSemantics>( // trackable, otherwise whatever this stage's frame carries; the // handler is resolved with it in hand (`resolve_catch_sink`). Expr::Try { expr, catch } => { - let entry_register = if trackable { - Some(value) - } else { - frame.register() + // jq mode only, like every other register admission in this + // file: real yq's lexer rejects `try`, so there is no oracle, + // and yq's scalar-write no-op convention would turn a wrong + // acceptance into silent corruption rather than a loud error. + let entry_register = match (S::TAG, trackable) { + (EvalTag::Jq, true) => Some(value), + (EvalTag::Jq, false) => frame.register(), + _ => None, }; match resolve_node_sink::(expr, value, trackable, snapshot, frame, keep, sink) { ResolveFlow::Escaped(EvalEscape::Error(e)) if !e.is_uncatchable() => { @@ -31042,15 +31046,23 @@ fn resolve_node_sink<'a, S: EvalSemantics>( e.payload(), frame, entry_register, + true, keep, sink, ) } + // A caught `break`'s payload is jq's `{"__jq":N}` label + // object, modelled as `null` here (a wording residual): it + // is never the register's node, so the handler must not be + // seeded trackable on a `null` register (#3133 review: `(.a + // | label $out | try (break $out) catch .b) = 1` on + // `{"a":null}` wrote where jq refuses). ResolveFlow::Escaped(EvalEscape::Break(_)) => resolve_catch_sink::( catch.as_deref(), OwnedValue::Null, frame, entry_register, + false, keep, sink, ), @@ -36495,7 +36507,8 @@ fn resolve_foreach<'a, S: EvalSemantics>( /// select(true) = "X"` still raises rather than silently replacing the /// whole document, caught now by `resolve_dynamic_indexes` instead of here. /// -/// Streams through [`resolve_against_cow_sink`] rather than collecting via +/// Streams (through [`resolve_seq_stage`] since #3133, before that through +/// [`resolve_against_cow_sink`]) rather than collecting via /// [`resolve_against_cow`] (#2235): a catch handler that itself iterates a /// generator (e.g. `try f catch (.[] | stderr)`) streams that generator's /// own side effects to `sink` one at a time instead of batching them all @@ -36506,6 +36519,7 @@ fn resolve_catch_sink<'a, S: EvalSemantics>( payload: OwnedValue, frame: &Frame, entry_register: Option<&OwnedValue>, + payload_may_be_node: bool, keep: Keep, sink: &mut dyn FnMut(PathBranch<'a>) -> Demand, ) -> ResolveFlow { @@ -36516,18 +36530,19 @@ fn resolve_catch_sink<'a, S: EvalSemantics>( // handler binding is unrelated to any `$x` frozen elsewhere in scope. // // #3133: the handler runs against the payload with the register jq - // restored at the `try`'s entry -- at this frame's own position (`at` is - // the register's position, trackable or carried) and holding - // `entry_register`. It is resolved as a seeded pipe, exactly as a - // destructuring bind's body is: untracked with that register in hand, - // so a `$y` marker for it re-establishes (`path(.a as $y | .a | try - // error(1) catch $y)` is jq's `["a"]`; it refused under the old - // `frame.unknown()`), or trackable outright when the payload is a - // `null`/`bool` identical to the register by jq's `jv_identical` (`null - // | path(try (.a | error(null)) catch .b)` is `["b"]`). A frame whose - // position is not provable carries no register either (`with_register` - // keeps them tied), so nothing widens there: the seed is then a plain - // untracked branch, as before. + // restored at the `try`'s entry (`entry_register`, jq mode only), at + // this frame's own position when that is provable (`at` is the + // register's position, trackable or carried; the register itself does + // not need one -- see `Frame::register`). It is resolved as a seeded + // pipe, exactly as a destructuring bind's body is: untracked with that + // register in hand, so a `$y` marker for it re-establishes (`path(.a as + // $y | .a | try error(1) catch $y)` is jq's `["a"]`; it refused under + // the old `frame.unknown()`), or trackable outright when the payload is + // a `null`/`bool` identical to the register by jq's `jv_identical` + // (`null | path(try (.a | error(null)) catch .b)` is `["b"]`) -- for an + // *error* payload only (`payload_may_be_node`): a caught `break`'s is a + // label object jq never finds identical. With no register in hand the + // seed is a plain untracked branch, exactly as before. // // Driven through `resolve_seq_stage` directly, every stage dynamic, // rather than `resolve_seq_from_seed`: that function's static-tail fast @@ -36539,7 +36554,7 @@ fn resolve_catch_sink<'a, S: EvalSemantics>( // exit 5. let frame = frame.with_register(entry_register); let seed = match frame.register() { - Some(register) if null_bool_identical(&payload, register) => { + Some(register) if payload_may_be_node && null_bool_identical(&payload, register) => { PathBranch::new(PathPrefix::root(), Cow::Owned(payload), true) } Some(register) => { @@ -39849,14 +39864,19 @@ fn resolve_seq_stage<'a, S: EvalSemantics>( // non-navigating stage has stepped off it (`prefix` does not advance // for those). What an `Origin::At` marker met in this stage, directly // or through a passthrough, is certified against. - // #3133: an untracked stage's frame also carries the register's *value*, - // so a pipe nested in this stage (under `try`/`if`/`,`) or its `catch` - // handler can seed itself with it -- see `Frame::register`. - let stage_frame = frame.extend(&prefix).with_register(if branch_trackable { - None - } else { - carried_register.as_deref() - }); + // #3133: an untracked stage's frame also carries the register's *value* + // (jq mode only, like every other register admission here), so a pipe + // nested in this stage (under `try`/`if`/`,`) or its `catch` handler can + // seed itself with it -- see `Frame::register`. Independent of `at`: + // the frame may know the register's value without a provable position. + let stage_frame = + frame + .extend(&prefix) + .with_register(if branch_trackable || S::TAG != EvalTag::Jq { + None + } else { + carried_register.as_deref() + }); // Whether this stage can carry a live path register across itself // at all (#1573). `false` is the answer for every stage this // resolver cannot see inside, because jq's register moves on any @@ -99241,9 +99261,31 @@ mod tests { r"[path(.a | (try error(null) catch .) | empty)]", "[]", ), + // ... and a caught `break` restores the register like an error + // does: the marker re-establishes. + ( + &br#"{"a":null}"#[..], + r"path(.a as $y | .a | label $out | try (break $out) catch $y)", + r#"["a"]"#, + ), ] { assert_eq!(outputs(doc, filter), [want], "{filter}"); } + // yq mode gets none of this (review): real yq's lexer rejects + // `try`, so there is no oracle, and its scalar-write no-op + // convention would make a wrong acceptance a silent corruption -- + // the register is neither carried into a nested pipe nor restored + // in a handler there. `main`'s own refusals, unchanged. + for filter in [ + r"(.a | try error(null) catch .b) = 1", + r"del(. as {a:$w} | try ($w | .b))", + ] { + yq_query!(br#"{"a":{"b":1}}"#, filter, + QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => { + assert!(is_resolver_refusal(&e), "{filter}: {}", e.message); + } + ); + } for (doc, filter, message) in [ ( &br#"{"x":{"a":1,"b":2}}"#[..], @@ -99267,6 +99309,21 @@ mod tests { r"path(.a | try error(.) catch .)", r#"Invalid path expression with result {"b":1}"#, ), + // Review: a caught `break`'s payload is jq's `{"__jq":0}` label + // object (modelled as `null`, a wording residual), never the + // register's node -- on a `null` register the handler must not + // be seeded trackable. jq refuses with `element "b" of + // {"__jq":0}`; both read and write. + ( + &br#"{"a":null}"#[..], + r"path(label $out | try (break $out) catch .b)", + r#"Invalid path expression near attempt to access element "b" of null"#, + ), + ( + &br#"{"a":null}"#[..], + r"(.a | label $out | try (break $out) catch .b) = 1", + r#"Invalid path expression near attempt to access element "b" of null"#, + ), ] { query!(doc, filter, QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => {