feat(codegen): hoist the generated support preamble into the runtime - #329
Conversation
…to the runtime Every generated parser module carried a 284-line grammar-independent support preamble (__GeneratedInput/__GeneratedTokenView, the TerminalNode/ErrorNode wrappers, the __GeneratedRuleContext source enum with its child-iteration helpers, the __FromActiveRuleContext trait and view adapters, __write_invocation_states, and the __RecoveryContextState markers) that was byte-identical across every generated parser, so N linked parsers compiled N copies of the same code and the __terminal_children triage had to be kept aligned with the runtime by hand. These items now live once in antlr4_runtime::generated as real #[doc(hidden)] (public API items excepted) definitions, and generated modules import them by name instead of re-declaring them: `pub use` for the items that were public generated API (TerminalNode, ErrorNode, StoredTreeContext, __GeneratedInput, __GeneratedTokenView) so existing paths keep resolving, and a plain `use` for the module-internal helpers the runtime-owned context macros resolve unhygienically at the invocation site. The helpers carry #[inline] so downstream recognizers keep the cross-crate inlining they previously got from module-local definitions. Two deliberate non-moves: - ValidatedTreeContext stays emitted per generated module. The accessors macro emits overlapping impl blocks for `State: __RecoveryContextState` and for `ValidatedTreeContext`, and rustc only accepts them while it can prove the validated marker never implements the trait - which requires the marker type to be local to the generated crate (E0592 "upstream crates may add a new impl" otherwise). This constraint is documented on the macro and on the emitted marker. - The terminal-child triage now exists exactly once: Node::terminal_view / Node::labeled_terminal_view in tree.rs back RuleNodeView::terminal_children, ParserRuleContext::terminal_children/child_tokens/labeled_terminal_children, and the hoisted generated helpers, so the hand-aligned copy (and its alignment comment) is gone. The generated-source contract changes, so __ANTLR4_RUST_CODEGEN_API is now 9; revisions 1-8 remain accepted because their generated source declares its own preamble and needs no removed runtime surface. TerminalNode/ErrorNode gained a hidden node() accessor because the emitted Visitable impls previously read the module-local private __node field, which a runtime-owned type no longer permits. Measured on the checked-in recognizers: each generated parser shrinks by 272 lines / 7,014 bytes (toml 3123 -> 2851, g4 7595 -> 7323, rust 30975 -> 30703; -816 lines total). The antlr4-rust-gen binary, which links all three parsers, is size-neutral (stripped: 35,117,720 -> 35,131,464 bytes, +0.04%) under the default no-LTO release profile. Closes #318
Copy/Paste DetectionFound 10 duplication(s) across 12 changed non-generated Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 26 line (142 tokens) duplication in the following files:
atn.set_end_state(1, 4).expect("block end state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 3 })
.expect("transition");
atn.add_transition(
2,
ParserTransitionSpec::Atom {
target: 4,
label: 1,
},
)
.expect("transition");
atn.add_transition(
3,
ParserTransitionSpec::Atom {
target: 4,
label: 2,
},
)
.expect("transition");
atn.add_transition(4, ParserTransitionSpec::Epsilon { target: 5 })
.expect("transition");
atn.add_decision_state(1).expect("decision state");
```rust
---
Found a 26 line (125 tokens) duplication in the following files:
* Starting at line 741 of crates/antlr-rust-runtime/src/generated.rs
* Starting at line 912 of crates/antlr-rust-runtime/src/generated.rs
```rust
$input: $crate::char_stream::CharStream,
$hooks: $crate::parser::SemanticHooks,
{
pub fn metadata() -> &'static $crate::generated::GrammarMetadata {
$metadata()
}
/// Adds a listener for lexer diagnostics.
pub fn add_error_listener<T>(&mut self, listener: T)
where
T: for<'a> $crate::errors::ErrorListener<dyn $crate::recognizer::Recognizer + 'a>
+ ::core::marker::Send
+ 'static,
{
$crate::recognizer::Recognizer::add_error_listener(&mut self.$base, listener);
}
/// Removes every lexer error listener, including the default console listener.
pub fn remove_error_listeners(&mut self) {
$crate::recognizer::Recognizer::remove_error_listeners(&mut self.$base);
}
/// Routes every token through ATN interpretation instead of the compiled
/// lexer DFA, so the learned-DFA trace (`lexer_dfa_string`) observes each
/// match.
pub fn set_force_interpreted(&mut self, force_interpreted: bool) {Found a 25 line (115 tokens) duplication in the following files:
atn.add_state(AtnStateKind::BlockStart, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
3
);
assert_eq!(
atn.add_state(AtnStateKind::BlockEnd, Some(0))
.expect("state")
.index(),
4
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStop, Some(0))
```rust
---
Found a 22 line (112 tokens) duplication in the following files:
* Starting at line 4311 of crates/antlr-rust-codegen/src/generator/tests.rs
* Starting at line 4385 of crates/antlr-rust-codegen/src/generator/tests.rs
```rust
fn plus_loop_atn() -> ParserAtn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::PlusBlockStart, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::BlockEnd, Some(0))Found a 27 line (110 tokens) duplication in the following files:
decision: 0,
alts: (1, 2),
track_alt_number: false,
allow_semantic_context: true,
force_context: false,
plus_loop: false,
fast_path: None,
body: &body,
},
0,
GeneratedStepRenderContext {
current_rule_index: 0,
embedded: None,
portable_locals: None,
decision_routing: DecisionRoutingRender::default(),
inline_action_statements: &BTreeMap::new(),
track_alt_numbers: false,
track_context_alt_numbers: false,
direct_generated_rule_calls: &[],
atn_preferred_rule_calls: &[],
adaptive_atn_preferred_rule_slots: &[],
adaptive_atn_probe_rule_slots: &[],
},
);
// The whole rendered star-loop captures the leading-predicate-to-exit-alt filtering.
insta::assert_snapshot!(
```rust
---
Found a 17 line (105 tokens) duplication in the following files:
* Starting at line 177 of crates/antlr-rust-runtime/src/generated.rs
* Starting at line 274 of crates/antlr-rust-runtime/src/generated.rs
```rust
fn __from_node_with_invocation_states(
node: $crate::RuleNodeView<'a>,
invocation_states: Option<Vec<isize>>,
) -> Self {
$(
let __default = <$attrs>::default();
let __attrs = node.generated_attrs::<$attrs>().unwrap_or(&__default);
)?
Self {
__node: __GeneratedRuleContext::Stored(node),
__invocation_states: invocation_states,
__state: std::marker::PhantomData,
$(
$($field: __attrs.$field.clone(),)+
)?
}
}Found a 25 line (104 tokens) duplication in the following files:
state: 1,
decision: 0,
track_alt_number: false,
allow_semantic_context: false,
force_context: false,
fast_path: None,
alts: &alts,
},
0,
GeneratedStepRenderContext {
current_rule_index: 0,
embedded: None,
portable_locals: None,
decision_routing: DecisionRoutingRender::default(),
inline_action_statements: &BTreeMap::new(),
track_alt_numbers: false,
track_context_alt_numbers: false,
direct_generated_rule_calls: &[],
atn_preferred_rule_calls: &[],
adaptive_atn_preferred_rule_slots: &[],
adaptive_atn_probe_rule_slots: &[],
},
);
assert!(rendered.contains("ll1_decision_prediction(atn(), 1)"));
```rust
---
Found a 28 line (102 tokens) duplication in the following files:
* Starting at line 3331 of crates/antlr-rust-codegen/src/generator/tests.rs
* Starting at line 3491 of crates/antlr-rust-codegen/src/generator/tests.rs
```rust
state: 1,
decision: 0,
track_alt_number: false,
allow_semantic_context: true,
force_context: false,
fast_path: None,
alts: &alts,
},
0,
GeneratedStepRenderContext {
current_rule_index: 0,
embedded: None,
portable_locals: None,
decision_routing: DecisionRoutingRender::default(),
inline_action_statements: &BTreeMap::new(),
track_alt_numbers: false,
track_context_alt_numbers: false,
direct_generated_rule_calls: &[],
atn_preferred_rule_calls: &[],
adaptive_atn_preferred_rule_slots: &[],
adaptive_atn_probe_rule_slots: &[],
},
);
// One decision renders into a fresh String; snapshot the whole emitted control flow (the
// semantic-context gate, both predicate probes, the alt rewrite, the no-viable fallback)
// instead of six positive probes plus one negative guard.
insta::assert_snapshot!(Found a 16 line (101 tokens) duplication in the following files:
atn.set_loop_back_state(3, 4).expect("loop back state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 3 })
.expect("transition");
atn.add_transition(
2,
ParserTransitionSpec::Atom {
target: 4,
label: 1,
},
)
.expect("transition");
atn.add_transition(4, ParserTransitionSpec::Epsilon { target: 1 })
```rust
---
Found a 20 line (100 tokens) duplication in the following files:
* Starting at line 15 of crates/antlr-rust-codegen/src/parser/surface/names.rs
* Starting at line 343 of crates/antlr-rust-codegen/src/parser/surface/support_abi.rs
```rust
for (rule_index, rule) in model.rules.iter().enumerate() {
if !rule.has_attrs() {
continue;
}
let struct_name = embedded::attrs_struct_name(rule_index);
let mut fields = String::new();
for attr in &rule.attrs {
let _ = writeln!(
fields,
" pub {}: {},",
embedded::escape_keyword(&attr.name),
attr.ty
);
}
let _ = writeln!(
out.attrs_structs,
"#[derive(Clone, Debug, Default)]\n#[allow(non_snake_case, dead_code)]\npub struct {struct_name} {{\n{fields}}}\n"
);
}
out.module_items.push_str(&render_embedded_context_types( |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (17)
📒 Files selected for processing (14)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe runtime now owns shared generated parser input, tree, context, and traversal support. The generator imports this support instead of emitting duplicate declarations. The codegen API revision advances to 9, with updated compatibility tests and documentation. ChangesRuntime-owned parser support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GeneratedParser
participant RuntimeGenerated
participant RuntimeTree
GeneratedParser->>RuntimeGenerated: import shared parser support
RuntimeGenerated->>RuntimeTree: convert terminal and error nodes
RuntimeTree-->>GeneratedParser: return tree and child views
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @tinovyatkin's task in 20m 42s —— View job Code Review: hoist the generated support preamble into the runtimeReviewed the full diff against Verification run on this branch
The hoist itself is semantically clean. I diffed each moved item against its removed generated counterpart: Three things I checked specifically and found correct:
A nice unadvertised side effect: because each generated file ends in Findings below are all non-blocking — test-quality, docs, and hardening. No correctness defect found. 1. Two assertions in
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| crates/antlr-rust-runtime/src/tree.rs | 329 (main: 339) 🟢 | 84 (main: 92) 🟢 | 178 (main: 176) 🔴 | 475 (main: 473) 🔴 | 0 ⚪ |
| crates/antlr-rust-runtime/src/generated.rs | 75 (main: 20) 🔴 | 8 (main: 0) 🔴 | 40 (main: 14) 🔴 | 61 (main: 21) 🔴 | 0 (main: 2.30) 🔴 |
| crates/antlr-rust-codegen/src/parser/surface/support_abi.rs | 157 ⚪ | 95 ⚪ | 30 ⚪ | 251 (main: 252) 🟢 | 0 ⚪ |
| crates/antlr-rust-codegen/src/parser/surface/names.rs | 65 ⚪ | 40 ⚪ | 16 ⚪ | 108 (main: 109) 🟢 | 5.08 (main: 5.04) 🟢 |
| crates/antlr-rust-codegen/src/parser/surface/contexts.rs | 25 ⚪ | 22 ⚪ | 4 ⚪ | 69 (main: 72) 🟢 | 16.57 (main: 11.54) 🟢 |
| crates/antlr-rust-codegen/src/parser/render/mod.rs | 43 ⚪ | 41 ⚪ | 3 ⚪ | 92 ⚪ | 6.99 (main: 6.92) 🟢 |
| crates/antlr-rust-codegen/src/parser/surface/traversal.rs | 6 ⚪ | 2 ⚪ | 3 ⚪ | 36 ⚪ | 15.68 (main: 14.92) 🟢 |
| crates/antlr-rust-codegen/src/parser/surface/facade.rs | 6 ⚪ | 1 ⚪ | 1 ⚪ | 11 ⚪ | 38.82 (main: 34.15) 🟢 |
| crates/antlr-rust-runtime/src/lib.rs | 4 ⚪ | 3 ⚪ | 1 ⚪ | 7 ⚪ | 29.51 (main: 29.59) 🔴 |
Generated by mehen v1.8.1 — the code quality watcher.
1 participant
Closes #318.
What
The 284-line grammar-independent support preamble that every generated parser re-declared (
__GeneratedInputthroughimpl __RecoveryContextState for __ActiveParserContext {}) now lives once inantlr4_runtime::generatedas real#[doc(hidden)]items (public generated API excepted), and generated modules import it:pub use antlr4_runtime::generated::{ErrorNode, StoredTreeContext, TerminalNode, __GeneratedInput, __GeneratedTokenView};keeps the previously-public items reachable under their current generated paths.usebrings the module-internal helpers (__GeneratedRuleContext, child-iteration helpers,__FromActiveRuleContext+ view adapters,__write_invocation_states,__RecoveryContextState) into scope so the runtime-owned__antlr4_rust_context!/__antlr4_rust_context_accessors!expansions — which resolve these names unhygienically at the invocation site — keep working unchanged for both old and new generated source.#[inline]so downstream recognizers keep the cross-crate inlining they previously got from module-local definitions.Triage dedup
Node::terminal_view/Node::labeled_terminal_view(crate-private,tree.rs) are now the single home of the terminal-child triage. They backRuleNodeView::{child_tokens, terminal_children},ParserRuleContext::{child_tokens, terminal_children, labeled_terminal_children}, and the hoisted__terminal_children/__labeled_token_children*helpers. The generated copy and its "Keep this triage aligned" comment are gone.What deliberately did not move
ValidatedTreeContextstays emitted per generated module. The accessors macro emits overlapping impl blocks forState: __RecoveryContextStateand forValidatedTreeContext; rustc accepts them only while it can prove the validated marker never implements the trait, and that negative reasoning requires the marker type to be local to the generated crate (moving it upstream fails with E0592 "upstream crates may add a new impl of trait__RecoveryContextStatefor typeValidatedTreeContext"). Documented on the macro and on the emitted marker.Compatibility
__ANTLR4_RUST_CODEGEN_API8 → 9; arms 1–8 remain accepted (old generated source declares its own preamble and needs no removed runtime surface). Diagnostic, compat test, snapshots, README, anddocs/migration.mdupdated.TerminalNode/ErrorNodegained a hiddennode()accessor because the emittedVisitableimpls previously read the module-local private__nodefield, which a runtime-owned type no longer permits.update-generated.sh --update, g4 via theupdate-stage0.shtwo-stage bootstrap, which converged and passed the pinned corpus).Measurements (acceptance criteria)
Generated source, per parser: −272 lines / −7,014 bytes
toml_parser.rsantlr_v4_parser.rsrust_parser.rsBinary size (
antlr4-rust-gen, which links all three generated parsers; default release profile, no LTO, x86-64 Linux): unstripped 43,281,296 → 43,298,288 (+0.04%); stripped 35,117,720 → 35,131,464 (+0.04%) — size-neutral. Symbol-level diff confirms the per-parser helper instantiations (e.g. three__context_childrenFromFninstances) collapse to one runtime instance; the small residual is scattered codegen-unit drift. Without the#[inline]hints the delta was +47 KB, so the hints are kept.Testing
cargo fmt --check(runtime + codegen)cargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo test --locked --workspace --all-featurestools/toml-syntax/update-generated.sh --checktools/rust-syntax/update-generated.sh --checktools/grammar-frontend/update-stage0.sh --checkNote for reviewers: the coherence constraint above is why
ValidatedTreeContextappears in generated output while its former neighbors don't — that split is intentional, not an oversight.Summary by CodeRabbit
New Features
Compatibility