Skip to content

Commit fd4f1dd

Browse files
authored
feat(ast): walk the AST export with one cursor; keep the indexed export (#1687)
* feat(ast): walk the AST export with one cursor; keep the indexed export `Ast::dump` enumerated children with `Node::child(i)` and `Node::field_name_for_child(i)`, commented as O(1). Both scan the parent's child array to the index — O(log n) under a balanced `repeat()`, O(index) under a flat node — so exporting a recovery `ERROR` was quadratic in its width: 1.75 s against 1.2 ms over 28 000 children. The default now walks one `Cursor`, which steps between siblings in amortised O(1) and reports a node's grammar field from its position. The indexed walk stays as `Ast::dump_indexed` / `AnyParser::dump_indexed` / `dump_indexed_inner`, with `dump_cursor` siblings naming the default explicitly; `Cursor` gains `goto_parent` and `field_name`. Both builders share one `Frame` / `Export` so the alterator, comment filter and ancestor chain cannot drift between them. Two observation counters make the dispatch testable, since no assertion on a dump can tell the builders apart. A cross-language parity suite compares the two strategies iteratively on every enabled language's fixture and on the shapes those fixtures lack; a Tcl fixture pins the ancestor chain's push half in a default build, which `Ancestors::checked` cannot see. Three `dump/*` scaling probes price the change: routing the default back to the indexed builder takes `dump/perl-error-tail-calls` from 1.02 to 1.98 while its width control holds. Fixes #1684 * refactor(ast): tighten the #1684 export docs and tests after review - Export::finish returns ControlFlow instead of a bespoke enum. - Test helpers take the comment flag and thread the strategy name into assertion messages; the comment-only test runs both strategies on both legs; one shared iterative kind counter replaces inline walks. - Cost prose no longer calls every recovery ERROR flat (only the EOF-recovery root is), no longer attributes the 1.75 s / 1.2 ms enumeration micro-benchmark to the export, and states the walk as O(n log n) on a balanced list rather than linear. - Export doc says which half of the chain bookkeeping the default-build debug_assert! sees; the dump/tcl-nested-eval probe is now measured against the Ancestors::unknown() swap it prices (0.75 -> 2.00). - Memory paragraph reports Vec capacities as capacities; the recovery probe list points at RECOVERY_PROBES instead of a hand count. - Cursor::field_name doc drops the false anonymous-token claim and the test-path pointer; the grammar-bump re-check lives in the parity module doc. Bench node counters are private. Refs #1684 * fix(ast): correct the flat-ERROR claim and pin an inherited field in #1684's export tests - The flat node is an ERROR at the *root* — a parse that failed at the top level — not one from a file that ends mid-parse; a nested ERROR is balanced. Four doc sites corrected (lesson 84; measured by the review). - "Never quadratic" is scoped to child enumeration: Tcl's argument_slots_are_readable is O(width) per braced word on an erroring line, pre-existing on both strategies and on main, now #1686. - New anchored test: Python's comparison_operator reaches its operator tokens through an .inherited field-map entry, asserted by name on both strategies; the parity sweep named no inherited field before. - The Cursor::reset leg now pins that the cursor moved before asserting the start node hides its field (a no-op reset passed it). - Root dispatch test seeds through Ast::dump so a rerouted dump_cursor is reported by its own row; the metadata test names the explicit strategy before the default; # Cost heading on both Node accessors; the book's stale pub(crate) paragraph about Node replaced with the STABILITY.md rule; benchmarking doc states the measured widths and the count pass's measured share. Refs #1684, #1686 * refactor(ast): route the inherited-field test through the shared strategy helpers Refs #1684 * test(ast): assert exact child shapes instead of failure-only format args Codecov flagged six lines of the #1684 patch uncovered. All six were the Debug-rendering arguments of two assert! messages, which only run when the assertion fails. Both now compare the whole child list through the kinds_and_fields helper, which is stronger — the `=` test used to accept any extra child — and leaves nothing in the file that only a failure executes. Refs #1684
1 parent 4dc9ece commit fd4f1dd

13 files changed

Lines changed: 1500 additions & 212 deletions

File tree

‎.bca-baseline.toml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,6 @@ qualified = "Span::new"
187187
metric = "nargs"
188188
value = 6.0
189189

190-
[[entry]]
191-
path = "big-code-analysis-ast/src/ast.rs"
192-
qualified = "build"
193-
metric = "halstead.effort"
194-
value = 92648.17221763342
195-
196190
[[entry]]
197191
path = "big-code-analysis-ast/src/c_macro.rs"
198192
qualified = "replace"

‎CHANGELOG.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,30 @@ for historical reference.
2424

2525
## [Unreleased]
2626

27+
### Added
28+
29+
- `Ast::dump_cursor` and `Ast::dump_indexed` name the AST export's two
30+
strategies explicitly: one cursor walk, or enumeration through
31+
`Node::child` / `Node::field_name_for_child`. Both produce the same
32+
tree. `AnyParser` and the parser-generic `dump_cursor_inner` /
33+
`dump_indexed_inner` in `big-code-analysis-ast` carry the same pair,
34+
and the `Cursor` wrapper there gains `goto_parent` and `field_name` —
35+
all three in a crate `STABILITY.md` leaves outside the shape
36+
contract. The `Ast` methods are additive and recorded in
37+
`STABILITY.md` under `Ast`. (#1684)
38+
2739
### Changed
2840

41+
- `Ast::dump` (and so the `/ast` endpoint and the Python `Ast.dump`;
42+
`bca dump` renders the raw tree through its own cursor walk and is
43+
unaffected) walks the tree with one cursor instead of indexing each
44+
child. The output is unchanged. The indexed lookups scan the parent's
45+
child array, so the old walk was quadratic in the width of a flat
46+
`ERROR` — one whose skipped tokens recovery left as direct children:
47+
enumerating 28 000 such children by index measured 1.75 s against a
48+
cursor's 1.2 ms. `Node::child` and `Node::field_name_for_child` now
49+
document that cost. (#1684)
50+
2951
- **(breaking), deferred to the next major (3.0):** the MI of a space
3052
with no code will serialize as `null` (an omitted key in TOML)
3153
instead of `0.0`. Throughout `2.x` the serialized value stays `0.0`;

‎STABILITY.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,13 @@ reach the raw tree-sitter surface.
730730
pin in the same value-not-stable sense as the `tree_sitter`
731731
re-export. The rest of `Ast`'s API surface (`parse`,
732732
`from_tree_sitter`, `metrics`, `ops`, `strip_comments`, `functions`,
733-
`dump`, `count`, `find`, `root_node`, `suppressions`, `language`,
734-
`source`, `name`) is shape-stable. `Ast::root_node` / `Ast::find`
733+
`dump`, `dump_cursor`, `dump_indexed`, `count`, `find`, `root_node`,
734+
`suppressions`, `language`, `source`, `name`) is shape-stable.
735+
`dump_cursor` and `dump_indexed` (#1684) name the two export
736+
strategies explicitly — one cursor walk, or `Node::child`-indexed
737+
enumeration — and both produce the same tree as `dump`, which runs
738+
the cursor walk; the indexed strategy is a supported choice, not a
739+
test-only reference. `Ast::root_node` / `Ast::find`
735740
return the public `Node` wrapper (#728), whose own `preorder()` /
736741
`descendants_by_kind()` traversal helpers and `as_tree_sitter()` escape
737742
hatch are shape-stable; the raw node *kinds* and `kind_id`s it surfaces

0 commit comments

Comments
 (0)