[codex] Expose parser state from generated parse helper - #50
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe generator's Changesparse_with_parser generated API
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Copy/Paste DetectionFound 9 duplication(s) across 1 changed Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 70 line (285 tokens) duplication in the following files:
let size = ch.len_utf8();
if line_comment {
line_comment = ch != '\n';
index += size;
continue;
}
if block_comment {
if source.as_bytes().get(index..index + 2) == Some(b"*/") {
block_comment = false;
index += 2;
} else {
index += size;
}
continue;
}
if char_set {
match ch {
_ if escaped => escaped = false,
'\\' => escaped = true,
']' => char_set = false,
_ => {}
}
index += size;
continue;
}
if escaped {
escaped = false;
index += size;
continue;
}
if single_quoted {
match ch {
'\\' => escaped = true,
'\'' => single_quoted = false,
_ => {}
}
index += size;
continue;
}
if double_quoted {
match ch {
'\\' => escaped = true,
'"' => double_quoted = false,
_ => {}
}
index += size;
continue;
}
match ch {
'/' if source.as_bytes().get(index..index + 2) == Some(b"//") => {
line_comment = true;
index += 2;
}
'/' if source.as_bytes().get(index..index + 2) == Some(b"/*") => {
block_comment = true;
index += 2;
}
'\'' => {
single_quoted = true;
index += size;
}
'"' => {
double_quoted = true;
index += size;
}
'[' => {
char_set = true;
index += size;
}
'{' => return Some(index),Found a 26 line (168 tokens) duplication in the following files:
atn.add_state(AtnState::new(5, AtnStateKind::RuleStop).with_rule_index(0));
atn.state_mut(0)
.expect("state 0")
.add_transition(Transition::Epsilon { target: 1 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 2 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 3 });
atn.state_mut(2)
.expect("state 2")
.add_transition(Transition::Atom {
target: 4,
label: 1,
});
atn.state_mut(3)
.expect("state 3")
.add_transition(Transition::Atom {
target: 4,
label: 2,
});
atn.state_mut(4)
.expect("state 4")
.add_transition(Transition::Epsilon { target: 5 });
atn.add_decision_state(1);Found a 43 line (157 tokens) duplication in the following files:
)
.expect("writing to a string cannot fail");
// Capture the rule start AFTER `enter_rule`, which advances the cursor past any
// leading hidden-channel tokens to the first visible token. Capturing before
// would make `$start`/`$text` in generated actions include a leading hidden
// prefix (e.g. whitespace), diverging from ANTLR and the rule context start.
writeln!(
out,
" let __rule_start = antlr4_runtime::IntStream::index(self.base.input());"
)
.expect("writing to a string cannot fail");
// Member-setting `@init` runs on rule entry (before the body) so same-rule
// predicates and actions observe the state it sets.
render_generated_init_action_entry(
out,
index,
step_render_context.init_entry_action_statements,
2,
);
// Queue the `@init` action event before the body steps so the buffered replay
// (`run_generated_action`) runs it ahead of body actions, matching ANTLR's
// "init before body" order. It sits after `__generated_action_marker`, so a
// fatal-sync abort that truncates back to the marker discards it too.
render_generated_init_action(out, index, entry_state, init_action_statements, 2);
writeln!(out, " let mut __consumed_eof = false;")
.expect("writing to a string cannot fail");
writeln!(
out,
" let mut __sync_error: Option<antlr4_runtime::AntlrError> = None;"
)
.expect("writing to a string cannot fail");
writeln!(
out,
" let __result = (|| -> Result<(), antlr4_runtime::AntlrError> {{"
)
.expect("writing to a string cannot fail");
render_generated_steps(out, &rule.steps, 3, step_render_context);
writeln!(out, " Ok(())").expect("writing to a string cannot fail");
writeln!(out, " }})();").expect("writing to a string cannot fail");
writeln!(out, " match __result {{").expect("writing to a string cannot fail");
writeln!(out, " Ok(()) => {{").expect("writing to a string cannot fail");
writeln!(
out,Found a 18 line (134 tokens) duplication in the following files:
atn.add_state(AtnState::new(4, AtnStateKind::BlockEnd).with_rule_index(0));
atn.add_state(AtnState::new(5, AtnStateKind::RuleStop).with_rule_index(0));
atn.state_mut(0)
.expect("state 0")
.add_transition(Transition::Epsilon { target: 1 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 2 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 3 });
atn.state_mut(2)
.expect("state 2")
.add_transition(Transition::Atom {
target: 4,
label: 1,
});
atn.state_mut(3)Found a 17 line (117 tokens) duplication in the following files:
atn.add_state(AtnState::new(5, AtnStateKind::RuleStop).with_rule_index(0));
atn.state_mut(0)
.expect("state 0")
.add_transition(Transition::Epsilon { target: 1 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 2 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 3 });
atn.state_mut(2)
.expect("state 2")
.add_transition(Transition::Atom {
target: 4,
label: 1,
});
atn.state_mut(4)Found a 20 line (109 tokens) duplication in the following files:
[GeneratedParserStep::Decision {
state: 1,
decision: 0,
track_alt_number: true,
allow_semantic_context: false,
force_context: false,
fast_path: Some(GeneratedDecisionFastPath {
arms: vec![
GeneratedDecisionFastArm {
alt: 1,
intervals: vec![(1, 1)],
},
GeneratedDecisionFastArm {
alt: 2,
intervals: vec![(2, 2)],
},
],
}),
alts: vec![vec![mt(1, 4)], vec![mt(2, 4)]],
}]Found a 13 line (105 tokens) duplication in the following files:
fn compile_generated_parser_star_loop(
context: &GeneratedParserCompileContext<'_>,
state: &antlr4_runtime::atn::AtnState,
decision: usize,
stop_state: usize,
visited: &mut BTreeSet<usize>,
) -> Option<Vec<GeneratedParserStep>> {
let mut enter = None;
let mut exit = None;
for (index, transition) in state.transitions.iter().enumerate() {
let alt = index + 1;
let target = transition.target();
let target_state = context.atn.state(target)?;Found a 17 line (105 tokens) duplication in the following files:
}
ActionTemplate::Noop
| ActionTemplate::Text { .. }
| ActionTemplate::TextWithPrefix { .. }
| ActionTemplate::RuleTextWithPrefix { .. }
| ActionTemplate::StringTree { .. }
| ActionTemplate::RuleInvocationStack { .. }
| ActionTemplate::ListenerWalk { .. }
| ActionTemplate::RuleValue { .. }
| ActionTemplate::RuleReturnValue { .. }
| ActionTemplate::SetIntReturn { .. }
| ActionTemplate::TokenText { .. }
| ActionTemplate::TokenTextWithPrefix { .. }
| ActionTemplate::TokenDisplay { .. }
| ActionTemplate::ExpectedTokenNames { .. }
| ActionTemplate::Literal { .. }
| ActionTemplate::MemberValue { .. }Found a 35 line (103 tokens) duplication in the following files:
writeln!(out, " self.base.exit_rule();")
.expect("writing to a string cannot fail");
writeln!(
out,
" self.generated_actions.truncate(__generated_action_marker);"
)
.expect("writing to a string cannot fail");
writeln!(
out,
" self.base.restore_int_members(__generated_member_checkpoint);"
)
.expect("writing to a string cannot fail");
writeln!(
out,
" self.base.restore_generated_diagnostics(__generated_diagnostic_marker);"
)
.expect("writing to a string cannot fail");
writeln!(
out,
" self.base.record_generated_syntax_error();"
)
.expect("writing to a string cannot fail");
writeln!(
out,
" return Err(GeneratedRuleError::Fatal(__error));"
)
.expect("writing to a string cannot fail");
writeln!(out, " }}").expect("writing to a string cannot fail");
writeln!(
out,
" self.base.recover_generated_rule(&mut __ctx, atn(), __error);"
)
.expect("writing to a string cannot fail");
writeln!(
out, |
There was a problem hiding this comment.
Code Review
This pull request introduces a new parse_with_parser helper function and a ParseOutput struct to the generated ANTLR4 Rust parser, allowing callers to access the parser state (such as syntax diagnostics or the token stream) after running the entry rule. The documentation and tests have been updated to reflect this change. The feedback suggests removing the redundant 'where L: TokenSource' trait bounds on the generated parse and parse_with_parser functions to simplify the generated code, as the compiler can infer this constraint automatically.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub fn parse_with_parser<L, R>( | ||
| input: impl AsRef<str>, | ||
| lexer: impl FnOnce(antlr4_runtime::InputStream) -> L, | ||
| entry: impl FnOnce(&mut {type_name}<L>) -> Result<R, antlr4_runtime::AntlrError>, | ||
| ) -> Result<ParseOutput<R, L>, antlr4_runtime::AntlrError> | ||
| where | ||
| L: TokenSource, |
There was a problem hiding this comment.
The where L: TokenSource clause is redundant here. The compiler can infer this constraint from the entry closure's argument type &mut {type_name}<L> and the return type Result<ParseOutput<R, L>, ...>, as both {type_name} and ParseOutput have a where L: TokenSource bound. Removing the explicit where clause would make the generated code slightly cleaner.
This also applies to the parse function.
| pub fn parse_with_parser<L, R>( | |
| input: impl AsRef<str>, | |
| lexer: impl FnOnce(antlr4_runtime::InputStream) -> L, | |
| entry: impl FnOnce(&mut {type_name}<L>) -> Result<R, antlr4_runtime::AntlrError>, | |
| ) -> Result<ParseOutput<R, L>, antlr4_runtime::AntlrError> | |
| where | |
| L: TokenSource, | |
| pub fn parse_with_parser<L, R>( | |
| input: impl AsRef<str>, | |
| lexer: impl FnOnce(antlr4_runtime::InputStream) -> L, | |
| entry: impl FnOnce(&mut {type_name}<L>) -> Result<R, antlr4_runtime::AntlrError>, | |
| ) -> Result<ParseOutput<R, L>, antlr4_runtime::AntlrError> |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b093882d51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Keeps the generated parser available after the entry rule runs so callers | ||
| /// can inspect diagnostics or recover the parser-owned token stream. | ||
| #[derive(Debug)] | ||
| pub struct ParseOutput<R, L> |
There was a problem hiding this comment.
Avoid a fixed helper type name collision
For grammars whose parser name renders to ParseOutput (for example --parser-name ParseOutput or a ParseOutput.interp input), this emits a public helper struct named ParseOutput and then later emits the generated parser struct with the same name in the same module, so the generated parser fails to compile with a duplicate type definition. Please make the helper name collision-proof relative to the generated parser type.
Useful? React with 👍 / 👎.
Summary
parse_with_parser(...)so callers can keep parser state after the entry rule runsparse(...)API intact by delegating through the new helper and returning only the parse resultReview follow-up
KotlinParserParseOutput, so a parser namedParseOutputcannot collide with the helper typewhere L: TokenSourceclauses with inlineL: TokenSourcebounds; the bound is still required for generated code to compileValidation
cargo +1.95.0 test --lockedcargo +1.95.0 clippy --locked --all-targets --all-features -- -D warningsrustfmt +1.95.0 --edition 2024 --check src/bin/antlr4-rust-gen.rsgit diff --checkRUSTUP_TOOLCHAIN=1.95.0 tests/kotlin-parity/run.sh --antlr-jar /tmp/antlr-cleanroom/tools/antlr-4.13.2-complete.jar --grammars-v4 /tmp/antlr-cleanroom/grammars-v4 --python /tmp/antlr-cleanroom/venv/bin/pythonCloses #49
Summary by CodeRabbit
New Features
parse_with_parserfunction to access parser state, error counts, and token streams after parsing.Documentation