fix: issue 65 parser recovery hang - #66
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe parser now detects observable ATN actions and predicates, allowing default runtime options to skip unnecessary action replay while preserving semantic hook behavior. Fast recognizer cycle tracking uses Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 1 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, |
![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 optimizations and fixes to the parser, including a fast-path bypass for ATN rules without observable actions or predicates, and updated cycle detection logic to handle recovery paths using a broader epsilon-state guard. It also updates the visiting set type to FastRecognizeKey. The review feedback focuses on performance improvements: caching the results of atn_has_observable_action_transitions to avoid redundant ATN traversals, and optimizing the cycle detection path by using a boolean flag instead of cloning the key multiple times.
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.
tinovyatkin
changed the title
|
@codex review |
![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: b51bea2b73
ℹ️ 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".
d20f9ce to
9f89541
Compare
1 participant
Summary
Fixes #65 by keeping default runtime-option parser fallback on the fast parser path when the ATN only contains non-observable no-op parser actions (
action_index = None). Those transitions are ANTLR metadata, not target action replay points, and forcing the older action-aware recognizer through them exposed the OpenGQL hang.The recovery cycle guard now also keys by the full fast-recognizer request when recovery can synthesize a zero-width loop-body child at EOF. That prevents a recovered empty
+loop iteration from re-entering the same loop at the same token index.Regression Coverage
s : a+ Y ; a : X ;at EOF, covering the zero-width recovered loop-body case that the conformance suite missed.action_index = Noneparser action transitions are ignored as no-op metadata and do not force action replay.Validation
cargo test --libpassed: 147 tests.cargo run --release --quiet --bin antlr4-runtime-testsuitepassed: 356 passed, 0 failed, 1 skipped.cargo clippy --locked --all-targets --all-features -- -D warningspassed.--iters 1000) showed no material regression versus detachedHEADbaseline:01-nested-types.kt: current avg 0.236ms, baseline avg 0.247ms02-dataframe.kt: current avg 1.773ms, baseline avg 1.771ms03-string-templates.kt: current avg 0.882ms, baseline avg 0.875msrustfmt --edition 2024 --check src/parser.rspassed.git diff --checkpassed.Note: repo-wide
cargo fmt --checkstill reports unrelated pre-existing formatting drift outside this change.