perf(prediction): terminate SLL conflicts by context containment - #343
Merged
Conversation
When ordinary state/context conflict detection is inconclusive, join prediction contexts per state and alternative and prove competing contexts are contained by the minimum alternative. Keep predicate and precedence-bearing sets on the established path, preserve LL diagnostic coordinates, and retain exactness in packed DFA metadata. Add focused containment, near-miss, diagnostic, and invalid-input recovery coverage. Fixes #334
Contributor
Author
|
@codex review |
Copy/Paste DetectionFound 23 duplication(s) across 4 changed non-generated Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 21 line (226 tokens) duplication in the following files:
(9, AtnStateKind::RuleStop),
] {
assert_eq!(atn.add_state(kind, Some(0)).expect("state").index(), state);
}
atn.set_left_recursive_rule(0)
.expect("left-recursive rule start");
atn.set_precedence_rule_decision(2)
.expect("precedence decision");
atn.set_loop_back_state(8, 7).expect("loop-back state");
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![9])
.expect("rule stop states");
for state in [1, 2, 3] {
atn.add_decision_state(state).expect("decision state");
}
for (source, target) in [(0, 1), (2, 3), (2, 8), (7, 2), (8, 9)] {
atn.add_transition(source, ParserTransitionSpec::Epsilon { target })
.expect("epsilon transition");
}
for (source, target, label) in [(1, 2, 1), (1, 2, 2), (4, 6, 4), (5, 6, 3), (6, 7, 1)] {
```rust
---
Found a 25 line (193 tokens) duplication in the following files:
* Starting at line 16632 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 17613 of crates/antlr-rust-runtime/src/parser.rs
```rust
let mut atn = ParserAtnBuilder::new(1);
for (state_number, kind) in [
(0, AtnStateKind::RuleStart),
(1, AtnStateKind::StarLoopEntry),
(2, AtnStateKind::Basic),
(3, AtnStateKind::Basic),
(4, AtnStateKind::StarLoopBack),
(5, AtnStateKind::LoopEnd),
(6, AtnStateKind::RuleStop),
] {
assert_eq!(
atn.add_state(kind, Some(0)).expect("state").index(),
state_number
);
}
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![6])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.set_loop_back_state(5, 4).expect("loop back state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("entry transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 })
.expect("loop body");Found 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![TestToken::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,
},
];
```rust
---
Found a 18 line (128 tokens) duplication in the following files:
* Starting at line 16298 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16323 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn epsilon_cycle_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(1);
for (state_number, kind) in [
(0, AtnStateKind::RuleStart),
(1, AtnStateKind::Basic),
(2, AtnStateKind::RuleStop),
] {
assert_eq!(
atn.add_state(kind, Some(0)).expect("state").index(),
state_number
);
}
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![2])
.expect("rule stop states");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");Found a 27 line (127 tokens) duplication in the following files:
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::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))
```rust
---
Found a 22 line (125 tokens) duplication in the following files:
* Starting at line 16779 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16851 of crates/antlr-rust-runtime/src/parser.rs
```rust
atn.add_state(AtnStateKind::BlockEnd, Some(0))
.expect("state")
.index(),
4
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStop, Some(0))
.expect("state")
.index(),
5
);
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(
1,
ParserTransitionSpec::Atom {
target: 2,Found a 15 line (124 tokens) duplication in the following files:
fn context_prediction_reports_context_sensitivity_for_dfa_conflict() {
let atn = two_token_decision_atn();
let mut simulator = ParserAtnSimulator::new(&atn);
let mut workspace = PredictionWorkspace::default();
let mut start_configs = AtnConfigSet::new();
start_configs.add(
AtnConfig::new(2, 1, EMPTY_CONTEXT, &simulator.store.contexts),
&mut simulator.store.contexts,
&mut workspace,
);
let start =
simulator.store.decision_to_dfa[0].add_state(DfaStateBuilder::new(start_configs));
simulator.store.decision_to_dfa[0].set_start_state(start);
let mut accept_configs = AtnConfigSet::new();
```rust
---
Found a 18 line (122 tokens) duplication in the following files:
* Starting at line 3907 of crates/antlr-rust-runtime/src/atn/parser.rs
* Starting at line 4058 of crates/antlr-rust-runtime/src/atn/parser.rs
```rust
atn.set_rule_to_stop_state(vec![7])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision 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: 4 })
.expect("transition");
atn.add_transition(
2,
ParserTransitionSpec::Atom {
target: 3,
label: 1,
},
)
.expect("transition");
atn.add_transition(3, ParserTransitionSpec::Epsilon { target: 6 })Found a 34 line (119 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,
committed_decision: transition_committed,
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);
```rust
---
Found a 13 line (117 tokens) duplication in the following files:
* Starting at line 145 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 172 of crates/antlr-rust-runtime/src/parser.rs
```rust
ordinary $parser:ident, $state:expr, $rule:expr, $allow_fallback:expr,
$atn:expr, $fatal:path;
retry [$($retry:tt)*];
bind ($ctx:ident, $rule_start:ident, $consumed_eof:ident, $sync_error:ident);
setup { $($setup:tt)* }
body { $($body:tt)* }
success { $($success:tt)* }
recovery { $($recovery:tt)* }
) => {
$crate::__antlr4_rust_generated_rule! {
@body
parser $parser;
enter $parser.base.enter_rule($state, $rule);Found a 24 line (113 tokens) duplication in the following files:
impl IntStream for LookaheadIntStream {
fn consume(&mut self) {
if self.la(1) != TOKEN_EOF {
self.index += 1;
}
}
fn la(&mut self, offset: isize) -> i32 {
if offset <= 0 {
return 0;
}
let offset = offset.cast_unsigned() - 1;
self.symbols
.get(self.index + offset)
.copied()
.unwrap_or(TOKEN_EOF)
}
fn index(&self) -> usize {
self.index
}
fn seek(&mut self, index: usize) {
self.index = index.min(self.symbols.len());
```rust
---
Found a 15 line (113 tokens) duplication in the following files:
* Starting at line 18193 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 18569 of crates/antlr-rust-runtime/src/parser.rs
```rust
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![
TestToken::new(3).with_text("z"),
TestToken::new(2).with_text("y"),Found a 18 line (112 tokens) duplication in the following files:
(4, AtnStateKind::Basic, 0),
(5, AtnStateKind::RuleStop, 0),
(6, AtnStateKind::RuleStart, 1),
(7, AtnStateKind::Basic, 1),
(8, AtnStateKind::RuleStop, 1),
] {
assert_eq!(
atn.add_state(kind, Some(rule_index))
.expect("state")
.index(),
state_number
);
}
atn.set_rule_to_start_state(vec![0, 6])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5, 8])
.expect("rule stop states");
atn.add_decision_state(2).expect("decision state");
```rust
---
Found a 12 line (112 tokens) duplication in the following files:
* Starting at line 15351 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 15434 of crates/antlr-rust-runtime/src/parser.rs
```rust
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),Found a 22 line (112 tokens) duplication in the following files:
fn predicate_after_token_atn() -> Atn {
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::Basic, 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))
```rust
---
Found a 22 line (111 tokens) duplication in the following files:
* Starting at line 15173 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 17299 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 17730 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn left_recursive_loop_with_caller_follow_atn(caller_symbol: i32) -> Atn {
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::Basic, 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::RuleStart, Some(1))Found a 14 line (110 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::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);
let matched = parser.match_token(1).expect("token 1 should match");
```rust
---
Found a 13 line (109 tokens) duplication in the following files:
* Starting at line 17863 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 21997 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::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 22 line (108 tokens) duplication in the following files:
) -> Option<RecognizeOutcome> {
let (error_index, message) = self.expected_error_message(rule_index, start_index, expected);
let diagnostic = diagnostic_for_token(self.token_at(error_index), message);
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;
}
let mut nodes = NodeSeqId::EMPTY;
```rust
---
Found a 15 line (108 tokens) duplication in the following files:
* Starting at line 22268 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 22292 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn outcome_ties_keep_later_non_recursive_alternative() {
let arena = RecognitionArena::default();
let first = RecognizeOutcome {
index: 1,
consumed_eof: false,
alt_number: 0,
member_values: MemberEnv::new(),
return_values: BTreeMap::new(),
diagnostics: DiagnosticSeqId::EMPTY,
decisions: Vec::new(),
actions: vec![ParserAction::new(1, 0, 0, None)],
nodes: NodeSeqId::EMPTY,
};
let second = RecognizeOutcome {
actions: vec![ParserAction::new(2, 0, 0, None)],Found a 17 line (107 tokens) duplication in the following files:
let report_unrecovered_error = self.is_top_level_entry();
let start_state = atn.rule_to_start_state().get(rule_index).ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no start state"))
})?;
let stop_state = atn
.rule_to_stop_state()
.get(rule_index)
.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();
self.reset_recognition_arena();
let caller_follow_state = self.pending_invoking_follow_state(atn);
```rust
---
Found a 13 line (104 tokens) duplication in the following files:
* Starting at line 15286 of crates/antlr-rust-runtime/src/parser.rs
* Starting at line 16690 of crates/antlr-rust-runtime/src/parser.rs
```rust
fn labeled_left_recursive_operator_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(4);
for (state, kind) in [
(0, AtnStateKind::RuleStart),
(1, AtnStateKind::BlockStart),
(2, AtnStateKind::StarLoopEntry),
(3, AtnStateKind::StarBlockStart),
(4, AtnStateKind::Basic),
(5, AtnStateKind::Basic),
(6, AtnStateKind::Basic),
(7, AtnStateKind::StarLoopBack),
(8, AtnStateKind::LoopEnd),
(9, AtnStateKind::RuleStop),Found a 13 line (100 tokens) duplication in the following files:
let mut expected = BTreeSet::new();
for index in (1..self.rule_context_stack.len()).rev() {
let invoking_state = self.rule_context_stack[index].invoking_state;
let Ok(state_number) = usize::try_from(invoking_state) else {
continue;
};
let Some(Transition::Rule { follow_state, .. }) = atn
.state(state_number)
.and_then(|state| state.transitions().first())
.map(ParserTransition::data)
else {
continue;
};
```rust |
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe parser runtime adds exact and context-containment SLL conflict classification. DFA states preserve this metadata. Full-context diagnostics use containment information to adjust stop indices. Tests cover early termination, recovery, diagnostics, and prediction-context analysis. ChangesSLL conflict handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ParserATNSimulator
participant exact_context_sll_conflict
participant ParserDfa
participant full_context_retry_prediction
ParserATNSimulator->>exact_context_sll_conflict: Analyze SLL configurations
exact_context_sll_conflict-->>ParserATNSimulator: Return conflict alternatives and metadata
ParserATNSimulator->>ParserDfa: Store conflict flags on accept state
ParserDfa-->>full_context_retry_prediction: Provide containment metadata
full_context_retry_prediction-->>ParserATNSimulator: Report full-context diagnostic stop index
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 Code is working… I'll analyze this and get back to you. |
Contributor
![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: f4126d9fec
ℹ️ 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".
![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
🤖 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 `@crates/antlr-rust-runtime/src/atn/parser.rs`:
- Around line 2814-2841: Rename
adaptive_predict_stops_at_an_exact_context_containment_conflict to
adaptive_predict_stops_at_a_context_containment_conflict to match the non-exact
assertion. Add a test covering context_containment_conflict through the
full-context memo path by disabling exact-ambiguity detection, running the same
decision twice, and comparing the memoized retry’s diagnostic result with the
fresh decision.
- Around line 1347-1351: Add an inline comment immediately above the conditional
assignment to `sll_stop_index` explaining that containment ends the SLL walk
earlier than the reference implementation, so replacing the recorded value with
`stop_index` restores the reference diagnostic coordinate.
In `@crates/antlr-rust-runtime/src/parser.rs`:
- Around line 17432-17535: Extract the shared 11-state containment ATN
construction into a reachable builder accepting the block-end-state option and
state 6 transition as parameters. In
crates/antlr-rust-runtime/src/parser.rs#L17432-17535, replace
context_containment_recovery_atn with a call requesting the block end state and
Atom transition labeled TOKEN_EOF; in
crates/antlr-rust-runtime/src/atn/parser.rs#L3758-3853, replace
context_containment_decision_atn with a call requesting no block end state and
an Epsilon transition. Ensure both fixtures use the same shared construction for
all unchanged states and transitions.
In `@crates/antlr-rust-runtime/src/prediction.rs`:
- Around line 1399-1479: Add targeted tests for exact_context_sll_conflict using
two distinct non-stop states so multiple state groups are processed: verify None
when a later group lacks min_alt, verify exact becomes false when groups have
different alternative sets, and verify exact becomes false when
dips_into_outer_context is set. Keep existing assertions for successful
conflicts and non-contained-context declines unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d4a9084f-e261-4e2a-bc3b-da93d3936846
⛔ Files ignored due to path filters (1)
crates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__committed_sll_containment_conflict_preserves_token_deletion_recovery.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
crates/antlr-rust-runtime/src/atn/parser.rscrates/antlr-rust-runtime/src/dfa.rscrates/antlr-rust-runtime/src/parser.rscrates/antlr-rust-runtime/src/prediction.rs
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Author
|
@codex review |
Share the containment ATN fixture across simulator and committed-parser tests, cover memoized full-context diagnostics and multi-state decline/exactness branches, and document the reference diagnostic coordinate adjustment.
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| crates/antlr-rust-runtime/src/parser.rs | 2446 (main: 2434) 🔴 | 1614 (main: 1611) 🔴 | 785 (main: 778) 🔴 | 5475 (main: 5439) 🔴 | 0 ⚪ |
| crates/antlr-rust-runtime/src/atn/parser.rs | 438 (main: 415) 🔴 | 309 (main: 298) 🔴 | 142 (main: 136) 🔴 | 1262 (main: 1173) 🔴 | 0 ⚪ |
| crates/antlr-rust-runtime/src/prediction.rs | 328 (main: 301) 🔴 | 217 (main: 186) 🔴 | 126 (main: 119) 🔴 | 727 (main: 629) 🔴 | 0 ⚪ |
| crates/antlr-rust-runtime/src/dfa.rs | 207 (main: 201) 🔴 | 80 (main: 78) 🔴 | 98 (main: 94) 🔴 | 377 (main: 369) 🔴 | 0 ⚪ |
Generated by mehen v1.8.1 — the code quality watcher.
Treat exact SLL conflicts as terminal exact ambiguities instead of always retrying full-context prediction. This gives the retained exactness metadata its intended runtime role and avoids unnecessary LL work. Rename the SLL containment regression to state explicitly that earlier commitment enables token-deletion recovery and can change the resulting tree and error text. Use named snapshots for structured conflict values. The pinned optimized fork computes exactness from each alternative's joined context; retain that behavior.
Contributor
Author
|
Addressed the review follow-ups on
Validation on the updated tree:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76be7e436f
ℹ️ 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".
Exact local conflicts can skip full-context prediction. Keep generated prediction diagnostics silent in SLL mode so the parser never reports an attempting-full-context event when no LL retry occurred.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5134dc1337
ℹ️ 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".
When an exact local conflict terminates prediction without an LL retry, emit the exact ambiguity diagnostic without a preceding attempting-full-context event. Keep SLL mode silent and cover both diagnostic paths.
Contributor
Author
|
@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". |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/antlr-rust-runtime/src/atn/parser.rs (1)
1359-1366: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a contrast test for the non-containment branch of this override.
This override applies only when
context_containment_conflictis true. No test exercises theelsebranch: an ordinary (non-containment) SLL conflict that resolves as an exact ambiguity through the LL retry. Add a test using a non-containment ambiguous ATN withset_exact_ambig_detection(true)that assertssll_stop_indexis not forced to equalll_stop_indexthere. This guards the branch condition that distinguishes containment conflicts from ordinary ones.🤖 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 `@crates/antlr-rust-runtime/src/atn/parser.rs` around lines 1359 - 1366, Add a contrast test for the parser prediction path around the sll_stop_index override, constructing a non-containment ambiguous ATN with set_exact_ambig_detection(true) so the LL retry resolves an ordinary exact ambiguity. Assert that the non-containment branch preserves a distinct sll_stop_index rather than forcing it to equal ll_stop_index, while leaving the containment behavior unchanged.
🤖 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 `@crates/antlr-rust-runtime/src/parser.rs`:
- Around line 5704-5715: Add a test adjacent to
local_exact_conflict_reports_ambiguity_without_full_context_attempt that
constructs an ambiguity diagnostic with requires_full_context true, kind
Ambiguity, and exact true. Assert the LL-retried case emits both the
reportAttemptingFullContext diagnostic and the ambiguity diagnostic, covering
the local_exact_ambiguity boundary in the surrounding parser logic.
---
Outside diff comments:
In `@crates/antlr-rust-runtime/src/atn/parser.rs`:
- Around line 1359-1366: Add a contrast test for the parser prediction path
around the sll_stop_index override, constructing a non-containment ambiguous ATN
with set_exact_ambig_detection(true) so the LL retry resolves an ordinary exact
ambiguity. Assert that the non-containment branch preserves a distinct
sll_stop_index rather than forcing it to equal ll_stop_index, while leaving the
containment behavior unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47aacdfc-f648-4037-b5db-9cc539766ef1
⛔ Files ignored due to path filters (7)
crates/antlr-rust-runtime/src/atn/snapshots/antlr4_runtime__atn__parser__tests__exact_sll_conflict_skips_full_context_retry.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__committed_sll_containment_conflict_enables_token_deletion_recovery.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__parser__tests__local_exact_conflict_reports_ambiguity_without_full_context_attempt.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__prediction__tests__exact_context_conflict_joins_semantically_distinct_configs.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__prediction__tests__exact_context_conflict_marks_different_state_alt_sets_inexact.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__prediction__tests__exact_context_conflict_marks_outer_context_reach_inexact.snapis excluded by!**/*.snapcrates/antlr-rust-runtime/src/snapshots/antlr4_runtime__prediction__tests__exact_context_conflict_proves_containment_without_shared_context_ids.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
crates/antlr-rust-runtime/src/atn/parser.rscrates/antlr-rust-runtime/src/parser.rscrates/antlr-rust-runtime/src/prediction.rs
Pin the complementary LL-retried exact ambiguity path so it continues to emit both reportAttemptingFullContext and reportAmbiguity while local exact conflicts omit the attempt.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
cargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo test --locked --workspace --all-featurescargo run --release --quiet -p antlr-rust-runtime-testsuite --bin antlr4-runtime-testsuite357 passed, 0 failed, 0 skippedFixes #334
Summary by CodeRabbit
Bug Fixes
Tests