From 2dd539b7407a5070da76061286dea606f0f78ec5 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Sat, 18 Jul 2026 15:01:50 +0200 Subject: [PATCH 1/4] Add Rust/Go AST parity gate and fix Java precpred loop-enter Parse-bench can now dump Rule/Term/Err trees from rust-antlr and go-antlr and require byte-identical, error-free ASTs before timing (--ast-check). Fix left-recursive loop-enter so multi-token operator prefixes that shadow a lower-precedence single-token op (Java > vs >>) defer to StarLoopEntry adaptive predict instead of forcing enter and failing precpred. CI runs AST parity on kotlin, java, and trino. C# Mono fixture divergence is tracked in #109. --- .github/workflows/parse-bench.yml | 9 + README.md | 12 ++ src/parser.rs | 294 ++++++++++++++++++++++++--- tools/parse-bench/README.md | 28 ++- tools/parse-bench/run.py | 324 +++++++++++++++++++++++++++++- tools/parse-bench/test_run.py | 15 ++ 6 files changed, 645 insertions(+), 37 deletions(-) diff --git a/.github/workflows/parse-bench.yml b/.github/workflows/parse-bench.yml index 02017bd..96d6622 100644 --- a/.github/workflows/parse-bench.yml +++ b/.github/workflows/parse-bench.yml @@ -69,6 +69,15 @@ jobs: git -C /tmp/antlr-cleanroom/grammars-v4 fetch --depth 1 origin 284602b3f23ca54dc30778204ab7ae9e969145e9 git -C /tmp/antlr-cleanroom/grammars-v4 checkout FETCH_HEAD + - name: Rust/Go AST parity (kotlin + java + trino) + run: | + python head/tools/parse-bench/run.py \ + --languages kotlin,java,trino \ + --runtimes rust-antlr,go-antlr \ + --ast-check \ + --quick \ + --work-dir head/target/parse-bench-ast + - name: Run base benchmark if: github.event_name == 'pull_request' run: | diff --git a/README.md b/README.md index 49f9e71..42bdcf3 100644 --- a/README.md +++ b/README.md @@ -450,6 +450,18 @@ python3 tools/parse-bench/run.py \ --markdown target/parse-bench/results.md ``` +Add `--ast-check` to require byte-identical Rust/Go parse trees (no error nodes) +before timing. Prefer that gate for fair comparisons; some C# Mono fixtures still +diverge today (see `tools/parse-bench/README.md`). For a clean smoke: + +```bash +python3 tools/parse-bench/run.py \ + --languages kotlin,trino \ + --runtimes rust-antlr,go-antlr \ + --ast-check \ + --quick +``` + The report prints `min`/`avg` parse time and a ratio against `rust-antlr` for every fixture. Use `--quick` for a 3-iteration/1-warmup smoke run, or adjust `--iters`/`--warmups` for longer, lower-variance runs; add diff --git a/src/parser.rs b/src/parser.rs index b243315..727d2eb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2336,7 +2336,15 @@ type DecisionLookaheadCache = FxHashMap>; #[derive(Debug, Default)] struct LeftRecursiveOperatorLookahead { - unconditional_symbols: TokenBitSet, + /// Operator alts whose token-prefix is fully matched by this one symbol + /// (then only epsilons/actions remain before the recursive RHS call). + /// Safe for one-token loop-enter fast path. + single_token_symbols: TokenBitSet, + /// Operator alts that start with this symbol but still require more tokens + /// (e.g. Java `>>` / `>>>` when only shift is precedence-viable). Must not + /// force enter from one-token lookahead — StarLoopEntry adaptive predict + /// has to weigh the exit alt as well. + multi_token_prefix_symbols: TokenBitSet, predicate_dependent_symbols: TokenBitSet, } @@ -2857,7 +2865,10 @@ enum OperatorSymbolReachability { #[default] None, PredicateDependent, - Unconditional, + /// Starts an operator but more tokens are required before the RHS call. + UnconditionalMultiToken, + /// One token completes the operator token-prefix. + UnconditionalSingleToken, } #[derive(Clone, Copy)] @@ -3001,6 +3012,53 @@ fn state_is_nullable_with_precedence_cached( nullable } +/// After the operator's first token is matched, is the operator token-prefix +/// finished without needing another token? That includes binary ops (next is +/// the recursive RHS rule call), postfix ops (nullable to loop-back / stop), +/// but not multi-token ops like Java `>>` (another `>` is required). +fn state_operator_token_prefix_complete( + atn: &Atn, + state_number: usize, + precedence: i32, + visited: &mut BTreeSet, +) -> bool { + if !visited.insert(state_number) { + return false; + } + let Some(state) = atn.state(state_number) else { + return false; + }; + match state.kind() { + AtnStateKind::RuleStop + | AtnStateKind::StarLoopBack + | AtnStateKind::StarLoopEntry + | AtnStateKind::PlusLoopBack + | AtnStateKind::LoopEnd => return true, + _ => {} + } + state.transitions().iter().any(|transition| match &transition.data() { + Transition::Rule { .. } => true, + Transition::Epsilon { target } | Transition::Action { target, .. } => { + state_operator_token_prefix_complete(atn, *target, precedence, visited) + } + Transition::Precedence { + target, + precedence: transition_precedence, + } if *transition_precedence >= precedence => { + state_operator_token_prefix_complete(atn, *target, precedence, visited) + } + Transition::Predicate { target, .. } => { + state_operator_token_prefix_complete(atn, *target, precedence, visited) + } + Transition::Atom { .. } + | Transition::Range { .. } + | Transition::Set { .. } + | Transition::NotSet { .. } + | Transition::Wildcard { .. } + | Transition::Precedence { .. } => false, + }) +} + fn state_can_reach_symbol_with_precedence( atn: &Atn, state_number: usize, @@ -3021,11 +3079,24 @@ fn state_can_reach_symbol_with_precedence( let mut reachability = OperatorSymbolReachability::None; for transition in &state.transitions() { if transition.matches(request.symbol, 1, atn.max_token_type()) { + let matched = if state_operator_token_prefix_complete( + atn, + transition.target(), + request.precedence, + &mut BTreeSet::new(), + ) { + OperatorSymbolReachability::UnconditionalSingleToken + } else { + OperatorSymbolReachability::UnconditionalMultiToken + }; if request.predicate_dependent { - reachability = OperatorSymbolReachability::PredicateDependent; - continue; + reachability = reachability.max(OperatorSymbolReachability::PredicateDependent); + } else if matched == OperatorSymbolReachability::UnconditionalSingleToken { + return matched; + } else { + reachability = reachability.max(matched); } - return OperatorSymbolReachability::Unconditional; + continue; } let transition_reachability = match &transition.data() { Transition::Rule { @@ -3044,7 +3115,7 @@ fn state_can_reach_symbol_with_precedence( nullable_ctx, visited, ); - if result == OperatorSymbolReachability::Unconditional { + if result == OperatorSymbolReachability::UnconditionalSingleToken { return result; } let Some(child_stop) = atn.rule_to_stop_state().get(*rule_index) else { @@ -3110,7 +3181,7 @@ fn state_can_reach_symbol_with_precedence( | Transition::NotSet { .. } | Transition::Wildcard { .. } => OperatorSymbolReachability::None, }; - if transition_reachability == OperatorSymbolReachability::Unconditional { + if transition_reachability == OperatorSymbolReachability::UnconditionalSingleToken { return transition_reachability; } reachability = reachability.max(transition_reachability); @@ -3152,8 +3223,11 @@ fn left_recursive_operator_lookahead( &mut nullable_ctx, &mut BTreeSet::new(), ) { - OperatorSymbolReachability::Unconditional => { - lookahead.unconditional_symbols.insert(symbol); + OperatorSymbolReachability::UnconditionalSingleToken => { + lookahead.single_token_symbols.insert(symbol); + } + OperatorSymbolReachability::UnconditionalMultiToken => { + lookahead.multi_token_prefix_symbols.insert(symbol); } OperatorSymbolReachability::PredicateDependent => { lookahead.predicate_dependent_symbols.insert(symbol); @@ -5269,8 +5343,18 @@ where /// Predicts a generated left-recursive loop from one-token lookahead. /// /// `Some(true)` enters the operator alternative, `Some(false)` exits, and - /// `None` means caller overlap or an unresolved semantic predicate requires - /// full-context adaptive prediction. + /// `None` means caller overlap, a dangerous multi-token prefix, or an + /// unresolved semantic predicate requires full StarLoopEntry adaptive + /// prediction (which includes the exit alt and precedence filtering). + /// + /// Single-token operators (`+`, `*`, relational `>` at low precedence) and + /// multi-token ops that do not shadow a lower-precedence single-token use of + /// the same symbol (e.g. `.`) keep the one-token enter fast path. + /// + /// Multi-token prefixes that **do** shadow a lower-precedence single-token + /// op (Java `>>`/`>>>` vs `>` when only shift is precedence-viable) must not + /// force enter — otherwise the operator decision picks relational, + /// `precpred` fails, and a legal outer expression dies. pub fn left_recursive_loop_enter_prediction( &mut self, atn: &Atn, @@ -5281,29 +5365,33 @@ where if symbol == TOKEN_EOF { return Some(false); } - let operator_lookahead = with_shared_atn_caches(atn, |cache| { - let key = (state_number, precedence); - if let Some(cached) = cache.left_recursive_operator_lookahead.get(&key) { - return Rc::clone(cached); - } - let lookahead = Rc::new(left_recursive_operator_lookahead( - atn, - state_number, - precedence, - )); - cache - .left_recursive_operator_lookahead - .insert(key, Rc::clone(&lookahead)); - lookahead - }); - if !operator_lookahead.unconditional_symbols.contains(symbol) { - if operator_lookahead - .predicate_dependent_symbols - .contains(symbol) - { + let operator_lookahead = Self::cached_left_recursive_operator_lookahead( + atn, + state_number, + precedence, + ); + let can_single = operator_lookahead.single_token_symbols.contains(symbol); + let can_multi = operator_lookahead + .multi_token_prefix_symbols + .contains(symbol); + let can_predicate = operator_lookahead + .predicate_dependent_symbols + .contains(symbol); + if !can_single && !can_multi && !can_predicate { + return Some(false); + } + if can_predicate && !can_single && !can_multi { + return None; + } + // Multi-token-only at this precedence, but the same symbol is a + // single-token operator at precedence 0: defer so exit can win when the + // multi-token sequence does not actually match (e.g. `>` vs `>>`). + if !can_single && can_multi && precedence > 0 { + let baseline = + Self::cached_left_recursive_operator_lookahead(atn, state_number, 0); + if baseline.single_token_symbols.contains(symbol) { return None; } - return Some(false); } let atn_key = SharedAtnCacheKey::for_atn(atn); let cached_overlap = self @@ -5345,6 +5433,28 @@ where Some(true) } + fn cached_left_recursive_operator_lookahead( + atn: &Atn, + state_number: usize, + precedence: i32, + ) -> Rc { + with_shared_atn_caches(atn, |cache| { + let key = (state_number, precedence); + if let Some(cached) = cache.left_recursive_operator_lookahead.get(&key) { + return Rc::clone(cached); + } + let lookahead = Rc::new(left_recursive_operator_lookahead( + atn, + state_number, + precedence, + )); + cache + .left_recursive_operator_lookahead + .insert(key, Rc::clone(&lookahead)); + lookahead + }) + } + /// Checks whether a generated left-recursive loop can unambiguously enter /// its operator alternative from one-token lookahead. pub fn left_recursive_loop_enter_matches( @@ -11927,6 +12037,92 @@ mod tests { parser } + fn left_recursive_loop_with_shared_gt_prefix_atn() -> Atn { + // StarLoopEntry with two operator alts that share leading token 1 (`>`): + // prec 2: token 1, token 1 (shift `>>`) + // prec 1: token 1 (relational `>`) + let mut atn = ParserAtnBuilder::new(1); + for (state, kind, rule) in [ + (0, AtnStateKind::RuleStart, 0), + (1, AtnStateKind::StarLoopEntry, 0), + (2, AtnStateKind::Basic, 0), // ops hub + (3, AtnStateKind::Basic, 0), // shift prec + (4, AtnStateKind::Basic, 0), // shift first > + (5, AtnStateKind::Basic, 0), // shift second > + (6, AtnStateKind::Basic, 0), // rel prec + (7, AtnStateKind::Basic, 0), // rel > + (8, AtnStateKind::LoopEnd, 0), + (9, AtnStateKind::RuleStop, 0), + ] { + assert_eq!( + atn.add_state(kind, Some(rule)).expect("state").index(), + state + ); + if state == 0 { + atn.set_left_recursive_rule(state) + .expect("left-recursive rule start"); + } else if state == 1 { + atn.set_precedence_rule_decision(state) + .expect("precedence decision"); + } + } + atn.set_rule_to_start_state(vec![0]) + .expect("rule start states"); + atn.set_rule_to_stop_state(vec![9]) + .expect("rule stop states"); + atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 }) + .expect("ops"); + atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 8 }) + .expect("exit"); + atn.add_transition(2, ParserTransitionSpec::Epsilon { target: 3 }) + .expect("to shift"); + atn.add_transition(2, ParserTransitionSpec::Epsilon { target: 6 }) + .expect("to rel"); + atn.add_transition( + 3, + ParserTransitionSpec::Precedence { + target: 4, + precedence: 2, + }, + ) + .expect("shift prec"); + atn.add_transition( + 4, + ParserTransitionSpec::Atom { + target: 5, + label: 1, + }, + ) + .expect("shift first >"); + atn.add_transition( + 5, + ParserTransitionSpec::Atom { + target: 1, + label: 1, + }, + ) + .expect("shift second >"); + atn.add_transition( + 6, + ParserTransitionSpec::Precedence { + target: 7, + precedence: 1, + }, + ) + .expect("rel prec"); + atn.add_transition( + 7, + ParserTransitionSpec::Atom { + target: 1, + label: 1, + }, + ) + .expect("rel >"); + atn.add_transition(8, ParserTransitionSpec::Epsilon { target: 9 }) + .expect("loop end"); + finish_atn(atn) + } + fn left_recursive_loop_with_nullable_operator_prefix_atn() -> Atn { let mut atn = ParserAtnBuilder::new(2); for (state, kind, rule) in [ @@ -12391,6 +12587,40 @@ mod tests { ); } + #[test] + fn left_recursive_loop_defers_multi_token_prefix_that_shadows_lower_single_token() { + // Models Java `>` (relational, prec 1, one token) vs `>>` (shift, prec 2, + // two tokens). At prec 2 only shift is viable; one-token lookahead on `>` + // must defer so StarLoopEntry adaptive predict can exit when the second + // `>` is absent (as in `a < b > c`). + let atn = left_recursive_loop_with_shared_gt_prefix_atn(); + let mut parser = mini_parser(vec![ + TestToken::new(1).with_text(">"), + TestToken::new(2).with_text("id"), + TestToken::eof("parser-test", 1, 1, 1), + ]); + parser.rule_context_stack = vec![RuleContextFrame { + rule_index: 0, + invoking_state: -1, + }]; + + assert_eq!( + parser.left_recursive_loop_enter_prediction(&atn, 1, 0), + Some(true), + "at low precedence relational `>` is a single-token operator" + ); + assert_eq!( + parser.left_recursive_loop_enter_prediction(&atn, 1, 1), + Some(true), + "relational remains single-token at its own precedence" + ); + assert_eq!( + parser.left_recursive_loop_enter_prediction(&atn, 1, 2), + None, + "at shift precedence, bare `>` must not force enter" + ); + } + #[test] fn left_recursive_loop_defers_predicate_guarded_operator() { let atn = left_recursive_loop_with_predicate_guarded_operator_atn(); diff --git a/tools/parse-bench/README.md b/tools/parse-bench/README.md index a153229..eff08b9 100644 --- a/tools/parse-bench/README.md +++ b/tools/parse-bench/README.md @@ -40,12 +40,13 @@ Quick local smoke: python3 tools/parse-bench/run.py --quick ``` -SQL-only Rust vs Go smoke: +SQL-only Rust vs Go smoke (with AST parity gate): ```bash python3 tools/parse-bench/run.py \ --languages trino \ --runtimes rust-antlr,go-antlr \ + --ast-check \ --quick ``` @@ -73,6 +74,31 @@ Use `--rust-generated-only` for Adaptive LL delivery evidence so the Rust generator fails if any parser rule lacks a generated body and the Rust runner fails if a generated parser path falls back to the interpreter. +### Rust vs Go AST parity + +Pass `--ast-check` with `--phase parse` and both `rust-antlr` and `go-antlr` +selected. Before timing, the harness dumps each fixture's parse tree from both +runners (same `Rule`/`Term`/`Err` format as the Kotlin parity dumper) and +requires: + +- byte-identical dumps, and +- no error nodes on either side (Rust also rejects a non-zero syntax-error count). + +Dumps land under `/ast-dumps//`. + +Kotlin, Java, and Trino fixtures currently pass this gate. Some C# Mono +fixtures diverge (preprocessor/`#if` handling differences between the Go support +base and the Rust `.interp` path) — the check fails those intentionally so +timings are not compared on unequal trees. + +```bash +python3 tools/parse-bench/run.py \ + --languages kotlin,trino \ + --runtimes rust-antlr,go-antlr \ + --ast-check \ + --quick +``` + ### Lex-only measurements Use `--phase lex` to time generated Rust lexing and token buffering without diff --git a/tools/parse-bench/run.py b/tools/parse-bench/run.py index aa33933..94d3fb3 100755 --- a/tools/parse-bench/run.py +++ b/tools/parse-bench/run.py @@ -6,6 +6,7 @@ import argparse import dataclasses import datetime as dt +import difflib import json import os import re @@ -407,6 +408,10 @@ def write_rust_runner( for spec in specs ) lex_arms = "\n".join(f' "{spec.name}" => lex_{spec.name}(&src)?,' for spec in specs) + dump_arms = "\n".join( + f' "{spec.name}" => dump_tree_{spec.name}(&src, out)?,' + for spec in specs + ) stats_arms = "\n".join( f' "{spec.name}" => prediction_stats_{spec.name}(&src).map_err(|err| err.to_string())?,' for spec in specs @@ -418,11 +423,12 @@ def write_rust_runner( use std::env; use std::fs; use std::hint::black_box; +use std::io::{{self, Write}}; use std::path::PathBuf; use std::process::ExitCode; use std::time::Instant; -use antlr4_runtime::{{CommonTokenStream, InputStream}}; +use antlr4_runtime::{{CommonTokenStream, InputStream, Node, NodeKind, Parser}}; mod generated {{ #![allow(dead_code, unused_imports, unreachable_pub, unused_qualifications)] @@ -445,6 +451,7 @@ def write_rust_runner( let mut args = env::args().skip(1); let mut language: Option = None; let mut input: Option = None; + let mut dump_tree: Option = None; let mut phase = "parse".to_owned(); let mut iters = 1_usize; let mut warmups = 0_usize; @@ -452,6 +459,7 @@ def write_rust_runner( match arg.as_str() {{ "--language" => language = args.next(), "--input" => input = args.next().map(PathBuf::from), + "--dump-tree" => dump_tree = args.next().map(PathBuf::from), "--phase" => phase = args.next().ok_or("missing value for --phase")?, "--iters" => iters = parse_usize(args.next(), "--iters")?, "--warmups" => warmups = parse_usize(args.next(), "--warmups")?, @@ -468,6 +476,18 @@ def write_rust_runner( }} let src = fs::read_to_string(&input) .map_err(|err| format!("failed to read {{}}: {{err}}", input.display()))?; + if let Some(path) = dump_tree {{ + if phase != "parse" {{ + return Err("--dump-tree requires --phase parse".to_owned()); + }} + let file = fs::File::create(&path) + .map_err(|err| format!("failed to create {{}}: {{err}}", path.display()))?; + let mut out = io::BufWriter::new(file); + dump_tree_once(&language, &src, &mut out)?; + out.flush() + .map_err(|err| format!("failed to flush {{}}: {{err}}", path.display()))?; + return Ok(()); + }} let collect_stats = env::var_os("ANTLR_PERF_DUMP").is_some(); for _ in 0..warmups {{ @@ -518,6 +538,14 @@ def write_rust_runner( Ok(()) }} +fn dump_tree_once(language: &str, src: &str, out: &mut dyn Write) -> Result<(), String> {{ + match language {{ +{dump_arms} + other => return Err(format!("unsupported language: {{other}}")), + }} + Ok(()) +}} + fn run_once(phase: &str, language: &str, src: &str) -> Result<(), String> {{ match phase {{ "parse" => parse_once(language, src), @@ -526,6 +554,38 @@ def write_rust_runner( }} }} +fn dump_node>( + out: &mut dyn Write, + tree: Node<'_>, + rule_names: &[S], + depth: usize, +) -> io::Result<()> {{ + let pad = " ".repeat(depth); + match tree.kind() {{ + NodeKind::Rule => {{ + let rule = tree.as_rule().expect("rule node kind checked"); + let name = rule_names + .get(rule.rule_index()) + .map_or("", |name| name.as_ref()); + writeln!(out, "{{pad}}Rule({{name}}, children={{}})", rule.child_count())?; + for child in rule.children() {{ + dump_node(out, child, rule_names, depth + 1)?; + }} + }} + NodeKind::Terminal => writeln!( + out, + "{{pad}}Term({{:?}})", + tree.as_terminal().expect("terminal node kind checked").text() + )?, + NodeKind::Error => writeln!( + out, + "{{pad}}Err({{:?}})", + tree.as_error().expect("error node kind checked").text() + )?, + }} + Ok(()) +}} + fn prediction_stats_once( language: &str, src: &str, @@ -622,6 +682,28 @@ def rust_parse_function(spec: LanguageSpec) -> str: Ok(()) }} +fn dump_tree_{spec.name}(src: &str, out: &mut dyn Write) -> Result<(), String> {{ + let lexer = generated::{spec.rust_lexer_module}::{spec.rust_lexer_type}::new(InputStream::new(src)); + let tokens = CommonTokenStream::new(lexer); + let mut parser = generated::{spec.rust_parser_module}::{spec.rust_parser_type}::new(tokens); + let root = parser + .{spec.rust_entry}() + .map_err(|err| err.to_string())?; + let syntax_errors = parser.number_of_syntax_errors(); + if syntax_errors != 0 {{ + return Err(format!( + "rust-antlr {spec.name} parse produced {{syntax_errors}} syntax error(s)" + )); + }} + dump_node( + out, + parser.node(root), + generated::{spec.rust_parser_module}::rule_names(), + 0, + ) + .map_err(|err| err.to_string()) +}} + fn prediction_stats_{spec.name}( src: &str, ) -> Result<(antlr4_runtime::PredictionContextStats, antlr4_runtime::ParserDfaStats), antlr4_runtime::AntlrError> {{ @@ -814,14 +896,20 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: f' case "{spec.name}":\n parse{go_func_name(spec.name)}(src)' for spec in specs ) + dump_arms = "\n".join( + f' case "{spec.name}":\n return dumpTree{go_func_name(spec.name)}(src, out)' + for spec in specs + ) functions = "\n\n".join(go_parse_function(spec) for spec in specs) (runner / "main.go").write_text( f"""package main import ( "fmt" + "io" "os" "strconv" + "strings" "time" "github.com/antlr4-go/antlr/v4" @@ -840,6 +928,7 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: func runMain() error {{ language := "" input := "" + dumpTreePath := "" iters := 1 warmups := 0 for i := 1; i < len(os.Args); i++ {{ @@ -850,6 +939,9 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: case "--input": i++ input = requiredArg(i, "--input") + case "--dump-tree": + i++ + dumpTreePath = requiredArg(i, "--dump-tree") case "--iters": i++ iters = parsePositiveInt(requiredArg(i, "--iters"), "--iters") @@ -871,6 +963,14 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: return err }} src := string(bytes) + if dumpTreePath != "" {{ + file, err := os.Create(dumpTreePath) + if err != nil {{ + return err + }} + defer file.Close() + return dumpTreeOnce(language, src, file) + }} for i := 0; i < warmups; i++ {{ parseOnce(language, src) }} @@ -897,6 +997,88 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: }} }} +func dumpTreeOnce(language string, src string, out io.Writer) error {{ + switch language {{ +{dump_arms} + default: + return fmt.Errorf("unsupported language: %s", language) + }} +}} + +func dumpNode(out io.Writer, tree antlr.Tree, ruleNames []string, depth int) error {{ + pad := strings.Repeat(" ", depth) + switch node := tree.(type) {{ + case antlr.ErrorNode: + _, err := fmt.Fprintf(out, "%sErr(%s)\\n", pad, rustDebugStr(node.GetText())) + return err + case antlr.TerminalNode: + _, err := fmt.Fprintf(out, "%sTerm(%s)\\n", pad, rustDebugStr(node.GetText())) + return err + case antlr.RuleNode: + ruleIndex := node.GetRuleContext().GetRuleIndex() + name := "" + if ruleIndex >= 0 && ruleIndex < len(ruleNames) {{ + name = ruleNames[ruleIndex] + }} + children := node.GetChildren() + if _, err := fmt.Fprintf(out, "%sRule(%s, children=%d)\\n", pad, name, len(children)); err != nil {{ + return err + }} + for _, child := range children {{ + if err := dumpNode(out, child, ruleNames, depth+1); err != nil {{ + return err + }} + }} + return nil + default: + return fmt.Errorf("unsupported tree node type %T", tree) + }} +}} + +func treeHasErrorNode(tree antlr.Tree) bool {{ + if _, ok := tree.(antlr.ErrorNode); ok {{ + return true + }} + if _, ok := tree.(antlr.TerminalNode); ok {{ + return false + }} + for _, child := range tree.GetChildren() {{ + if treeHasErrorNode(child) {{ + return true + }} + }} + return false +}} + +func rustDebugStr(text string) string {{ + var b strings.Builder + b.WriteByte('"') + for _, r := range text {{ + switch r {{ + case '\\\\': + b.WriteString(`\\\\`) + case '"': + b.WriteString(`\\"`) + case '\\n': + b.WriteString(`\\n`) + case '\\r': + b.WriteString(`\\r`) + case '\\t': + b.WriteString(`\\t`) + case 0: + b.WriteString(`\\0`) + default: + if r < 0x20 || r == 0x7f {{ + fmt.Fprintf(&b, `\\u{{%x}}`, r) + }} else {{ + b.WriteRune(r) + }} + }} + }} + b.WriteByte('"') + return b.String() +}} + func requiredArg(index int, flag string) string {{ if index >= len(os.Args) {{ panic("missing value for " + flag) @@ -935,13 +1117,27 @@ def go_package_name(spec: LanguageSpec) -> str: def go_parse_function(spec: LanguageSpec) -> str: - return f"""func parse{go_func_name(spec.name)}(src string) {{ + pkg = go_package_name(spec) + func_name = go_func_name(spec.name) + return f"""func parse{func_name}(src string) {{ input := antlr.NewInputStream(src) - lexer := {go_package_name(spec)}.New{spec.lexer_name}(input) + lexer := {pkg}.New{spec.lexer_name}(input) tokens := antlr.NewCommonTokenStream(lexer, antlr.TokenDefaultChannel) - p := {go_package_name(spec)}.New{spec.parser_name}(tokens) + p := {pkg}.New{spec.parser_name}(tokens) tree := p.{spec.go_entry}() sink = tree +}} + +func dumpTree{func_name}(src string, out io.Writer) error {{ + input := antlr.NewInputStream(src) + lexer := {pkg}.New{spec.lexer_name}(input) + tokens := antlr.NewCommonTokenStream(lexer, antlr.TokenDefaultChannel) + p := {pkg}.New{spec.parser_name}(tokens) + tree := p.{spec.go_entry}() + if treeHasErrorNode(tree) {{ + return fmt.Errorf("go-antlr {spec.name} parse produced error nodes") + }} + return dumpNode(out, tree, p.GetRuleNames(), 0) }}""" @@ -1133,6 +1329,115 @@ def measure_fixture( ) +def dump_tree_has_error_nodes(dump: str) -> bool: + return any(line.lstrip().startswith("Err(") for line in dump.splitlines()) + + +def format_tree_diff(rust_dump: str, go_dump: str, *, max_lines: int = 80) -> str: + diff = list( + difflib.unified_diff( + rust_dump.splitlines(), + go_dump.splitlines(), + fromfile="rust-antlr", + tofile="go-antlr", + lineterm="", + ) + ) + if len(diff) > max_lines: + omitted = len(diff) - max_lines + diff = diff[:max_lines] + [f"... ({omitted} more diff lines omitted)"] + return "\n".join(diff) + + +def dump_fixture_tree( + runtime: str, + runner: Path, + fixture: Fixture, + output: Path, + args: argparse.Namespace, +) -> str: + env = None + if runtime == "rust-antlr" and args.rust_generated_only: + env = os.environ.copy() + env["ANTLR4_RUST_GENERATED_ONLY"] = "1" + cmd = [ + str(runner), + "--language", + fixture.language, + "--input", + str(fixture.abs_path), + "--dump-tree", + str(output), + ] + if runtime == "rust-antlr": + cmd.extend(["--phase", "parse"]) + run(cmd, env=env, quiet=True) + return output.read_text() + + +def validate_rust_go_ast_parity( + fixtures: list[Fixture], + runners: dict[str, Path], + args: argparse.Namespace, +) -> None: + if not args.ast_check: + return + if args.phase != "parse": + raise SystemExit("--ast-check requires --phase parse") + if "rust-antlr" not in runners or "go-antlr" not in runners: + raise SystemExit( + "--ast-check requires --runtimes to include both rust-antlr and go-antlr" + ) + + dump_root = args.work_dir / "ast-dumps" + dump_root.mkdir(parents=True, exist_ok=True) + failures: list[str] = [] + + for fixture in fixtures: + fixture_dir = dump_root / fixture.language + fixture_dir.mkdir(parents=True, exist_ok=True) + rust_path = fixture_dir / f"{fixture.name}.rust.txt" + go_path = fixture_dir / f"{fixture.name}.go.txt" + try: + rust_dump = dump_fixture_tree( + "rust-antlr", runners["rust-antlr"], fixture, rust_path, args + ) + go_dump = dump_fixture_tree( + "go-antlr", runners["go-antlr"], fixture, go_path, args + ) + except subprocess.CalledProcessError as err: + detail = (err.stderr or err.stdout or "").strip() + failures.append( + f"{fixture.language}/{fixture.name}: tree dump failed" + + (f": {detail}" if detail else "") + ) + continue + + if dump_tree_has_error_nodes(rust_dump): + failures.append( + f"{fixture.language}/{fixture.name}: rust-antlr AST contains error nodes " + f"(dump: {rust_path})" + ) + if dump_tree_has_error_nodes(go_dump): + failures.append( + f"{fixture.language}/{fixture.name}: go-antlr AST contains error nodes " + f"(dump: {go_path})" + ) + if rust_dump != go_dump: + failures.append( + f"{fixture.language}/{fixture.name}: rust-antlr and go-antlr ASTs differ\n" + f"{format_tree_diff(rust_dump, go_dump)}" + ) + else: + print(f"{fixture.language}/{fixture.name}: rust/go AST parity ok") + + if failures: + print("rust/go AST parity check failed:", file=sys.stderr) + for failure in failures: + print(f" {failure}", file=sys.stderr) + raise SystemExit(1) + + def print_table(results: list[Measurement]) -> None: rust_by_fixture = { (result.language, result.fixture): result.avg_ns @@ -1292,6 +1597,16 @@ def parse_args() -> argparse.Namespace: "of falling back to the interpreter." ), ) + parser.add_argument( + "--ast-check", + action="store_true", + help=( + "Before timing, dump each fixture's parse tree from rust-antlr and " + "go-antlr (Rule/Term/Err format) and require byte-identical dumps " + "with no error nodes. Requires both runtimes for --phase parse; " + "use this to gate fair throughput comparisons." + ), + ) parser.add_argument("--json", type=Path, help="Write machine-readable results.") parser.add_argument("--markdown", type=Path, help="Write a Markdown table report.") return parser.parse_args() @@ -1331,6 +1646,7 @@ def main() -> int: ensure_python_dependencies(args.python, runtimes) runners = prepare_work(args, specs, runtimes) + validate_rust_go_ast_parity(fixtures, runners, args) results: list[Measurement] = [] for fixture in fixtures: diff --git a/tools/parse-bench/test_run.py b/tools/parse-bench/test_run.py index 64c26ef..f39b041 100644 --- a/tools/parse-bench/test_run.py +++ b/tools/parse-bench/test_run.py @@ -13,6 +13,21 @@ SPEC.loader.exec_module(RUN) +class DumpTreeHelpersTests(unittest.TestCase): + def test_detects_error_nodes(self) -> None: + clean = 'Rule(root, children=1)\n Term("x")\n' + dirty = 'Rule(root, children=1)\n Err("!")\n' + self.assertFalse(RUN.dump_tree_has_error_nodes(clean)) + self.assertTrue(RUN.dump_tree_has_error_nodes(dirty)) + + def test_format_tree_diff_mentions_both_runtimes(self) -> None: + diff = RUN.format_tree_diff("Rule(a, children=0)\n", "Rule(b, children=0)\n") + self.assertIn("rust-antlr", diff) + self.assertIn("go-antlr", diff) + self.assertIn("-Rule(a, children=0)", diff) + self.assertIn("+Rule(b, children=0)", diff) + + class ClearWorkDirTests(unittest.TestCase): def test_rejects_runtime_root_and_ancestor(self) -> None: with tempfile.TemporaryDirectory() as temp: From 8f09dfcb40998d861461f05b9af402c19185fc95 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Sat, 18 Jul 2026 15:11:48 +0200 Subject: [PATCH 2/4] Fix left-recursion review and Clippy findings --- src/parser.rs | 95 ++++++++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 50 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 727d2eb..5c495dc 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2339,13 +2339,13 @@ struct LeftRecursiveOperatorLookahead { /// Operator alts whose token-prefix is fully matched by this one symbol /// (then only epsilons/actions remain before the recursive RHS call). /// Safe for one-token loop-enter fast path. - single_token_symbols: TokenBitSet, + single_token: TokenBitSet, /// Operator alts that start with this symbol but still require more tokens /// (e.g. Java `>>` / `>>>` when only shift is precedence-viable). Must not - /// force enter from one-token lookahead — StarLoopEntry adaptive predict + /// force enter from one-token lookahead — `StarLoopEntry` adaptive predict /// has to weigh the exit alt as well. - multi_token_prefix_symbols: TokenBitSet, - predicate_dependent_symbols: TokenBitSet, + multi_token_prefix: TokenBitSet, + predicate_dependent: TokenBitSet, } #[derive(Default)] @@ -3036,27 +3036,30 @@ fn state_operator_token_prefix_complete( | AtnStateKind::LoopEnd => return true, _ => {} } - state.transitions().iter().any(|transition| match &transition.data() { - Transition::Rule { .. } => true, - Transition::Epsilon { target } | Transition::Action { target, .. } => { - state_operator_token_prefix_complete(atn, *target, precedence, visited) - } - Transition::Precedence { - target, - precedence: transition_precedence, - } if *transition_precedence >= precedence => { - state_operator_token_prefix_complete(atn, *target, precedence, visited) - } - Transition::Predicate { target, .. } => { - state_operator_token_prefix_complete(atn, *target, precedence, visited) - } - Transition::Atom { .. } - | Transition::Range { .. } - | Transition::Set { .. } - | Transition::NotSet { .. } - | Transition::Wildcard { .. } - | Transition::Precedence { .. } => false, - }) + state + .transitions() + .iter() + .any(|transition| match &transition.data() { + Transition::Rule { .. } => true, + Transition::Epsilon { target } | Transition::Action { target, .. } => { + state_operator_token_prefix_complete(atn, *target, precedence, visited) + } + Transition::Precedence { + target, + precedence: transition_precedence, + } => { + *transition_precedence >= precedence + && state_operator_token_prefix_complete(atn, *target, precedence, visited) + } + Transition::Predicate { target, .. } => { + state_operator_token_prefix_complete(atn, *target, precedence, visited) + } + Transition::Atom { .. } + | Transition::Range { .. } + | Transition::Set { .. } + | Transition::NotSet { .. } + | Transition::Wildcard { .. } => false, + }) } fn state_can_reach_symbol_with_precedence( @@ -3224,13 +3227,13 @@ fn left_recursive_operator_lookahead( &mut BTreeSet::new(), ) { OperatorSymbolReachability::UnconditionalSingleToken => { - lookahead.single_token_symbols.insert(symbol); + lookahead.single_token.insert(symbol); } OperatorSymbolReachability::UnconditionalMultiToken => { - lookahead.multi_token_prefix_symbols.insert(symbol); + lookahead.multi_token_prefix.insert(symbol); } OperatorSymbolReachability::PredicateDependent => { - lookahead.predicate_dependent_symbols.insert(symbol); + lookahead.predicate_dependent.insert(symbol); } OperatorSymbolReachability::None => {} } @@ -5344,7 +5347,7 @@ where /// /// `Some(true)` enters the operator alternative, `Some(false)` exits, and /// `None` means caller overlap, a dangerous multi-token prefix, or an - /// unresolved semantic predicate requires full StarLoopEntry adaptive + /// unresolved semantic predicate requires full `StarLoopEntry` adaptive /// prediction (which includes the exit alt and precedence filtering). /// /// Single-token operators (`+`, `*`, relational `>` at low precedence) and @@ -5365,18 +5368,11 @@ where if symbol == TOKEN_EOF { return Some(false); } - let operator_lookahead = Self::cached_left_recursive_operator_lookahead( - atn, - state_number, - precedence, - ); - let can_single = operator_lookahead.single_token_symbols.contains(symbol); - let can_multi = operator_lookahead - .multi_token_prefix_symbols - .contains(symbol); - let can_predicate = operator_lookahead - .predicate_dependent_symbols - .contains(symbol); + let operator_lookahead = + Self::cached_left_recursive_operator_lookahead(atn, state_number, precedence); + let can_single = operator_lookahead.single_token.contains(symbol); + let can_multi = operator_lookahead.multi_token_prefix.contains(symbol); + let can_predicate = operator_lookahead.predicate_dependent.contains(symbol); if !can_single && !can_multi && !can_predicate { return Some(false); } @@ -5387,9 +5383,8 @@ where // single-token operator at precedence 0: defer so exit can win when the // multi-token sequence does not actually match (e.g. `>` vs `>>`). if !can_single && can_multi && precedence > 0 { - let baseline = - Self::cached_left_recursive_operator_lookahead(atn, state_number, 0); - if baseline.single_token_symbols.contains(symbol) { + let baseline = Self::cached_left_recursive_operator_lookahead(atn, state_number, 0); + if baseline.single_token.contains(symbol) { return None; } } @@ -12045,12 +12040,12 @@ mod tests { for (state, kind, rule) in [ (0, AtnStateKind::RuleStart, 0), (1, AtnStateKind::StarLoopEntry, 0), - (2, AtnStateKind::Basic, 0), // ops hub - (3, AtnStateKind::Basic, 0), // shift prec - (4, AtnStateKind::Basic, 0), // shift first > - (5, AtnStateKind::Basic, 0), // shift second > - (6, AtnStateKind::Basic, 0), // rel prec - (7, AtnStateKind::Basic, 0), // rel > + (2, AtnStateKind::Basic, 0), // ops hub + (3, AtnStateKind::Basic, 0), // shift prec + (4, AtnStateKind::Basic, 0), // shift first > + (5, AtnStateKind::Basic, 0), // shift second > + (6, AtnStateKind::Basic, 0), // rel prec + (7, AtnStateKind::Basic, 0), // rel > (8, AtnStateKind::LoopEnd, 0), (9, AtnStateKind::RuleStop, 0), ] { From 396465f4c577a4fb3c2eb8dabadec90432a0cf31 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Sat, 18 Jul 2026 15:35:11 +0200 Subject: [PATCH 3/4] Address AST parity review findings --- src/parser.rs | 544 ++++++++++++++++++++++++++++------ tools/parse-bench/run.py | 87 ++++-- tools/parse-bench/test_run.py | 61 ++++ 3 files changed, 577 insertions(+), 115 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 5c495dc..d5bbbb4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2860,15 +2860,62 @@ fn state_sync_symbols_inner( } } -#[derive(Clone, Copy, Debug, Default, Eq, Ord, PartialEq, PartialOrd)] -enum OperatorSymbolReachability { - #[default] - None, - PredicateDependent, - /// Starts an operator but more tokens are required before the RHS call. - UnconditionalMultiToken, - /// One token completes the operator token-prefix. - UnconditionalSingleToken, +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +struct OperatorSymbolReachability { + /// One token completes an unconditional operator token-prefix. + single_token: bool, + /// An unconditional operator path requires more tokens before its operand. + multi_token: bool, + /// At least one matching operator path depends on a semantic predicate. + predicate_dependent: bool, +} + +impl OperatorSymbolReachability { + const ADAPTIVE_FALLBACK: Self = Self { + single_token: false, + multi_token: false, + predicate_dependent: true, + }; + + const fn single_token(predicate_dependent: bool) -> Self { + if predicate_dependent { + Self { + single_token: false, + multi_token: false, + predicate_dependent: true, + } + } else { + Self { + single_token: true, + multi_token: false, + predicate_dependent: false, + } + } + } + + const fn multi_token(predicate_dependent: bool) -> Self { + if predicate_dependent { + Self { + single_token: false, + multi_token: false, + predicate_dependent: true, + } + } else { + Self { + single_token: false, + multi_token: true, + predicate_dependent: false, + } + } + } + + const fn union(self, other: Self) -> Self { + Self { + single_token: self.single_token || other.single_token, + multi_token: self.multi_token || other.multi_token, + predicate_dependent: self.predicate_dependent || other.predicate_dependent, + } + } } #[derive(Clone, Copy)] @@ -2876,6 +2923,14 @@ struct OperatorReachabilityRequest { symbol: i32, precedence: i32, predicate_dependent: bool, + operator_rule_index: usize, +} + +#[derive(Clone, Copy, Debug)] +struct OperatorRuleContinuation { + stop_state: usize, + follow_state: usize, + return_precedence: i32, } struct NullablePrecedenceCtx { @@ -3012,54 +3067,136 @@ fn state_is_nullable_with_precedence_cached( nullable } -/// After the operator's first token is matched, is the operator token-prefix -/// finished without needing another token? That includes binary ops (next is -/// the recursive RHS rule call), postfix ops (nullable to loop-back / stop), -/// but not multi-token ops like Java `>>` (another `>` is required). -fn state_operator_token_prefix_complete( +/// Classifies what remains after the operator's first token is matched. +fn state_operator_token_prefix_reachability( atn: &Atn, state_number: usize, - precedence: i32, - visited: &mut BTreeSet, -) -> bool { - if !visited.insert(state_number) { - return false; + request: OperatorReachabilityRequest, + continuations: &[OperatorRuleContinuation], + visited: &mut BTreeSet<(usize, i32, bool)>, +) -> OperatorSymbolReachability { + let key = ( + state_number, + request.precedence, + request.predicate_dependent, + ); + if !visited.insert(key) { + // Recursive helper rules can grow the return stack without consuming + // input. Delegate cycles to adaptive prediction instead of forcing a + // potentially incomplete one-token answer. + return OperatorSymbolReachability::ADAPTIVE_FALLBACK; + } + if let Some((continuation, remaining)) = continuations.split_last() + && state_number == continuation.stop_state + { + let result = state_operator_token_prefix_reachability( + atn, + continuation.follow_state, + OperatorReachabilityRequest { + precedence: continuation.return_precedence, + ..request + }, + remaining, + visited, + ); + visited.remove(&key); + return result; } let Some(state) = atn.state(state_number) else { - return false; + visited.remove(&key); + return OperatorSymbolReachability::default(); }; - match state.kind() { - AtnStateKind::RuleStop - | AtnStateKind::StarLoopBack + let completes_operator = match state.kind() { + AtnStateKind::RuleStop => continuations.is_empty(), + AtnStateKind::StarLoopBack | AtnStateKind::StarLoopEntry | AtnStateKind::PlusLoopBack - | AtnStateKind::LoopEnd => return true, - _ => {} + | AtnStateKind::LoopEnd => state.rule_index() == Some(request.operator_rule_index), + _ => false, + }; + if completes_operator { + visited.remove(&key); + return OperatorSymbolReachability::single_token(request.predicate_dependent); } - state - .transitions() - .iter() - .any(|transition| match &transition.data() { - Transition::Rule { .. } => true, + let mut reachability = OperatorSymbolReachability::default(); + for transition in &state.transitions() { + let transition_reachability = match &transition.data() { + Transition::Rule { rule_index, .. } if *rule_index == request.operator_rule_index => { + OperatorSymbolReachability::single_token(request.predicate_dependent) + } + Transition::Rule { + target, + rule_index, + follow_state, + precedence: rule_precedence, + } => { + let Some(child_stop) = atn.rule_to_stop_state().get(*rule_index) else { + continue; + }; + let mut nested = continuations.to_vec(); + nested.push(OperatorRuleContinuation { + stop_state: child_stop, + follow_state: *follow_state, + return_precedence: request.precedence, + }); + state_operator_token_prefix_reachability( + atn, + *target, + OperatorReachabilityRequest { + precedence: *rule_precedence, + ..request + }, + &nested, + visited, + ) + } Transition::Epsilon { target } | Transition::Action { target, .. } => { - state_operator_token_prefix_complete(atn, *target, precedence, visited) + state_operator_token_prefix_reachability( + atn, + *target, + request, + continuations, + visited, + ) } Transition::Precedence { target, precedence: transition_precedence, } => { - *transition_precedence >= precedence - && state_operator_token_prefix_complete(atn, *target, precedence, visited) - } - Transition::Predicate { target, .. } => { - state_operator_token_prefix_complete(atn, *target, precedence, visited) + if *transition_precedence < request.precedence { + OperatorSymbolReachability::default() + } else { + state_operator_token_prefix_reachability( + atn, + *target, + request, + continuations, + visited, + ) + } } + Transition::Predicate { target, .. } => state_operator_token_prefix_reachability( + atn, + *target, + OperatorReachabilityRequest { + predicate_dependent: true, + ..request + }, + continuations, + visited, + ), Transition::Atom { .. } | Transition::Range { .. } | Transition::Set { .. } | Transition::NotSet { .. } - | Transition::Wildcard { .. } => false, - }) + | Transition::Wildcard { .. } => { + OperatorSymbolReachability::multi_token(request.predicate_dependent) + } + }; + reachability = reachability.union(transition_reachability); + } + visited.remove(&key); + reachability } fn state_can_reach_symbol_with_precedence( @@ -3067,38 +3204,31 @@ fn state_can_reach_symbol_with_precedence( state_number: usize, request: OperatorReachabilityRequest, nullable_ctx: &mut NullablePrecedenceCtx, + continuations: &mut Vec, visited: &mut BTreeSet<(usize, i32, bool)>, ) -> OperatorSymbolReachability { - if !visited.insert(( + let key = ( state_number, request.precedence, request.predicate_dependent, - )) { - return OperatorSymbolReachability::None; + ); + if !visited.insert(key) { + return OperatorSymbolReachability::ADAPTIVE_FALLBACK; } let Some(state) = atn.state(state_number) else { - return OperatorSymbolReachability::None; + visited.remove(&key); + return OperatorSymbolReachability::default(); }; - let mut reachability = OperatorSymbolReachability::None; + let mut reachability = OperatorSymbolReachability::default(); for transition in &state.transitions() { if transition.matches(request.symbol, 1, atn.max_token_type()) { - let matched = if state_operator_token_prefix_complete( + reachability = reachability.union(state_operator_token_prefix_reachability( atn, transition.target(), - request.precedence, + request, + continuations, &mut BTreeSet::new(), - ) { - OperatorSymbolReachability::UnconditionalSingleToken - } else { - OperatorSymbolReachability::UnconditionalMultiToken - }; - if request.predicate_dependent { - reachability = reachability.max(OperatorSymbolReachability::PredicateDependent); - } else if matched == OperatorSymbolReachability::UnconditionalSingleToken { - return matched; - } else { - reachability = reachability.max(matched); - } + )); continue; } let transition_reachability = match &transition.data() { @@ -3108,6 +3238,14 @@ fn state_can_reach_symbol_with_precedence( follow_state, precedence: rule_precedence, } => { + let Some(child_stop) = atn.rule_to_stop_state().get(*rule_index) else { + continue; + }; + continuations.push(OperatorRuleContinuation { + stop_state: child_stop, + follow_state: *follow_state, + return_precedence: request.precedence, + }); let mut result = state_can_reach_symbol_with_precedence( atn, *target, @@ -3116,14 +3254,10 @@ fn state_can_reach_symbol_with_precedence( ..request }, nullable_ctx, + continuations, visited, ); - if result == OperatorSymbolReachability::UnconditionalSingleToken { - return result; - } - let Some(child_stop) = atn.rule_to_stop_state().get(*rule_index) else { - continue; - }; + continuations.pop(); if state_is_nullable_with_precedence( atn, *target, @@ -3141,7 +3275,7 @@ fn state_can_reach_symbol_with_precedence( false, nullable_ctx, ); - result = result.max(state_can_reach_symbol_with_precedence( + result = result.union(state_can_reach_symbol_with_precedence( atn, *follow_state, OperatorReachabilityRequest { @@ -3149,6 +3283,7 @@ fn state_can_reach_symbol_with_precedence( ..request }, nullable_ctx, + continuations, visited, )); } @@ -3166,7 +3301,14 @@ fn state_can_reach_symbol_with_precedence( ) { continue; } - state_can_reach_symbol_with_precedence(atn, *target, request, nullable_ctx, visited) + state_can_reach_symbol_with_precedence( + atn, + *target, + request, + nullable_ctx, + continuations, + visited, + ) } Transition::Predicate { target, .. } => state_can_reach_symbol_with_precedence( atn, @@ -3176,19 +3318,18 @@ fn state_can_reach_symbol_with_precedence( ..request }, nullable_ctx, + continuations, visited, ), Transition::Atom { .. } | Transition::Range { .. } | Transition::Set { .. } | Transition::NotSet { .. } - | Transition::Wildcard { .. } => OperatorSymbolReachability::None, + | Transition::Wildcard { .. } => OperatorSymbolReachability::default(), }; - if transition_reachability == OperatorSymbolReachability::UnconditionalSingleToken { - return transition_reachability; - } - reachability = reachability.max(transition_reachability); + reachability = reachability.union(transition_reachability); } + visited.remove(&key); reachability } @@ -3200,6 +3341,9 @@ fn left_recursive_operator_lookahead( let Some(state) = atn.state(state_number) else { return LeftRecursiveOperatorLookahead::default(); }; + let Some(operator_rule_index) = state.rule_index() else { + return LeftRecursiveOperatorLookahead::default(); + }; let mut lookahead = LeftRecursiveOperatorLookahead::default(); let mut nullable_ctx = NullablePrecedenceCtx { cache: FxHashMap::default(), @@ -3215,27 +3359,27 @@ fn left_recursive_operator_lookahead( continue; } for symbol in 1..=atn.max_token_type() { - match state_can_reach_symbol_with_precedence( + let reachability = state_can_reach_symbol_with_precedence( atn, target, OperatorReachabilityRequest { symbol, precedence, predicate_dependent: false, + operator_rule_index, }, &mut nullable_ctx, + &mut Vec::new(), &mut BTreeSet::new(), - ) { - OperatorSymbolReachability::UnconditionalSingleToken => { - lookahead.single_token.insert(symbol); - } - OperatorSymbolReachability::UnconditionalMultiToken => { - lookahead.multi_token_prefix.insert(symbol); - } - OperatorSymbolReachability::PredicateDependent => { - lookahead.predicate_dependent.insert(symbol); - } - OperatorSymbolReachability::None => {} + ); + if reachability.single_token { + lookahead.single_token.insert(symbol); + } + if reachability.multi_token { + lookahead.multi_token_prefix.insert(symbol); + } + if reachability.predicate_dependent { + lookahead.predicate_dependent.insert(symbol); } } } @@ -5376,7 +5520,7 @@ where if !can_single && !can_multi && !can_predicate { return Some(false); } - if can_predicate && !can_single && !can_multi { + if can_predicate && !can_single { return None; } // Multi-token-only at this precedence, but the same symbol is a @@ -12118,6 +12262,195 @@ mod tests { finish_atn(atn) } + fn left_recursive_loop_with_rule_wrapped_gt_prefix_atn() -> Atn { + let mut atn = ParserAtnBuilder::new(2); + for (state, kind, rule) in [ + (0, AtnStateKind::RuleStart, 0), + (1, AtnStateKind::StarLoopEntry, 0), + (2, AtnStateKind::Basic, 0), + (3, AtnStateKind::Basic, 0), + (4, AtnStateKind::Basic, 0), + (5, AtnStateKind::Basic, 0), + (6, AtnStateKind::Basic, 0), + (7, AtnStateKind::Basic, 0), + (8, AtnStateKind::LoopEnd, 0), + (9, AtnStateKind::RuleStop, 0), + (10, AtnStateKind::RuleStart, 1), + (11, AtnStateKind::Basic, 1), + (12, AtnStateKind::RuleStop, 1), + ] { + assert_eq!( + atn.add_state(kind, Some(rule)).expect("state").index(), + state + ); + if state == 0 { + atn.set_left_recursive_rule(state) + .expect("left-recursive rule start"); + } else if state == 1 { + atn.set_precedence_rule_decision(state) + .expect("precedence decision"); + } + } + atn.set_rule_to_start_state(vec![0, 10]) + .expect("rule start states"); + atn.set_rule_to_stop_state(vec![9, 12]) + .expect("rule stop states"); + atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 }) + .expect("ops"); + atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 8 }) + .expect("exit"); + atn.add_transition(2, ParserTransitionSpec::Epsilon { target: 3 }) + .expect("to shift"); + atn.add_transition(2, ParserTransitionSpec::Epsilon { target: 6 }) + .expect("to relational"); + atn.add_transition( + 3, + ParserTransitionSpec::Precedence { + target: 4, + precedence: 2, + }, + ) + .expect("shift precedence"); + atn.add_transition( + 4, + ParserTransitionSpec::Rule { + target: 10, + rule_index: 1, + follow_state: 5, + precedence: 0, + }, + ) + .expect("first shift token helper"); + atn.add_transition( + 5, + ParserTransitionSpec::Atom { + target: 1, + label: 1, + }, + ) + .expect("second shift token"); + atn.add_transition( + 6, + ParserTransitionSpec::Precedence { + target: 7, + precedence: 1, + }, + ) + .expect("relational precedence"); + atn.add_transition( + 7, + ParserTransitionSpec::Atom { + target: 1, + label: 1, + }, + ) + .expect("relational token"); + atn.add_transition(8, ParserTransitionSpec::Epsilon { target: 9 }) + .expect("loop end"); + atn.add_transition(10, ParserTransitionSpec::Epsilon { target: 11 }) + .expect("helper entry"); + atn.add_transition( + 11, + ParserTransitionSpec::Atom { + target: 12, + label: 1, + }, + ) + .expect("first shift token"); + finish_atn(atn) + } + + fn left_recursive_loop_with_predicate_and_multi_token_prefix_atn() -> Atn { + let mut atn = ParserAtnBuilder::new(1); + for (state, kind) in [ + (0, AtnStateKind::RuleStart), + (1, AtnStateKind::StarLoopEntry), + (2, AtnStateKind::Basic), + (3, AtnStateKind::Basic), + (4, AtnStateKind::Basic), + (5, AtnStateKind::Basic), + (6, AtnStateKind::Basic), + (7, AtnStateKind::Basic), + (8, AtnStateKind::Basic), + (9, AtnStateKind::LoopEnd), + (10, AtnStateKind::RuleStop), + ] { + assert_eq!(atn.add_state(kind, Some(0)).expect("state").index(), state); + if state == 0 { + atn.set_left_recursive_rule(state) + .expect("left-recursive rule start"); + } else if state == 1 { + atn.set_precedence_rule_decision(state) + .expect("precedence decision"); + } + } + atn.set_rule_to_start_state(vec![0]) + .expect("rule start states"); + atn.set_rule_to_stop_state(vec![10]) + .expect("rule stop states"); + atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 }) + .expect("ops"); + atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 9 }) + .expect("exit"); + atn.add_transition(2, ParserTransitionSpec::Epsilon { target: 3 }) + .expect("to multi-token operator"); + atn.add_transition(2, ParserTransitionSpec::Epsilon { target: 6 }) + .expect("to predicate operator"); + atn.add_transition( + 3, + ParserTransitionSpec::Precedence { + target: 4, + precedence: 2, + }, + ) + .expect("multi-token precedence"); + atn.add_transition( + 4, + ParserTransitionSpec::Atom { + target: 5, + label: 1, + }, + ) + .expect("multi-token first"); + atn.add_transition( + 5, + ParserTransitionSpec::Atom { + target: 1, + label: 1, + }, + ) + .expect("multi-token second"); + atn.add_transition( + 6, + ParserTransitionSpec::Precedence { + target: 7, + precedence: 2, + }, + ) + .expect("predicate precedence"); + atn.add_transition( + 7, + ParserTransitionSpec::Predicate { + target: 8, + rule_index: 0, + pred_index: 0, + context_dependent: false, + }, + ) + .expect("operator predicate"); + atn.add_transition( + 8, + ParserTransitionSpec::Atom { + target: 1, + label: 1, + }, + ) + .expect("predicate single token"); + atn.add_transition(9, ParserTransitionSpec::Epsilon { target: 10 }) + .expect("loop end"); + finish_atn(atn) + } + fn left_recursive_loop_with_nullable_operator_prefix_atn() -> Atn { let mut atn = ParserAtnBuilder::new(2); for (state, kind, rule) in [ @@ -12616,6 +12949,51 @@ mod tests { ); } + #[test] + fn left_recursive_loop_preserves_rule_wrapped_operator_continuation() { + let atn = left_recursive_loop_with_rule_wrapped_gt_prefix_atn(); + let mut parser = mini_parser(vec![ + TestToken::new(1).with_text(">"), + TestToken::new(2).with_text("id"), + TestToken::eof("parser-test", 1, 1, 1), + ]); + parser.rule_context_stack = vec![RuleContextFrame { + rule_index: 0, + invoking_state: -1, + }]; + + assert_eq!( + parser.left_recursive_loop_enter_prediction(&atn, 1, 0), + Some(true), + "the direct relational alternative remains a one-token operator" + ); + assert_eq!( + parser.left_recursive_loop_enter_prediction(&atn, 1, 2), + None, + "a token matched in the helper rule must return to the second shift token" + ); + } + + #[test] + fn left_recursive_loop_preserves_predicate_and_multi_token_reachability() { + let atn = left_recursive_loop_with_predicate_and_multi_token_prefix_atn(); + let mut parser = mini_parser(vec![ + TestToken::new(1).with_text(">"), + TestToken::new(2).with_text("id"), + TestToken::eof("parser-test", 1, 1, 1), + ]); + parser.rule_context_stack = vec![RuleContextFrame { + rule_index: 0, + invoking_state: -1, + }]; + + assert_eq!( + parser.left_recursive_loop_enter_prediction(&atn, 1, 2), + None, + "a predicate-gated single-token path must not be hidden by a multi-token path" + ); + } + #[test] fn left_recursive_loop_defers_predicate_guarded_operator() { let atn = left_recursive_loop_with_predicate_guarded_operator_atn(); diff --git a/tools/parse-bench/run.py b/tools/parse-bench/run.py index 94d3fb3..3b672f0 100755 --- a/tools/parse-bench/run.py +++ b/tools/parse-bench/run.py @@ -574,18 +574,28 @@ def write_rust_runner( }} NodeKind::Terminal => writeln!( out, - "{{pad}}Term({{:?}})", - tree.as_terminal().expect("terminal node kind checked").text() + "{{pad}}Term({{}})", + tree_text(tree.as_terminal().expect("terminal node kind checked").text()) )?, NodeKind::Error => writeln!( out, - "{{pad}}Err({{:?}})", - tree.as_error().expect("error node kind checked").text() + "{{pad}}Err({{}})", + tree_text(tree.as_error().expect("error node kind checked").text()) )?, }} Ok(()) }} +fn tree_text(text: &str) -> String {{ + let mut escaped = String::new(); + escaped.push('"'); + for ch in text.chars() {{ + escaped.extend(ch.escape_unicode()); + }} + escaped.push('"'); + escaped +}} + fn prediction_stats_once( language: &str, src: &str, @@ -684,15 +694,17 @@ def rust_parse_function(spec: LanguageSpec) -> str: fn dump_tree_{spec.name}(src: &str, out: &mut dyn Write) -> Result<(), String> {{ let lexer = generated::{spec.rust_lexer_module}::{spec.rust_lexer_type}::new(InputStream::new(src)); - let tokens = CommonTokenStream::new(lexer); + let mut tokens = CommonTokenStream::new(lexer); + tokens.fill(); + let lexer_errors = tokens.drain_source_errors().len(); let mut parser = generated::{spec.rust_parser_module}::{spec.rust_parser_type}::new(tokens); let root = parser .{spec.rust_entry}() .map_err(|err| err.to_string())?; let syntax_errors = parser.number_of_syntax_errors(); - if syntax_errors != 0 {{ + if lexer_errors != 0 || syntax_errors != 0 {{ return Err(format!( - "rust-antlr {spec.name} parse produced {{syntax_errors}} syntax error(s)" + "rust-antlr {spec.name} parse produced {{lexer_errors}} lexer error(s) and {{syntax_errors}} parser syntax error(s)" )); }} dump_node( @@ -918,6 +930,25 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: var sink any +type countingErrorListener struct {{ + *antlr.DefaultErrorListener + count int +}} + +func newCountingErrorListener() *countingErrorListener {{ + return &countingErrorListener{{DefaultErrorListener: antlr.NewDefaultErrorListener()}} +}} + +func (l *countingErrorListener) SyntaxError( + _ antlr.Recognizer, + _ interface{{}}, + _, _ int, + _ string, + _ antlr.RecognitionException, +) {{ + l.count++ +}} + func main() {{ if err := runMain(); err != nil {{ fmt.Fprintln(os.Stderr, err) @@ -1009,10 +1040,10 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: pad := strings.Repeat(" ", depth) switch node := tree.(type) {{ case antlr.ErrorNode: - _, err := fmt.Fprintf(out, "%sErr(%s)\\n", pad, rustDebugStr(node.GetText())) + _, err := fmt.Fprintf(out, "%sErr(%s)\\n", pad, treeText(node.GetText())) return err case antlr.TerminalNode: - _, err := fmt.Fprintf(out, "%sTerm(%s)\\n", pad, rustDebugStr(node.GetText())) + _, err := fmt.Fprintf(out, "%sTerm(%s)\\n", pad, treeText(node.GetText())) return err case antlr.RuleNode: ruleIndex := node.GetRuleContext().GetRuleIndex() @@ -1050,30 +1081,11 @@ def write_go_runner(work_dir: Path, specs: list[LanguageSpec]) -> Path: return false }} -func rustDebugStr(text string) string {{ +func treeText(text string) string {{ var b strings.Builder b.WriteByte('"') for _, r := range text {{ - switch r {{ - case '\\\\': - b.WriteString(`\\\\`) - case '"': - b.WriteString(`\\"`) - case '\\n': - b.WriteString(`\\n`) - case '\\r': - b.WriteString(`\\r`) - case '\\t': - b.WriteString(`\\t`) - case 0: - b.WriteString(`\\0`) - default: - if r < 0x20 || r == 0x7f {{ - fmt.Fprintf(&b, `\\u{{%x}}`, r) - }} else {{ - b.WriteRune(r) - }} - }} + fmt.Fprintf(&b, `\\u{{%x}}`, r) }} b.WriteByte('"') return b.String() @@ -1131,11 +1143,22 @@ def go_parse_function(spec: LanguageSpec) -> str: func dumpTree{func_name}(src string, out io.Writer) error {{ input := antlr.NewInputStream(src) lexer := {pkg}.New{spec.lexer_name}(input) + lexerErrors := newCountingErrorListener() + lexer.RemoveErrorListeners() + lexer.AddErrorListener(lexerErrors) tokens := antlr.NewCommonTokenStream(lexer, antlr.TokenDefaultChannel) + tokens.Fill() p := {pkg}.New{spec.parser_name}(tokens) + parserErrors := newCountingErrorListener() + p.RemoveErrorListeners() + p.AddErrorListener(parserErrors) tree := p.{spec.go_entry}() - if treeHasErrorNode(tree) {{ - return fmt.Errorf("go-antlr {spec.name} parse produced error nodes") + if lexerErrors.count != 0 || parserErrors.count != 0 || treeHasErrorNode(tree) {{ + return fmt.Errorf( + "go-antlr {spec.name} parse produced %d lexer error(s), %d parser syntax error(s), or error nodes", + lexerErrors.count, + parserErrors.count, + ) }} return dumpNode(out, tree, p.GetRuleNames(), 0) }}""" diff --git a/tools/parse-bench/test_run.py b/tools/parse-bench/test_run.py index f39b041..bb7ebed 100644 --- a/tools/parse-bench/test_run.py +++ b/tools/parse-bench/test_run.py @@ -14,6 +14,22 @@ class DumpTreeHelpersTests(unittest.TestCase): + def generated_sources(self) -> tuple[str, str]: + with tempfile.TemporaryDirectory() as temp: + work_dir = Path(temp) + spec = RUN.LANGUAGES["java"] + rust_runner = RUN.write_rust_runner( + work_dir, + [spec], + RUN.ROOT, + rust_thin_lto=False, + ) + go_runner = RUN.write_go_runner(work_dir, [spec]) + return ( + (rust_runner / "src" / "main.rs").read_text(), + (go_runner / "main.go").read_text(), + ) + def test_detects_error_nodes(self) -> None: clean = 'Rule(root, children=1)\n Term("x")\n' dirty = 'Rule(root, children=1)\n Err("!")\n' @@ -27,6 +43,51 @@ def test_format_tree_diff_mentions_both_runtimes(self) -> None: self.assertIn("-Rule(a, children=0)", diff) self.assertIn("+Rule(b, children=0)", diff) + def test_generated_dumpers_share_unicode_scalar_encoding(self) -> None: + rust_source, go_source = self.generated_sources() + + self.assertIn("tree_text(tree.as_terminal()", rust_source) + self.assertIn("tree_text(tree.as_error()", rust_source) + self.assertIn("escaped.extend(ch.escape_unicode());", rust_source) + self.assertNotIn("Term({:?})", rust_source) + self.assertNotIn("Err({:?})", rust_source) + + self.assertIn("treeText(node.GetText())", go_source) + self.assertIn(r"fmt.Fprintf(&b, `\u{%x}`, r)", go_source) + self.assertNotIn("rustDebugStr", go_source) + + sample = "\u0301\u00a0\u2028a" + expected = '"\\u{301}\\u{a0}\\u{2028}\\u{61}"' + self.assertEqual( + '"' + "".join(f"\\u{{{ord(ch):x}}}" for ch in sample) + '"', + expected, + ) + + def test_generated_dumpers_reject_lexer_and_parser_diagnostics(self) -> None: + rust_source, go_source = self.generated_sources() + rust_dump = rust_source[rust_source.index("fn dump_tree_java") :] + go_dump = go_source[go_source.index("func dumpTreeJava") :] + + rust_fill = rust_dump.index("tokens.fill();") + rust_lexer_errors = rust_dump.index("tokens.drain_source_errors().len()") + rust_parse = rust_dump.index("let root = parser") + self.assertLess(rust_fill, rust_lexer_errors) + self.assertLess(rust_lexer_errors, rust_parse) + self.assertIn( + "if lexer_errors != 0 || syntax_errors != 0", + rust_dump, + ) + + go_listener = go_dump.index("lexer.AddErrorListener(lexerErrors)") + go_fill = go_dump.index("tokens.Fill()") + go_parse = go_dump.index("tree := p.CompilationUnit()") + self.assertLess(go_listener, go_fill) + self.assertLess(go_fill, go_parse) + self.assertIn( + "lexerErrors.count != 0 || parserErrors.count != 0", + go_dump, + ) + class ClearWorkDirTests(unittest.TestCase): def test_rejects_runtime_root_and_ancestor(self) -> None: From ad4f553e9dcf32a91a30656ca93ebccd830019d1 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Sat, 18 Jul 2026 15:44:35 +0200 Subject: [PATCH 4/4] Keep parser guidance grammar agnostic --- src/parser.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index d5bbbb4..4f9c40e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2341,8 +2341,8 @@ struct LeftRecursiveOperatorLookahead { /// Safe for one-token loop-enter fast path. single_token: TokenBitSet, /// Operator alts that start with this symbol but still require more tokens - /// (e.g. Java `>>` / `>>>` when only shift is precedence-viable). Must not - /// force enter from one-token lookahead — `StarLoopEntry` adaptive predict + /// before the operand. Must not force enter from one-token lookahead when a + /// shorter operator shares the prefix; `StarLoopEntry` adaptive prediction /// has to weigh the exit alt as well. multi_token_prefix: TokenBitSet, predicate_dependent: TokenBitSet, @@ -5494,14 +5494,12 @@ where /// unresolved semantic predicate requires full `StarLoopEntry` adaptive /// prediction (which includes the exit alt and precedence filtering). /// - /// Single-token operators (`+`, `*`, relational `>` at low precedence) and - /// multi-token ops that do not shadow a lower-precedence single-token use of - /// the same symbol (e.g. `.`) keep the one-token enter fast path. + /// Single-token operators and multi-token prefixes that do not shadow a + /// lower-precedence single-token operator keep the one-token enter fast path. /// /// Multi-token prefixes that **do** shadow a lower-precedence single-token - /// op (Java `>>`/`>>>` vs `>` when only shift is precedence-viable) must not - /// force enter — otherwise the operator decision picks relational, - /// `precpred` fails, and a legal outer expression dies. + /// operator must not force enter; the adaptive decision may need to select + /// the loop exit instead. pub fn left_recursive_loop_enter_prediction( &mut self, atn: &Atn,