Beat Go on generated Solidity parser performance#77
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe parser generator now translates portable boolean locals, including declarations, assignments, predicates, failure messages, and literal rule arguments, across rule rendering and semantic prediction. Embedded rule parsing records local names and handles Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 12 duplication(s) across 3 changed Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 27 line (145 tokens) duplication in the following files:
fn generated_match_token_recovers_missing_token_from_context_follow() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'")],
[None, Some("X"), Some("Y")],
[None::<&str>, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![CommonToken::eof("parser-test", 3, 1, 3)],
index: 0,
}),
data,
);
parser.rule_context_stack = vec![
RuleContextFrame {
rule_index: 0,
invoking_state: 0,
},
RuleContextFrame {
rule_index: 1,
invoking_state: 1,
},
];Found a 30 line (120 tokens) duplication in the following files:
let boundary = left_recursive_boundary(atn, state, *target);
outcomes.extend(
self.recognize_state_fast(
atn,
FastRecognizeRequest {
state_number: *target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
precedence,
depth: depth + 1,
recovery_symbols: Rc::clone(&epsilon_recovery_symbols),
recovery_state: epsilon_recovery_state,
},
visiting,
memo,
expected,
)
.into_iter()
.map(|mut outcome| {
if let Some(rule_index) = boundary {
outcome.nodes.prepend(Rc::new(
FastRecognizedNode::LeftRecursiveBoundary { rule_index },
));
}
outcome
}),
);
}Found a 33 line (115 tokens) duplication in the following files:
outcomes.extend(
self.recognize_state(
atn,
RecognizeRequest {
state_number: *target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
init_action_rules,
predicates,
semantics,
rule_args,
member_actions,
return_actions,
local_int_arg,
member_values: member_values.clone(),
return_values: return_values.clone(),
rule_alt_number: next_alt_number,
track_alt_numbers,
consumed_eof,
precedence,
depth: depth + 1,
recovery_symbols: epsilon_recovery_symbols.clone(),
recovery_state: epsilon_recovery_state,
},
visiting,
memo,
expected,
)
.into_iter()
.map(|mut outcome| {
prepend_decision(&mut outcome, decision);Found a 15 line (114 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
CommonToken::new(1).with_text("x"),
CommonToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
assert_eq!(
parser.match_token(1).expect("token 1 should match").text(),Found a 15 line (113 tokens) duplication in the following files:
fn generated_match_token_counts_single_token_deletion_recovery() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'"), Some("'Z'")],
[None, Some("X"), Some("Y"), Some("Z")],
[None::<&str>, None, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![
CommonToken::new(3).with_text("z"),
CommonToken::new(2).with_text("y"),Found a 20 line (111 tokens) duplication in the following files:
FastRecognizedNode::Rule {
rule_index,
invoking_state,
start_index,
stop_index,
children,
} => {
let mut context = ParserRuleContext::with_child_capacity(
*rule_index,
*invoking_state,
children.len(),
);
if let Some(token) = self.token_ref_at(*start_index) {
context.set_start_ref(token);
}
if let Some(token) = stop_index.and_then(|index| self.token_ref_at(index)) {
context.set_stop_ref(token);
}
if children.has_left_recursive_boundary() {
let folded = fold_fast_left_recursive_boundaries(children.to_vec());Found a 14 line (110 tokens) duplication in the following files:
fn outcome_ties_keep_later_non_recursive_alternative() {
let first = RecognizeOutcome {
index: 1,
consumed_eof: false,
alt_number: 0,
member_values: BTreeMap::new(),
return_values: BTreeMap::new(),
diagnostics: Vec::new(),
decisions: Vec::new(),
actions: vec![ParserAction::new(1, 0, 0, None)],
nodes: vec![RecognizedNode::Token { index: 0 }],
};
let second = RecognizeOutcome {
actions: vec![ParserAction::new(2, 0, 0, None)],Found a 13 line (109 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
CommonToken::new(1).with_text("x"),
CommonToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);Found a 20 line (107 tokens) duplication in the following files:
let start_state = atn
.rule_to_start_state()
.get(rule_index)
.copied()
.ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no start state"))
})?;
let stop_state = atn
.rule_to_stop_state()
.get(rule_index)
.copied()
.filter(|state| *state != usize::MAX)
.ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no stop state"))
})?;
let start_index = self.current_visible_index();
self.clear_prediction_diagnostics();
self.reset_per_parse_caches();
let caller_follow_state = self.pending_invoking_follow_state(atn);Found a 15 line (107 tokens) duplication in the following files:
FastRecognizedNode::MissingToken {
token_type,
at_index,
text,
} => {
let current = self.token_at(*at_index);
let token = CommonToken::new(*token_type)
.with_text(text.as_str())
.with_span(usize::MAX, usize::MAX)
.with_position(
current.as_ref().map(Token::line).unwrap_or_default(),
current.as_ref().map(Token::column).unwrap_or_default(),
);
Ok(ParseTree::Error(ErrorNode::new(token)))
}Found a 21 line (102 tokens) duplication in the following files:
) -> Option<RecognizeOutcome> {
let (error_index, message) = self.expected_error_message(rule_index, start_index, expected);
let token = self.token_at(error_index);
let mut next_index = error_index;
loop {
let symbol = self.token_type_at(next_index);
if sync_symbols.contains(&symbol) {
if next_index == error_index {
return None;
}
break;
}
if symbol == TOKEN_EOF {
break;
}
let after = self.consume_index(next_index, symbol);
if after == next_index {
break;
}
next_index = after;
}Found a 12 line (102 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.set_rule_to_start_state(vec![0]);
atn.set_rule_to_stop_state(vec![5]);
atn.add_decision_state(1);
atn.state_mut(0)
.expect("state 0")
.add_transition(Transition::Epsilon { target: 1 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Atom {
target: 2, |
|
@codex review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for portable raw-grammar boolean locals, allowing actions and predicates to be translated without embedded mode. It also optimizes GeneratedMatch by avoiding vector allocations for single-child matches and refactors left-recursive loop prediction to handle ambiguous lookahead cases more robustly. Feedback on the changes suggests using safe accessors like .get_mut() instead of direct indexing on out.declarations to prevent potential runtime panics.
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 700aea67f5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 700aea67f5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92b4392ab2
ℹ️ 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".
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bin/antlr4-rust-gen.rs (1)
6846-6960: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep rules with portable-local semantics on the generated path.
Portable declarations, assignments, and predicates exist only in generated methods, but affected rules are not required to generate and may also be marked ATN-preferred, which routes them through interpretation in normal mode. Either path silently skips these semantics while the inventory reports them as
Translated.Track the owning rule indices, reject unavailable generated rules, and exclude them from interpreted/ATN-preferred routing.
As per coding guidelines, keep generic code-generation paths grammar-agnostic and derive behavior from ANTLR metadata and ATN structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bin/antlr4-rust-gen.rs` around lines 6846 - 6960, Update the rule-generation flow around portable_local_data and generated_rules to track owning rule indices for portable declarations, assignments, and predicates from ANTLR metadata/ATN structure. Require those rules to have generated implementations, reject generation when any required rule is unavailable, and exclude them from interpreted or ATN-preferred routing so all portable-local semantics execute on the generated path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bin_support/embedded.rs`:
- Around line 1235-1255: Update parses_raw_default_valued_rule_locals to also
assert the expected local_names values produced by parse_embedded_rules_model,
matching the names consumed by build_portable_local_data. Keep the existing
attrs assertion and verify both locals are present in the correct order.
- Around line 184-196: Update strip_default_initializer to split on the
declaration’s initializer assignment rather than the final equals sign,
preserving expressions containing == such as other == false and other == 0.
Ensure only raw initializers whose trimmed value is exactly false or 0 are
stripped, and add regression cases covering both comparison expressions.
In `@src/bin/antlr4-rust-gen.rs`:
- Around line 8870-8875: Update the boolean argument recognition around
RuleArgTemplate::Literal to use the grammar-aware cursor rather than raw
substring matching, accepting only actual rule references and excluding comments
and string literals. Add regression coverage for comment and string cases,
including /* flag[true] */ flag[false], while preserving valid boolean rule
arguments.
In `@src/parser.rs`:
- Around line 2360-2418: Update state_before_stop_lookahead_inner and related
cached lookahead logic to model Transition::Rule calls using the callee target,
nullability, and follow_state, resuming at the follow state when the child can
return without consuming input. Handle RuleStop states by unwinding through the
caller PredictionContext into parent return paths, deriving behavior from ATN
metadata rather than generic epsilon traversal. Add regressions covering
nullable child calls and nullable caller-to-parent unwinding, ensuring overlap
is not cached as false and recursive operators do not consume caller-path
tokens.
---
Outside diff comments:
In `@src/bin/antlr4-rust-gen.rs`:
- Around line 6846-6960: Update the rule-generation flow around
portable_local_data and generated_rules to track owning rule indices for
portable declarations, assignments, and predicates from ANTLR metadata/ATN
structure. Require those rules to have generated implementations, reject
generation when any required rule is unavailable, and exclude them from
interpreted or ATN-preferred routing so all portable-local semantics execute on
the generated path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a115bfe-9d77-4c11-86a0-57c5628ff5c6
📒 Files selected for processing (4)
README.mdsrc/bin/antlr4-rust-gen.rssrc/bin_support/embedded.rssrc/parser.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92b4392ab2
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63095fbdbc
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27a5361b07
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
tinovyatkin
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (1)
435-437: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the benchmark settings explicit The text says 10 measured iterations after 2 warmups, but
--quickruns only 3 iterations and 1 warmup. Use--iters 10 --warmups 2here, or update the figures to match the quick preset.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 435 - 437, Update the benchmark documentation near the warm-parse figures to make its settings match the stated measurements: explicitly show --iters 10 --warmups 2, or revise the figures and wording to reflect the --quick preset’s 3 iterations and 1 warmup.src/parser.rs (1)
2263-2305: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftPreserve child-rule precedence during operator reachability.
The traversal reuses the caller’s
precedenceand keysvisitedonly by state, althoughTransition::Ruleinvokes its target with a separate precedence argument. Reaching the same nullable or left-recursive child under different arguments can therefore cache an incorrect operator FIRST result and make loop prediction enter or exit incorrectly.Thread
rule_precedenceinto the child traversal, retain the caller precedence forfollow_state, key visits by(state_number, precedence), and use precedence-aware child nullability.Proposed direction
- visited: &mut BTreeSet<usize>, + visited: &mut BTreeSet<(usize, i32)>, ) -> bool { - if !visited.insert(state_number) { + if !visited.insert((state_number, precedence)) { return false; } ... Transition::Rule { target, rule_index, follow_state, - .. + precedence: rule_precedence, } => { ... - precedence, + *rule_precedence, ... - if rule_first_set(atn, *target, child_stop, first_set_cache).nullable + if state_can_reach_stop_with_precedence( + atn, + *target, + child_stop, + *rule_precedence, + first_set_cache, + )As per coding guidelines, generic runtime prediction must derive behavior from ANTLR metadata and ATN structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser.rs` around lines 2263 - 2305, Update state_can_reach_symbol_with_precedence to track visited states by both state_number and precedence, preventing results from different precedence contexts from being reused. In each Transition::Rule branch, pass rule_precedence to the target traversal, keep the caller’s precedence for follow_state traversal, and use the precedence-aware child nullability check when deciding whether to continue through the rule.Source: Coding guidelines
src/bin/antlr4-rust-gen.rs (1)
4890-5120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a targeted test for portable-aware alt selection inside a decision/loop.
All the shown
render_generated_decision/render_generated_star_looptests (e.g.generated_loop_filters_failed_leading_predicate_to_exit_alt,semantic_alt_search_orders_unresolved_alts_last) passportable_locals: None; onlytranslates_portable_boolean_local_semanticsexercises portable locals, and only for a single non-looped predicate. The new portable-awaresemantic_alt_candidate_condition(_with_la)/loop_entry_conditionlogic for multi-alt decisions and loops isn't directly covered.Consider adding a test mirroring
generated_loop_filters_failed_leading_predicate_to_exit_altbut withportable_locals: Some(...)supplying a portable predicate condition, to lock in that the portable boolean expression (not a runtime/embedded predicate) is used for alt-candidate filtering in that context.Also applies to: 5280-5720
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bin/antlr4-rust-gen.rs` around lines 4890 - 5120, Add a focused test alongside generated_loop_filters_failed_leading_predicate_to_exit_alt that supplies portable_locals: Some(...) and exercises a multi-alternative decision or loop. Verify generated alt-candidate filtering uses the portable predicate expression rather than embedded or runtime predicate evaluation, covering semantic_alt_candidate_condition(_with_la) and loop_entry_condition behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 428: Update the README performance claim beginning “Rust is faster than
Go on every fixture” to avoid asserting per-fixture results from group-level
geometric means; state that Rust is faster on average, or provide a link to raw
per-fixture results if that claim is verified.
In `@src/parser.rs`:
- Around line 2308-2331: Update state_can_reach_symbol_with_precedence so
Transition::Predicate is not treated as unconditional epsilon reachability:
track predicate-dependent symbol paths separately and return None unless an
unconditional operator path exists, while preserving precedence filtering and
unconditional transition traversal. Ensure the fast path cannot commit through a
predicate guarded by false, allowing adaptive prediction to select the loop
exit; add a regression covering a false predicate immediately before an operator
token.
---
Outside diff comments:
In `@README.md`:
- Around line 435-437: Update the benchmark documentation near the warm-parse
figures to make its settings match the stated measurements: explicitly show
--iters 10 --warmups 2, or revise the figures and wording to reflect the --quick
preset’s 3 iterations and 1 warmup.
In `@src/bin/antlr4-rust-gen.rs`:
- Around line 4890-5120: Add a focused test alongside
generated_loop_filters_failed_leading_predicate_to_exit_alt that supplies
portable_locals: Some(...) and exercises a multi-alternative decision or loop.
Verify generated alt-candidate filtering uses the portable predicate expression
rather than embedded or runtime predicate evaluation, covering
semantic_alt_candidate_condition(_with_la) and loop_entry_condition behavior.
In `@src/parser.rs`:
- Around line 2263-2305: Update state_can_reach_symbol_with_precedence to track
visited states by both state_number and precedence, preventing results from
different precedence contexts from being reused. In each Transition::Rule
branch, pass rule_precedence to the target traversal, keep the caller’s
precedence for follow_state traversal, and use the precedence-aware child
nullability check when deciding whether to continue through the rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b38a22e-c255-472f-92ca-5812a8096ef4
📒 Files selected for processing (4)
README.mdsrc/bin/antlr4-rust-gen.rssrc/bin_support/embedded.rssrc/parser.rs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0630eb00ed
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fdb33cc90
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae637fc954
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55a3f33086
ℹ️ 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".
| .expect("writing to a string cannot fail"); | ||
| writeln!( | ||
| out, | ||
| "{pad} antlr4_runtime::ParserAtnSimulatorError::NoViableAlt {{ index, .. }} => self.base.no_viable_alternative_error_at(__decision_start, index)," |
There was a problem hiding this comment.
For non-embedded generated left-recursive loops, the None path is taken exactly when the one-token shortcut deferred to adaptive prediction because of caller-follow overlap or predicate-dependent operator lookahead. If that adaptive call reports NoViableAlt, this now emits a hard syntax error, whereas the previous generated code converted NoViableAlt into the loop exit alternative when left_recursive_loop_enter_matches was false. In those deferred cases (for example an expression followed by an overlapping caller token, or a predicate-guarded operator that is not viable), valid inputs can be rejected instead of exiting the precedence loop and letting the caller consume the token.
Useful? React with 👍 / 👎.
1 participant
Closes #76
What changed
GeneratedMatchresult inline instead of allocating aVecfor every successful token match.All runtime and codegen behavior remains grammar-agnostic; there are no Solidity-specific rule names or workarounds.
Root cause
The Solidity grammar's portable local predicates prevented complete strict generated-parser coverage. Once generated, expression-heavy left-recursive rules still paid for adaptive ATN simulation at every operator boundary, and every normal token match allocated a one-element children vector. The first optimization exposed repeated caller-follow graph walks in other left-recursive grammars, so those pure ATN results now live in the shared grammar cache rather than being recomputed per parse.
Correctness
Strict generation emits all 93 Solidity parser rules. The semantic manifest contains 60 translated and 2 synthetic coordinates with none unsupported. Parse trees are byte-identical to the Go ANTLR runtime for Ownable, ERC20, ERC721, and Governor, including the caller-follow ambiguity around
new string[]that requires full-context fallback.Solidity performance
Same official Solidity grammar, same OpenZeppelin fixtures, full CST enabled, release builds with warmups, 200 measured iterations per process on Apple M3 Pro. The table is the final complete round after three repeated rounds on the final head:
Across all three final-head complete rounds, every fixture remained above 2x; the worst observed ratio was 2.29x.
Governor scaling, 100 iterations at each size on the final head:
Governor allocation profile improves from the issue baseline of 189,489 allocations / 42.0 MB allocated / 17.6 MB peak live to 65,107 / 18.77 MB / 5.60 MB.
Artifact sizes:
-ldflags='-s -w': 4,150,706 bytesCross-language benchmark
Same-machine
origin/mainand PR runs used 10 measured iterations after two warmups. The 17-fixture comparator passed with a 1.15x maximum-regression threshold. Go/Rust ratios use the README's warm-minimum metric; current/main uses geometric mean average Rust time.The final precedence and context-boundary review fixes were also rerun over all 17 Rust fixtures and passed the same 1.15x threshold against the preceding PR head.
Validation
cargo fmt --checkcargo test --all-targets --all-featurescargo clippy --locked --all-targets --all-features -- -D warningscargo run --release --quiet --bin antlr4-runtime-testsuite(356 passed, 0 failed, 1 skipped)tools/parse-bench/compare.pyon same-protocolorigin/mainvs PR and final-head vs preceding-head reports (17results passed at1.15x)--sem-unknown error --require-generated-parser --require-full-semantics