Implement semantic predicate accountability hooks - #55
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Copy/Paste DetectionFound 21 duplication(s) across 15 changed Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 30 line (252 tokens) duplication in the following files:
let mut atn = Atn::new(AtnType::Parser, 3);
add_state(&mut atn, 0, AtnStateKind::RuleStart);
add_state(&mut atn, 1, AtnStateKind::BlockStart);
add_state(&mut atn, 2, AtnStateKind::Basic);
add_state(&mut atn, 3, AtnStateKind::Basic);
add_state(&mut atn, 4, AtnStateKind::Basic);
add_state(&mut atn, 5, AtnStateKind::Basic);
add_state(&mut atn, 6, AtnStateKind::BlockEnd);
add_state(&mut atn, 7, AtnStateKind::RuleStop);
atn.set_rule_to_start_state(vec![0]);
atn.set_rule_to_stop_state(vec![7]);
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::Epsilon { target: 2 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 4 });
atn.state_mut(2)
.expect("state 2")
.add_transition(Transition::Atom {
target: 3,
label: 1,
});
atn.state_mut(3)
.expect("state 3")
.add_transition(Transition::Atom {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![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 24 line (134 tokens) duplication in the following files:
let atn = AtnDeserializer::new(&SerializedAtn::from_i32(&[
4, 0, 2, // version, lexer, max token type
9, // states
6, -1, // 0 token start
2, 0, // 1 rule 0 start
1, 0, // 2
1, 0, // 3
7, 0, // 4 rule 0 stop
2, 1, // 5 rule 1 start
1, 1, // 6
1, 1, // 7
7, 1, // 8 rule 1 stop
0, // non-greedy
0, // precedence
2, // rules
1, 1, // rule 0 starts at 1, token type 1
5, 2, // rule 1 starts at 5, token type 2
1, // modes
0, // default mode starts at 0
0, // sets
8, // edges
0, 1, 1, 0, 0, 0, // start -> rule 0
0, 5, 1, 0, 0, 0, // start -> rule 1
1, 2, 5, 'a' as i32, 0, 0, 2, 3, 5, 'b' as i32, 0, 0, 3, 4, 1, 0, 0, 0, 5, 6, 5,Found a 23 line (133 tokens) duplication in the following files:
pub fn next_token_with_hooks<I, F, A, P, E>(
lexer: &mut BaseLexer<I, F>,
atn: &Atn,
mut custom_action: A,
mut semantic_predicate: P,
mut accept_adjuster: E,
) -> CommonToken
where
I: CharStream,
F: TokenFactory,
A: FnMut(&mut BaseLexer<I, F>, LexerCustomAction),
P: FnMut(&BaseLexer<I, F>, LexerPredicate) -> bool,
E: FnMut(&mut BaseLexer<I, F>, i32, usize),
{
next_token_with_hooks_impl(
lexer,
atn,
&mut custom_action,
&mut semantic_predicate,
&mut accept_adjuster,
LexerMatchStrategy {
compiled: None,
use_cache: false,Found a 20 line (131 tokens) duplication in the following files:
atn.set_rule_to_stop_state(vec![7]);
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::Epsilon { target: 2 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 4 });
atn.state_mut(2)
.expect("state 2")
.add_transition(Transition::Atom {
target: 3,
label: 1,
});
atn.state_mut(3)
.expect("state 3")
.add_transition(Transition::Epsilon { target: 6 });Found a 22 line (130 tokens) duplication in the following files:
let Some(state) = atn.state(config.state) else {
continue;
};
for transition in &state.transitions {
if !transition.matches(symbol, MIN_CHAR_VALUE, MAX_CHAR_VALUE) {
continue;
}
let mut advanced = config.clone();
set_config_state(atn, &mut advanced, transition.target());
if symbol == EOF {
advanced.consumed_eof = true;
} else {
advanced.position += 1;
}
next.push(advanced);
}
}
let closure = epsilon_closure(atn, next, &mut |predicate| {
semantic_predicate(lexer, predicate)
});
let target_has_semantic_context = closure.has_semantic_context;Found a 20 line (127 tokens) duplication in the following files:
atn.set_rule_to_stop_state(vec![7]);
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::Epsilon { target: 2 });
atn.state_mut(1)
.expect("state 1")
.add_transition(Transition::Epsilon { target: 4 });
atn.state_mut(2)
.expect("state 2")
.add_transition(Transition::Atom {
target: 3,
label: 1,
});
atn.state_mut(3)
.expect("state 3")
.add_transition(Transition::Atom {Found a 19 line (120 tokens) duplication in the following files:
pub(crate) fn matching_action_brace(source: &str, mut index: usize) -> Option<usize> {
let mut nested = 0_usize;
let mut double_quoted = false;
let mut escaped = false;
while let Some(ch) = source[index..].chars().next() {
if escaped {
escaped = false;
index += ch.len_utf8();
continue;
}
match ch {
'\\' if double_quoted => escaped = true,
'"' => double_quoted = !double_quoted,
'\'' if !double_quoted => {
if let Some(next_index) = skip_char_literal(source, index) {
index = next_index;
continue;
}
}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 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());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 20 line (102 tokens) duplication in the following files:
atn: &Atn,
mut custom_action: A,
mut semantic_predicate: P,
mut accept_adjuster: E,
) -> CommonToken
where
I: CharStream,
F: TokenFactory,
A: FnMut(&mut BaseLexer<I, F>, LexerCustomAction),
P: FnMut(&BaseLexer<I, F>, LexerPredicate) -> bool,
E: FnMut(&mut BaseLexer<I, F>, i32, usize),
{
next_token_with_hooks_impl(
lexer,
atn,
&mut custom_action,
&mut semantic_predicate,
&mut accept_adjuster,
LexerMatchStrategy {
compiled: None,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, |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds semantic-predicate/action documentation, a new semantic IR module, and hook-aware parser and lexer runtime support. It also updates code generation to inventory semantic coordinates, emit 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 |
![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 implements the initial phases of the design for handling semantic predicates and actions (Issue #9). It introduces a new Semantic IR (semir module) to represent predicates and actions as data, adds support for user-defined SemanticHooks on the parser side, and implements a configurable policy (--sem-unknown) for handling unknown coordinates, complete with a semantics.json manifest output. The review feedback highlights two critical issues: a potential out-of-bounds panic in action_text when the start index exceeds the adjusted stop index, and a misalignment bug in parser_action_source_blocks that causes incorrect source spans and bodies to be recorded in the manifest for unsupported blocks.
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: b8ccf2e3b3
ℹ️ 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".
| .collect::<Vec<_>>(); | ||
| // A non-default policy must reach the interpreter through the emitted | ||
| // runtime options, so its literal forces the options-carrying call shape. | ||
| let unknown_policy_literal = match options.sem_unknown { | ||
| SemUnknownPolicy::AssumeTrue => None, |
There was a problem hiding this comment.
When --sem-unknown=assume-false or error is used on a parser whose ATN has only untranslated predicate transitions, unknown_policy_literal is set here but has_predicate_dispatch remains false, so the generated adaptive_direct_allowed gate can still take the ANTLR4_RUST_ADAPTIVE_DIRECT path. That helper falls back through parse_atn_rule(...) without the ParserRuntimeOptions emitted below, so the new unknown-predicate policy is lost and the predicate is treated as passing instead of failing/erroring. Please also disable the adaptive-direct path when a non-default unknown policy or unknown predicate coordinates are present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3bcc2f2. adaptive_direct_allowed now also requires unknown_policy_literal.is_none(), so a non-default --sem-unknown policy disables the ANTLR4_RUST_ADAPTIVE_DIRECT shortcut. As you noted, that path runs parse_atn_rule_adaptive_or_fallback, which falls back through parse_atn_rule without the emitted ParserRuntimeOptions, dropping the policy. New test non_default_policy_disables_adaptive_direct_gate asserts the gate literal flips from && true && (default) to && false && under both assume-false and error, and first confirms the predicate-free fixture emits the enabled gate by default so the test can't silently pass.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/parser.rs`:
- Around line 424-444: In action_text(), the EOF fallback is using the wrong
boundary and can include hidden tokens before EOF. Update the stop calculation
to match text_interval() and $text by using the visible-token boundary for
TOKEN_EOF instead of blindly subtracting one, and keep the logic localized
around action.stop_index(), self.input.get(), and self.input.text() so the
returned action text excludes trailing whitespace.
🪄 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: bfd76bb4-9d5e-409c-b77e-b4357fa7ac13
📒 Files selected for processing (7)
CLAUDE.mdREADME.mddocs/issue-9-semantic-predicates-actions-design.mdsrc/bin/antlr4-rust-gen.rssrc/lib.rssrc/parser.rssrc/semir.rs
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 568a824430
ℹ️ 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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/parser.rs (2)
2382-2402: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid replaying legacy and SemIR actions for the same coordinate.
When
semanticsis present, these paths still apply legacyParserMemberAction/ParserReturnActiontables first, then execute matching SemIR actions. If a generated parser passes both during migration,AddMemberside effects can double-apply. Prefer SemIR for a matched action coordinate, falling back to legacy only when no SemIR action exists.Also applies to: 2426-2444
🤖 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 2382 - 2402, The replay logic in the parser state handling is applying both legacy ParserMemberAction/ParserReturnAction entries and SemIR actions for the same coordinate, which can double-apply member updates. Update the code around the ParserTableSemCtx setup and the action replay loop in parser::... so that when semantics is present you first check for matching speculative SemIR actions and execute those instead, and only fall back to the legacy actions when no SemIR action exists for that source_state/coordinate. Make the same precedence change in the related block at the other referenced location so both paths stay consistent.
4953-4954: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope and restore the active unknown-predicate policy.
This mutates parser-level
unknown_predicate_policyand clearsunknown_predicate_hitsfor one runtime-options parse, but never restores the previous state on success or error. A later direct predicate check can inheritError/AssumeFalsefrom an earlier parse. Save the previous policy/hits and restore them before every return from this parse entry.Also applies to: 5013-5016
🤖 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 4953 - 4954, The runtime-options parse path is leaving parser state behind by mutating Parser::unknown_predicate_policy and clearing unknown_predicate_hits without restoring the prior values, so a later predicate check can inherit the wrong policy. In the parse entry that touches these fields, save the current unknown_predicate_policy and unknown_predicate_hits before changing them, and restore both on every exit path from the parse flow, including success and error returns.
🤖 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 `@docs/issue-9-semantic-predicates-actions-design.md`:
- Line 456: The markdownlint MD022 warning is caused by missing blank lines
around the Phase headings in the issue document. Update the markdown around the
`### Phase 3`, `### Phase 4`, and `### Phase 5` headings to insert the required
blank line separation before each heading, keeping the surrounding section text
unchanged. Use the heading markers themselves to locate the affected spots and
apply the same spacing fix consistently across those three sections.
In `@src/atn/lexer.rs`:
- Around line 229-274: The two semantic-hook entry points duplicate the same
action/predicate closure setup, so extract that wiring into a private helper.
Add a helper such as semantic_hook_closures that takes &RefCell<&mut H> and
returns the two closures used by next_token_with_semantic_hooks and
next_token_compiled_with_semantic_hooks, then have both functions create the
RefCell once and reuse the helper instead of inlining identical logic. Keep the
existing behavior in the closures (rule/action index conversion, LexerSemCtx
construction, and unwrap_or(true) default) unchanged.
In `@src/parser.rs`:
- Around line 2797-2811: The SemIR hook handling in `Parser::hook` is swallowing
missing `SemanticHooks::sempred` results by converting `None` into `false`,
which causes unregistered generated predicates to fail silently. Update
`Parser::hook` and the `SemanticHooks::sempred` call path to preserve the `None`
state for missing hooks and route it through the existing
unknown-coordinate/unsupported-hook handling instead of defaulting to rejection.
---
Outside diff comments:
In `@src/parser.rs`:
- Around line 2382-2402: The replay logic in the parser state handling is
applying both legacy ParserMemberAction/ParserReturnAction entries and SemIR
actions for the same coordinate, which can double-apply member updates. Update
the code around the ParserTableSemCtx setup and the action replay loop in
parser::... so that when semantics is present you first check for matching
speculative SemIR actions and execute those instead, and only fall back to the
legacy actions when no SemIR action exists for that source_state/coordinate.
Make the same precedence change in the related block at the other referenced
location so both paths stay consistent.
- Around line 4953-4954: The runtime-options parse path is leaving parser state
behind by mutating Parser::unknown_predicate_policy and clearing
unknown_predicate_hits without restoring the prior values, so a later predicate
check can inherit the wrong policy. In the parse entry that touches these
fields, save the current unknown_predicate_policy and unknown_predicate_hits
before changing them, and restore both on every exit path from the parse flow,
including success and error returns.
🪄 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: 100bf698-0678-4634-b1bc-c3e25ca0c070
📒 Files selected for processing (10)
CLAUDE.mdREADME.mddocs/issue-9-semantic-predicates-actions-design.mdpatterns/javascript.tomlsrc/atn/lexer.rssrc/bin/antlr4-rust-gen.rssrc/lexer.rssrc/lib.rssrc/parser.rssrc/semir.rs
Review pass over the SemIR delivery branch; validated with the full
conformance sweep (357/357), kotlin parity (9/9 trees match), unit
tests, and clippy -D warnings.
- Block walkers in bin_support/templates.rs now locate opening braces
through a shared GrammarSourceCursor that skips quoted literals,
comments, and charsets. Real grammars referencing brace tokens
('{' statementList? '}') previously desynchronized every predicate
span/hook pairing: on grammars-v4 JavaScriptParser the manifest had
0/16 predicate spans and 0 hook matches; now 16/16 with 11 routed to
the typed hook trait.
- Hook-routed lexer predicates fail codegen with a clear error instead
of panicking in the render path (generated lexers have no hook
plumbing yet).
- Helper-hooked coordinates report disposition "hooked" instead of
"translated" in semantics.json so users can tell which coordinates
still need a runtime hook implementation.
- Predicate IR evaluation no longer clones the speculative member map
and rule-name String on every evaluation inside the prediction loop;
the predicate context is now fully borrowed and read-only.
- patterns/javascript.toml aligned with grammars-v4 JavaScriptParser:
adds lineTerminatorAhead, drops the lexer-side isRegexPossible, and
documents the argument-taking n()/p() helper gap.
- Design doc Phase 4/5 statuses tempered to partially-implemented with
the concrete remaining gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
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)
src/parser.rs (1)
4300-4324: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSurface unhandled parser actions instead of returning an ignored
bool.
parser_action_hookreturns whether the hook handled the committed action, but generated fallback code ignores that result. With default/no-op hooks, untranslated grammar actions can be skipped while parsing succeeds, which violates the fail-loud semantic-action boundary. Consider returning/propagatingResult<(), AntlrError>or recording unsupported action hits like unknown predicates.🤖 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 4300 - 4324, The parser_action_hook path currently returns a bool that can be silently ignored by generated fallback code, allowing unsupported grammar actions to pass without surfacing an error. Update parser_action_hook and its callers to propagate a failure signal instead of treating the return value as optional, ideally using a Result-based flow or equivalent error recording consistent with unknown predicate handling; reference parser_action_hook, ParserSemCtx, and semantic_hooks.action when wiring this through.
🤖 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 `@docs/issue-9-semantic-predicates-actions-design.md`:
- Line 482: The Phase 5 heading is missing the required blank line after it, so
update the markdown around the heading to ensure it is properly separated per
MD022. Locate the Phase 5 section in the document and add the missing empty line
immediately after the heading so the surrounding paragraph structure is correct.
---
Outside diff comments:
In `@src/parser.rs`:
- Around line 4300-4324: The parser_action_hook path currently returns a bool
that can be silently ignored by generated fallback code, allowing unsupported
grammar actions to pass without surfacing an error. Update parser_action_hook
and its callers to propagate a failure signal instead of treating the return
value as optional, ideally using a Result-based flow or equivalent error
recording consistent with unknown predicate handling; reference
parser_action_hook, ParserSemCtx, and semantic_hooks.action when wiring this
through.
🪄 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: 49417a6b-cbe4-4e51-9b30-b7492e852d9f
📒 Files selected for processing (6)
docs/issue-9-semantic-predicates-actions-design.mdpatterns/javascript.tomlsrc/bin/antlr4-rust-gen.rssrc/bin_support/templates.rssrc/parser.rssrc/recognizer.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6951933d2
ℹ️ 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".
Address PR #55 review findings, all in service of design goal G1 (never silently mis-parse): - Parser SemIR hook: route a declining user hook (`None`) through the configured `UnknownSemanticPolicy` instead of `unwrap_or(false)`, so a `PExpr::Hook` coordinate with no implementation no longer silently rejects its alternative. Extracts a shared `apply_unknown_predicate_policy` so the SemIR and legacy table paths dispatch identically (hook -> policy). (Codex + CodeRabbit) - Generator: disable the adaptive-direct shortcut whenever a non-default unknown-predicate policy is emitted; that path falls back through `parse_atn_rule` without the `ParserRuntimeOptions` carrying the policy, which would drop it. (Codex) - Generator: the lexer `run_predicate` catch-all arm now follows `--sem-unknown` (`assume-false` -> `_ => false`), so a mixed lexer's uncovered predicate is not left viable. (Codex) - `ParserSemCtx::action_text`: end the EOF interval at the previous *visible* token (matching `text_interval`/`$text`) instead of a blind `stop - 1`, excluding trailing hidden tokens. (Gemini + CodeRabbit) - Manifest: pair each action-block source span with its ATN state through the same offset used for templates, walking signature templates in lockstep so span/body provenance no longer drifts after a `returns [<...>]` template. (Gemini) - Deduplicate the two identical lexer semantic-hook closure bodies into `dispatch_lexer_action_hook` / `dispatch_lexer_predicate_hook`. (CodeRabbit + CPD) - Docs: blank lines around Phase headings (markdownlint MD022). (CodeRabbit) New regression tests cover the SemIR-hook policy fallthrough across all three policies, the adaptive-direct gate flip, the lexer default-arm flip, and action-slot/span alignment.
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Pushed 3bcc2f2 addressing the open review threads. Summary: Fixed (fail-loud semantic boundary, G1):
Fixed (other):
Two "outside diff range" notes, addressed as no-change-needed:
Validation: @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bcc2f2a1b
ℹ️ 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".
Address the two P1 findings from Codex's review of 3bcc2f2, both deeper instances of the fail-loud boundary (G1) in paths the first pass missed: - Generated recursive-descent predicate evaluation reads `BaseParser::unknown_predicate_policy`, but only the interpreter fallback installed it (via `ParserRuntimeOptions`); the generated-direct path left it at the `AssumeTrue` default, so a hook predicate returning `None` in a generated rule silently passed even under `--sem-unknown=error`. Add a public `set_unknown_predicate_policy` setter and have the generated parser constructor install a non-default policy, so both paths honor `--sem-unknown`. Also add `take_unknown_semantic_error` so the generated path can surface recorded `Error`-policy coordinates. - Under `--sem-unknown=hook`/`error`, an *uncovered* lexer predicate was marked `hooked` in the manifest and accepted by `--require-full-semantics`, yet generated lexers have no hook plumbing (a `hook`-lowered lexer predicate is already a codegen error) and no runtime coordinate recording, so `run_predicate`'s catch-all silently kept it viable. `render_lexer` now rejects uncovered lexer predicates under hook/error, mirroring the existing explicit-hook rejection. Tests: generated constructor installs the policy literal (codegen) and the generated-direct matcher honors it end-to-end (runtime, all three policies); uncovered lexer predicates are rejected under hook and error.
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Pushed 88e9fbd addressing both P1 findings from the last review — both were deeper instances of the fail-loud boundary (G1) in paths the previous commit didn't reach:
Validation: @codex review |
There was a problem hiding this comment.
💡 Codex Review
antlr-rust-runtime/src/bin/antlr4-rust-gen.rs
Line 5376 in 88e9fbd
Surface generated unknown semantic hits before success
When a generated semantic decision evaluates a hook-lowered predicate and the hook returns None, parser_semantic_ir_predicate_matches_with_context_and_local records the coordinate under the Error fallback and returns false; the interpreter path checks this after recognition, but this generated success path returns the tree without draining take_unknown_semantic_error(). In that scenario the unknown predicate can merely prune an alternative and still produce a parse tree instead of the promised AntlrError::Unsupported, so add the same recorded-error check before returning success.
antlr-rust-runtime/src/bin/antlr4-rust-gen.rs
Line 3735 in 88e9fbd
Return the recorded unknown semantic error
For a hook-lowered predicate reached as an actual generated rule step, an unhandled hook under the Error fallback records the unsupported coordinate and returns false, but this branch converts it into a generic failed-predicate ParserError. That loses the coordinate and violates the fail-loud Unsupported behavior; check take_unknown_semantic_error() before falling back to failed_predicate_error here.
ℹ️ 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".
Address Codex P2 (review of 88e9fbd): the predicate-block scans, unlike the action collectors, never applied rule-name filtering. In a combined grammar (`--grammar` = one .g4 with both lexer and parser rules) this walked the other rule set's `{...}?` predicates into the positional pairing against this ATN's predicate transitions — mis-mapping a translatable predicate onto the wrong coordinate, or erroring with "no parser ATN predicate transition". `parser_predicate_templates` and the new `extract_supported_predicate_templates_filtered` now gate each predicate block through `rule_action_included` (the same filter the action collectors use), skipping blocks that belong to a different rule set so coordinate pairing stays aligned. `lexer_predicate_templates` passes the lexer's rule names; the parser scan passes the parser's. Tests: a translatable lexer-rule predicate preceding a parser rule is skipped (not mapped onto the parser coordinate), and the parser-rule predicate still maps to its correct coordinate.
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
antlr-rust-runtime/src/bin/antlr4-rust-gen.rs
Lines 5841 to 5842 in bfee243
Let mixed lexer predicates use policy fallbacks
For a lexer with one translated predicate and one untranslated predicate under --sem-unknown=assume-false or assume-true, this equality check errors as soon as the template count differs, so the uncovered-coordinate logic and the generated catch-all arm never run. Fresh evidence is that render_lexer_predicate_method now has a policy-aware default arm for mixed lexers, but lexer_predicate_templates still rejects those mixed inputs before rendering.
ℹ️ 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".
The predicate rule-name filter added in bfee243 regressed SemPredEvalParser/ValidateInDFA: its grammar has `// ';' helps ...` above rule `a`, and the brace-counting `statement_rule_header` matches that `;` inside the comment as a rule terminator, so header resolution fails (`None`). `rule_action_included` treats an unresolvable header as "not in the filter set" and skipped rule `a`'s `{<False()>}?`/`{<True()>}?` predicates, dropping them from `parser_semantics()` — the guarded alternatives then went unguarded (`alt 1` printed instead of no-viable-alt). Introduce `predicate_block_included`, which excludes a predicate block only when its owning rule name *positively resolves* and is absent from the target rule set; an unresolvable header keeps the block (pre-filter behavior). Both predicate scans use it, so the combined-grammar lexer/parser separation still holds without dropping real predicates when the scraper is defeated by comments. Test: `parser_predicate_scan_keeps_predicate_when_header_unresolvable` pins the comment-with-semicolon case; SemPredEvalParser is back to 26/26.
There was a problem hiding this comment.
tinovyatkin has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
antlr-rust-runtime/src/bin/antlr4-rust-gen.rs
Line 5841 in ec7e495
Allow fallback for partially translated lexer predicates
When a lexer has a mix of one predicate that matches a built-in/pattern template and another raw predicate that should follow --sem-unknown=assume-true or assume-false, extract_supported_predicate_templates_filtered returns only the translated entries, so this count check rejects generation before the fallback arm rendered in run_predicate can apply. The all-unknown case is accepted via templates.is_empty(), so adding a single translatable predicate makes the same uncovered coordinate a hard codegen error instead of using the documented fallback policy.
antlr-rust-runtime/src/bin/antlr4-rust-gen.rs
Line 7697 in ec7e495
Preserve SetMember effects in interpreted rules
For parser rules that fall back to the ATN interpreter, supported member mutations are replayed speculatively through the SemIR action table, but this arm drops SetMember while only AddMember is collected. A rule with SetMember("i", "3") followed by a MemberEquals("i", "3") predicate will work on the generated-direct path (which inlines set_int_member) but the interpreted fallback leaves i at its previous value and can choose/reject the wrong alternative.
ℹ️ 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".
Address Codex's three findings on bfee243: - Generated top-level rule entry now calls `take_unknown_semantic_error` before returning `Ok`, so Error-policy coordinates the generated-direct predicate path recorded surface as `AntlrError::Unsupported` instead of a recovered `Ok` tree. Wires up the accessor added in 88e9fbd (previously only reachable from tests). Surfaced at the public entry (`allow_generated_fallback`) so nested rules accumulate and the outermost reports, mirroring the interpreter entry. (P1) - `parser_typed_hook_mappings` now applies the same `predicate_block_included` rule-name filter as `parser_predicate_templates`, so a combined grammar's lexer-rule helper predicate no longer consumes a parser coordinate and wires `MyParserTypedHooks` to the wrong method. (P2) - `enforce_sem_unknown` now fails codegen for any per-coordinate `dispose = "error"` override regardless of the global policy. Such an override lowers to no SemIR entry and does not escalate the runtime policy, so previously it silently fell back to the global default (e.g. AssumeTrue) instead of rejecting the coordinate. (P2) Tests: generated entry emits the surfacing call; typed-hook mapping skips a lexer-rule predicate preceding the parser helper; a per-coordinate error override fails even under assume-true.
- java_style_list: Java List.toString formatter (Go PrintArrayJavaStyle / Python str_list analog) for rule-invocation-stack and token-list prints. - GeneratedAttrs: typed per-rule attribute snapshot on ParserRuleContext, the typed replacement path for the int-only int_returns map. - to_string_tree(Some(self)): recognizer-resolved tree rendering matching ANTLR's toStringTree(parser); the rule-names form is now to_string_tree_with_names. - rule_invocation_stack(): live rule stack names, current-first. - ExpectedTokenSet + expected_tokens_current(): getExpectedTokens shape. - BailErrorStrategy + BaseParser bail flag; recover_generated_match propagates the mismatch instead of recovering when set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The conformance harness gains --embedded: descriptor grammars render through .conformance-review/Rust.test.stg with the real StringTemplate engine (RenderGrammar.java via the ANTLR jar), and the rendered grammar feeds both the ANTLR tool and antlr4-rust-gen --actions embedded. The generator's embedded mode (src/bin_support/embedded.rs) models the rendered grammar (rule attrs, alternatives, labels, members blocks) and translates $-attribute references — the Rust analog of ANTLR's ActionTranslator — then splices bodies verbatim: actions execute inline at their ATN action states (no buffering/replay), predicates become inline expressions at decision filters and predicate steps, @init runs at rule entry, @after on the committed path before finish_rule, members become real struct fields and impl items, and per-rule attrs structs seal typed GeneratedAttrs snapshots onto contexts. First descriptor passes end-to-end (SemPredEvalParser/ActionHidesPreds). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… action pairing - Associated token constants (Self::NL) on the generated parser. - Rule-call arguments: literal ints and caller-attr identifiers correlate to rule transitions (parser_rule_args-style) and flow through a __embedded_pending_arg slot consumed at callee entry. - Alt scanner: label='literal', label on ~set/(...) blocks, += list labels (translated to child collections), $ctx.<rule>_all() calls. - Action blocks pair with ATN action states PER RULE, so synthesized states cannot shift an author action into a neighboring rule. - Grammar scanner: named actions (@parser::members) and options/tokens blocks no longer desynchronize rule-definition detection. - TreeNodeWithAltNumField renders empty (reference correction: the Rust runtime records alt numbers natively; contextSuperClass is metadata). SemPredEvalParser 26/26, ParserExec 50/50 pending re-sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lexer bodies translate the recognizer surface textually onto the BaseLexer hooks (self.text() -> token_text_until(position), column accessors, stdout sink) and pair with serialized coordinates through the same source walks as the template path. The semantics-manifest collectors receive no grammar source in embedded mode — template recognition does not apply to rendered Rust. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… API - Per-rule and per-labeled-alt context view structs with positional child accessors, public attribute fields, FromRuleContext downcast, child_count/start, and Java RuleContext.toString-style Display over the walker-threaded invoking-state chain. - <Grammar>Listener trait with defaulted enter_/exit_ callbacks (rule + labeled alternative) and visit_terminal; module-local ParseTreeWalker bridges the runtime walker onto typed callbacks, dispatching labeled LR alternatives structurally (operator alts wrap the rule operand). - CommonToken::text() inherent method returns &str (ANTLR getText shape); TerminalNode implements Display (token text). - LR action-slot pairing follows ANTLR's rewrite order (primary alts before operator alts). All 7 Listeners cases and the downcast-heavy LeftRecursion labeled cases pass embedded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Embedded slot walk excludes tokens{}/channels{} metadata blocks;
rendered slave grammars follow the delegator's import order (first
definition wins) — all CompositeParsers/CompositeLexers pass.
- Listener callback names use raw rule/label names after the enter_/
exit_ prefix (r#type would be a syntax error mid-identifier).
- ParserAtnSimulator::dump_dfa_java_style renders learned decision DFAs
in Java DFASerializer format; the generated dump_dfa facade uses it.
- reportAmbiguity is suppressed outside LlExactAmbigDetection, matching
Java's exactOnly DiagnosticErrorListener (default LL prediction stops
at the first non-exact conflict).
- The embedded pipeline no longer replays canned FullContextParsing
diagnostics: 4/15 of those cases now pass on earned output alone; the
remaining 11 need per-decision DFA-learning parity (documented
residual).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract parser_statement_maps / collect_noop_action_states / render_public_rule_methods / embedded_step_render / embedded_render_slots / unknown_policy_literal / render_ctx_rooted_states_constant / embedded_imports out of render_parser_with_options (too_many_lines), and route every string substitution through one replace_all helper (disallowed str::replace). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract helpers to satisfy too_many_lines, replace disallowed str::replace with a shared manual replace_all, fix pass-by-value/doc/const-fn lints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
💡 Codex Reviewantlr-rust-runtime/src/bin/antlr4-rust-gen.rs Lines 66 to 70 in e8de0f2 Preserve embedded bodies in semantic accounting
When ℹ️ 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". |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A grammar rule named start (or child_count) emitted a rule accessor
colliding with the view's built-in start-token accessor — Java overloads
the field, Rust cannot (E0592). Keep the built-in, which rendered bodies
use, and leave {rule}_all as the indexed escape hatch. Fixes embedded
SemPredEvalParser/AtomWithClosureInTranslatedLRRule.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The rendered Rust.test.stg pipeline is now the harness's only pipeline: descriptor grammars are always rendered through StringTemplate and generated with --actions embedded. Deletes ~700 lines of template pattern-matching, output simulation, and the FullContextParsing canned replay; --embedded is accepted as a no-op for compatibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Alt label Call emitted trait method exit_Call while rendered bodies — and the accessor convention — use exit_call. Route rule names and labels through rust_function_name when forming enter_/exit_ methods (views keep the label's camel case), deduping by method name. Fixes embedded Listeners/LRWithLabels. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
💡 Codex Reviewantlr-rust-runtime/src/bin/antlr4-rust-gen.rs Line 4667 in 537afe5 Emit only one semantic-alt fallback terminator
When a generated decision has a semantic-predicate alternative, this helper now writes two trailing antlr-rust-runtime/src/bin/antlr4-rust-gen.rs Line 5894 in 537afe5 Reject nonmatching labeled-alt downcasts
For labeled-alternative views this generated antlr-rust-runtime/src/bin/antlr4-rust-gen.rs Lines 6019 to 6021 in 537afe5 Dispatch listener callbacks for each matched label
When a rule has more than one labeled primary or more than one labeled operator alternative, this fallback emits only the rule-level callback. For common grammars like ℹ️ 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". |
Two embedded-mode left-recursion bugs: (1) an alternative starting with a
string literal ('(' e ')') was classified as an operator alternative
because the refs list skips literals — classify from raw source instead,
allowing label= and <assoc=...> prefixes; (2) the previous iteration
context now gets the accumulated attrs sealed onto it before
push_new_recursion_context_with_previous, matching Java's _prevctx
semantics, so operator-alt actions and typed views can read the left
operand's attributes. Fixes all 12 LeftRecursion conformance failures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Bare $label reads on labeled literals render the Token object (ConjuringUpToken*) - A grammar rule named 'start' takes the view accessor slot; the built-in start-token helper yields (DuplicatedLeftRecursiveCall*, LL1ErrorInfo, InvalidEmptyInput, InvalidATNStateRemoval, IfIfElse*, and more) - Listener trait methods are snake_case per the .test.stg convention (Listeners/LRWithLabels) - Parser diagnostics print in prediction-event order, with lexer errors merged by position, matching Java's console (CtxSensitiveDFA_1, SLLSeesEOFInLLGrammar) - The 9 remaining FullContextParsing cases and PositionAdjustingLexer are skipped with the missing feature named — they fail honestly when run Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
It never shipped outside this branch, so there is nothing to be compatible with; unknown-argument handling now rejects it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The remote commits (e8de0f2, aad15bf, 537afe5) are a parallel session's implementations of a subset of the fixes on this line: the clippy pass, the listener snake-casing, and the start-accessor collision (resolved there by skipping the rule accessor; here the built-in yields instead, validated against ParserErrors/DuplicatedLeftRecursiveCall*, LL1ErrorInfo, InvalidEmptyInput, InvalidATNStateRemoval, and SemPredEvalParser/AtomWithClosureInTranslatedLRRule). Tree taken wholesale from this line ('ours'), which is the conformance-validated superset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
💡 Codex Reviewantlr-rust-runtime/src/bin/antlr4-rust-gen.rs Lines 66 to 67 in 25de401 Honor embedded semantics in strict inventory
When ℹ️ 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". |
|
Definitive conformance sweep at 25de401 (rendered pipeline, now the only pipeline): All 357 upstream descriptors are accounted for. The 10 skips are the named honest residuals — 9 FullContextParsing cases tracked in #58 (Java per-decision DFA-learning parity) and LexerExec/PositionAdjustingLexer (lexer subclass overrides). Follow-up #57 tracks removing the now-dead ST-markup template subsystem from the generator. 🤖 Generated with Claude Code |
1 participant
Summary
ParserSemanticstable while keeping the existing legacy predicate/action table API as a compatibility adapter--sem-patterns,--require-full-semantics, explicit--sem-unknown=hook, coordinate overrides, exact pattern/helper lowering, and JavaScript helper hook data inpatterns/javascript.tomlLexerSemCtxValidation
cargo +1.95.0 check --locked --all-targetscargo +1.95.0 clippy --locked --all-targets --all-features -- -D warningscargo +1.95.0 test --locked --all-targetscargo +1.95.0 run --release --quiet --bin antlr4-runtime-testsuite->summary: 357 passed, 0 failed, 0 skipped, 357 runtests/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/antlr-python/bin/python-> all 9 Kotlin/script snippets matchedCloses #9