fix(jq): carry the path register into nested pipes and catch handlers (#3133) - #3144
Conversation
CoverageTotal: 93.92% ⚪ 0.01 pp vs Comparing No per-file coverage changes vs 🔇 0 ignored region(s), 167 tolerated region(s)
Patch coveragePatch: 99.66% (296/297 new lines covered)
Uncovered new lines (1)
|
CoverageTotal: 94.01% ⚪ 0.01 pp vs Comparing No per-file coverage changes vs 🔇 0 ignored region(s), 166 tolerated region(s)
Patch coveragePatch: 99.66% (296/297 new lines covered)
Uncovered new lines (1)
|
…#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<OwnedValue>` -- 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.
…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.
91fabe7 to
d20e122
Compare
Fixes #3133
Refs #3120, #3043, #2649, #2979, #2042
Summary
A pipe nested under
try/if/,is resolved byresolve_node_sink, which has noPathBranchto carry the path register in, so it started register-less. A$wmarker inside it could not re-establish, its navigation raised the resolver's own refusal, andtrycaught that refusal exactly as jq'strycatches its own path errors — but jq had no error to catch, because$wis its register. The write was silently discarded:Fix — the register rides the
Frame.Framegains the register's value beside its position (register, anRc<OwnedValue>— a refcount bump under #2999's structural sharing).resolve_seq_stagesets it on an untracked stage's frame fromcarried_register;resolve_seq_sinkseeds a nested pipe from it when thePipearm passes none; theAsPatternarm hands it toresolve_as_pattern. It is cleared whereveratis moved or forgotten (extendby a non-empty path,unknown,at), so the same one-directional invariant covers it. The outer stage then trusts a trackable step out of an untracked stage that carried a register (step_may_reestablish, generalising #3120'sstep_certified): from an untracked input, only a re-establishment against that very register can produce one.The
catchhandler gets the register jq restores at thetry's entry. jq's 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.brefuses. The handler is now a seeded pipe with that register (trackable outright for anull/boolpayload identical to it), driven throughresolve_seq_stagedirectly so its untracked output still reaches the stages after thetry([path(.a | (try error(null) catch .) | empty)]stays[]). This closes thecatch-handler-varrow (path(.a as $y | .a | try error("x") catch $y)→["a"]) and thenull-document destructuring row #3120 recorded — for the right reason this time.A pre-existing write-through the fix would have widened, closed.
. as $xon an untracked stage binds a value the stage computed, not a node, yet carriedOrigin::Snapshot, whose value rule certified a constructed copy against the register:del(.a | {b:{c:1}} | . as $x | $x)on{"a":{"b":{"c":1}}}deleted.aonmainwhere jq refuses — and with the register reaching nested pipes,... | $x | .b | .cnavigated through it too (caught by the #2978 trap test while working on this). Such a bind isOrigin::Untrackednow (identity_bind_position); a marker source keeps its own origin; anull/bool.loses nothing.Rows that flip to jq's answer
$q[0]inside anifbody (path(. as {a:$q} ?// $z | if $q then $q[0] else $z end)→["a",0],del→{"a":[2,3]}) — thedestructure-alt-artefact-guardpair the jq: destructuring moves jq's path register — path(. as {a:$q} | $q) is ["a"] and path(. as {a:$q} | .a) raises #2649 CLI test pinned as "MUST stay a refusal" (the guard existed for exactly this artefact; it stays for the artefacts that remain).catch-handler-var,untracked-catch-handler-null-document,untracked-nested-if-null-register,literal-then-fold-untracked-init(already stale) — sixREFUSE_ONLYsweep entries removed.tryhas no register, andtryswallows the refusal —del(... | try ($x as {b:$q} | $q))silently discards the write #3133 rows plus the source-position twin, all written as jq writes; the sibling-copy control (.a as $y | .c | 5 | try ($y | .b)) still writes nothing, as in jq.Two refuse-only residuals recorded:
path(.a | try error(.) catch .)(jq["a"]; a payload equal to the register cannot be told from a rebuilt copy, which jq refuses) and(if true then $x else . end) as $yon an untracked stage (the condition is not evaluated).Acceptance oracle
scripts/jq-bind-origin-oracle-sweep.sh: 25 new rows (nested-*,catch-*,computed-identity-bind-*);agree=301 fabricate=0 mismatch=0 refuse-only=36 refuse-only-NEW=0.scripts/jq-bind-origin-fuzz.py, 3 seeds × 6000 against a merge-base build: see the test plan.Review round (
/code-review 3144, standard effort)Four findings, three fixed in a follow-up commit: a caught
break's payload is jq's{"__jq":N}label object (modelled asnull), so the new null/bool seed made the handler trackable on a null register and(.a | label $out | try (break $out) catch .b) = 1wrote where jq refuses — the seed now takes apayload_may_be_nodeflag; the entry register was not gated to jq mode, so the same shape wrote in yq mode, where real yq's lexer rejectstryand there is no oracle (both theTryarm andresolve_seq_stage'swith_registerare jq-only now); and two stale comment claims (the register is not tied to a provableat, andresolve_catch_sinkno longer streams throughresolve_against_cow_sink). The fourth — the same silent discard surviving in a fold's UPDATE/EXTRACT body, which resolves under a register-less frame — is pre-existing, filed as #3145 and recorded inlimitations.md. Suggest a human run/code-review ultrafor the deepest pass if wanted; not a merge gate here.Test plan
cargo build --features clicargo test --features cli,simd,regex,serdecargo clippy --all-targets --all-features -- -D warningscargo clippy --all-targets --features std,simd,serde,cli,regex,bench-runner,large-tests,mmap-tests -- -D warningscargo fmt --all -- --check/usr/bin/jq1.7.1 (unit matrices, CLI rows, sweep)--baselinetest_nested_try_body_keeps_the_register_3133(replaces the fix(jq): check a destructuring bind on an untracked stage against the carried register (#3043, #3120) #3130 characterization),test_catch_handler_runs_with_the_entry_register_3133,test_identity_bind_on_untracked_stage_is_untracked_3133,test_as_pattern_in_catch_handler_checks_the_entry_register_3133; thecatch-handler-varrow moved to the jq: widening TrackedVar's gate needs a bind-time path — provenance alone fabricates paths for equal-valued siblings #2042 accepting matrix; two jq: destructuring moves jq's path register — path(. as {a:$q} | $q) is ["a"] and path(. as {a:$q} | .a) raises #2649 CLI pins moved to accept assertionscargo llvm-cov --features cli,simd,regex,serde --workspace— every instrumented added line ineval.rscovered except onedebug_assert!pattern line/code-review 3144(standard effort): three fixes, one follow-up filed (jq: a fold UPDATE/EXTRACT body nested undertry/ifhas no register —del(foreach .a as $v (.; try ($v | .b); .))silently discards the write #3145); full verification, sweep and fuzz rerun after