Skip to content

collapse_up merges constraint-carrying Up instructions, dropping intermediate last-child checks #471

Description

@zharinov

Problem

collapse_up merges two directly-adjacent constraint-carrying Ups (e.g. two UpSkipTrivia(1)) into one UpSkipTrivia(2). That is wrong.

The VM's go_up runs the last-child check once, before ascending N levels. So a merged Up(2) only checks last-child-ness at the innermost level. The outer level's check is dropped.

Nested trailing anchors silently lose all but the innermost assertion:

  • soft . over-matches
  • strict .! mis-matches
# soft: Want no match — (object) is not the last named child of (array), `b` follows
# Got:  {"p":{"kind":"pair",...}}
cargo run -p plotnik -- run -l javascript -s '[{a:1}, b]' -q 'Q = (program (expression_statement (array (object (pair) @p .) .)))'

# strict: Want no match — (return_statement) is not the absolute-last child of the block, `}` follows
# Got:  {"id":{"kind":"identifier","text":"x",...}}
cargo run -p plotnik -- run -l javascript -s 'function f(){ return x }' -q 'Q = (program (function_declaration body: (statement_block (return_statement (identifier) @id .!) .!)))'

dump shows the two Up(1)s collapsed to ─•┘² (UpSkipTrivia(2)) / !─┘² (UpExact(2)).

Cause

compile/collapse_up.rs:151-160 same_up_mode allows merging the three constraint-carrying modes (UpSkipTrivia/UpSkipExtras/UpExact). The merge gate is collapse_up.rs:85.

crates/plotnik-vm/src/engine/cursor.rs:140-189 go_up runs the lastness check once (lines 142-181), then ascends levels times in the loop (184-188). Each constraint mode means "assert last-child at every level," but the merged instruction only checks the innermost one.

Only plain Up(n) (UpMode::Any, no per-level constraint) is safe to coalesce.

docs/tree-navigation.md:264 already documents this exact merge as unsound, but the code permits it and compile/collapse_up_tests.rs:80-187 assert it should happen.

Fix

Restrict collapse_up to plain Nav::Up only:

  • Keep the (Up, Up) arm in same_up_mode, reject the three constraint-carrying modes.
  • Update the module doc (collapse_up.rs:1-9) and docs/tree-navigation.md:240.
  • Invert or remove the three tests that codify the unsound merge: collapse_up_tests.rs:80-106 (skip_trivia), :109-135 (skip_extras), :160-187 (exact).

Acceptance

  • Both repros return no match.
  • conformance nested_trailing_soft_anchor_drops_outer_last_child_check and nested_trailing_strict_anchor_drops_outer_last_child_check flip to <no match>; nested_trailing_soft_anchor_matches_when_truly_last still matches.

Related

The fix must invert the three collapse_up_tests.rs cases that assert the unsound merge — they are part of the bug, not a regression guard.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions