Skip to content

Move actiont checking from codegen to ast validation. #659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ratmice wants to merge 10 commits into
base: master
Choose a base branch
from
+198 −76
Open

Move actiont checking from codegen to ast validation. #659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
492b074
Move actiont checking from codegen to ast validation.
ratmice Aug 8, 2026
9e956c2
Move check that there is action code to complete_and_validate.
ratmice Aug 9, 2026
6313b96
Move action '$' variable checks to complete_and_validate
ratmice Aug 9, 2026
ca3a9b1
Add test for unrecognized action variables
ratmice Aug 9, 2026
ff5a2d6
Add docs for action variable check
ratmice Aug 9, 2026
c2d027c
Rename error kind
ratmice Aug 9, 2026
d244993
Fix the UnrecognisedActionVariable span calculation
ratmice Aug 10, 2026
9f55993
Improve UnrecognisedActionSpan
ratmice Aug 10, 2026
d148d1b
Remove now out of date comment
ratmice Aug 10, 2026
a840a91
Remove extraneous string trimming
ratmice Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 170 additions & 21 deletions cfgrammar/src/lib/yacc/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use super::{
use crate::{
Span,
header::{GrmtoolsSectionParser, HeaderError, HeaderErrorKind, HeaderValue},
yacc::YaccOriginalActionKind,
};

/// Any error from the Yacc parser returns an instance of this struct.
Expand Down Expand Up @@ -55,7 +56,9 @@ impl ASTWithValidityInfo {
let mut yp = YaccParser::new(yacc_kind, s);
yp.parse().map_err(|e| errs.extend(e)).ok();
let mut ast = yp.build();
ast.complete_and_validate().map_err(|e| errs.push(e)).ok();
ast.complete_and_validate(Some(yacc_kind))
.map_err(|e| errs.push(e))
.ok();
ast
};
ASTWithValidityInfo {
Expand Down Expand Up @@ -122,7 +125,9 @@ impl FromStr for ASTWithValidityInfo {
let mut yp = YaccParser::new(yacc_kind, src);
yp.parse().map_err(|e| errs.extend(e)).ok();
let mut ast = yp.build();
ast.complete_and_validate().map_err(|e| errs.push(e)).ok();
ast.complete_and_validate(Some(yacc_kind))
.map_err(|e| errs.push(e))
.ok();
ast
};
Ok(ASTWithValidityInfo {
Expand Down Expand Up @@ -307,9 +312,19 @@ impl GrammarAST {
/// 3) Every token reference references a declared token
/// 4) If a production has a precedence token, then it references a declared token
/// 5) Every token declared with %epp matches a known token
///
/// If the validation succeeds, None is returned.
pub(crate) fn complete_and_validate(&mut self) -> Result<(), YaccGrammarError> {
/// 6) If `yacc_kind` is specified, perform any kind specific validation.
/// * If the kind requires an action type, check that each rule has one
/// * That each production has action code
/// * That `$` variables referred to in action code are recognised.
pub(crate) fn complete_and_validate(
&mut self,
yacc_kind: Option<YaccKind>,
) -> Result<(), YaccGrammarError> {
let kind_requires_action_checks = matches!(
yacc_kind,
Some(YaccKind::Original(YaccOriginalActionKind::UserAction)) | Some(YaccKind::Grmtools)

@ratmice ratmice Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider making this boolean check a method on YaccKind?
With a different name like YaccKind::requires_actions?

I think it might also simplify a similar check in CTParserBulder.

);

match self.start {
None => {
return Err(YaccGrammarError {
Expand All @@ -327,8 +342,48 @@ impl GrammarAST {
}
}
for rule in self.rules.values() {
if kind_requires_action_checks && rule.actiont.is_none() {
return Err(YaccGrammarError {
kind: YaccGrammarErrorKind::MissingActionType,
spans: vec![rule.name.1],
});
}
for &pidx in &rule.pidxs {
let prod = &self.prods[pidx];
if kind_requires_action_checks {
if let Some((action_code, action_span)) = prod.action.as_ref() {
let mut last = 0;
while let Some(off) = action_code[last..].find('$') {
if !(action_code[last + off..].starts_with("$$")
|| action_code[last + off..].starts_with("$lexer")
|| action_code[last + off..].starts_with("$span")
|| (last + off + 1 < action_code.len()
&& action_code[last + off + 1..]
.starts_with(|c: char| c.is_numeric())))
{
// Starting from the `$` find the end of a variable name, otherwise default to the span of the `$`
let m = crate::yacc::parser::RE_NAME
.find(&action_code[last + off + 1..]);
let var_start_pos = action_span.start() + last + off;
let var_end_pos = m
.map(|m| var_start_pos + 1 + m.end())
.unwrap_or(var_start_pos + 1);
return Err(YaccGrammarError {
kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
spans: vec![Span::new(var_start_pos, var_end_pos)],
});
} else {
last = last + off + 2;
}
}
} else {
return Err(YaccGrammarError {
kind: YaccGrammarErrorKind::MissingActionCode,
spans: vec![prod.prod_span],
});
}
}

if let Some(ref n) = prod.precedence {
if !self.tokens.contains(n) {
return Err(YaccGrammarError {
Expand Down Expand Up @@ -510,7 +565,7 @@ mod test {
#[test]
fn test_empty_grammar() {
let mut grm = GrammarAST::new();
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::NoStartRule,
..
Expand All @@ -526,7 +581,7 @@ mod test {
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("B".to_string(), empty_span), None);
grm.add_prod("B".to_string(), vec![], None, None, empty_span);
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::InvalidStartRule(_),
..
Expand All @@ -542,7 +597,7 @@ mod test {
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("A".to_string(), empty_span), None);
grm.add_prod("A".to_string(), vec![], None, None, empty_span);
assert!(grm.complete_and_validate().is_ok());
assert!(grm.complete_and_validate(None).is_ok());
}

#[test]
Expand All @@ -554,7 +609,7 @@ mod test {
grm.add_rule(("B".to_string(), empty_span), None);
grm.add_prod("A".to_string(), vec![rule("B")], None, None, empty_span);
grm.add_prod("B".to_string(), vec![], None, None, empty_span);
assert!(grm.complete_and_validate().is_ok());
assert!(grm.complete_and_validate(None).is_ok());
}

#[test]
Expand All @@ -564,7 +619,7 @@ mod test {
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("A".to_string(), empty_span), None);
grm.add_prod("A".to_string(), vec![rule("B")], None, None, empty_span);
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::UnknownRuleRef(_),
..
Expand All @@ -581,7 +636,7 @@ mod test {
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("A".to_string(), empty_span), None);
grm.add_prod("A".to_string(), vec![token("b")], None, None, empty_span);
assert!(grm.complete_and_validate().is_ok());
assert!(grm.complete_and_validate(None).is_ok());
}

#[test]
Expand All @@ -594,7 +649,7 @@ mod test {
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("A".to_string(), empty_span), None);
grm.add_prod("A".to_string(), vec![rule("b")], None, None, empty_span);
assert!(grm.complete_and_validate().is_err());
assert!(grm.complete_and_validate(None).is_err());
}

#[test]
Expand All @@ -604,7 +659,7 @@ mod test {
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("A".to_string(), empty_span), None);
grm.add_prod("A".to_string(), vec![token("b")], None, None, empty_span);
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::UnknownToken(_),
..
Expand All @@ -626,7 +681,7 @@ mod test {
None,
Span::new(0, 2),
);
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::UnknownRuleRef(_),
..
Expand All @@ -644,7 +699,7 @@ mod test {
grm.add_prod("A".to_string(), vec![], None, None, empty_span);
grm.epp
.insert("k".to_owned(), (empty_span, ("v".to_owned(), empty_span)));
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::UnknownEPP(_),
spans,
Expand Down Expand Up @@ -677,7 +732,7 @@ mod test {
None,
empty_span,
);
assert!(grm.complete_and_validate().is_ok());
assert!(grm.complete_and_validate(None).is_ok());
}

#[test]
Expand All @@ -693,15 +748,15 @@ mod test {
None,
empty_span,
);
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::UnknownToken(_),
..
}) => (),
_ => panic!("Validation error"),
}
grm.tokens.insert("b".to_string());
match grm.complete_and_validate() {
match grm.complete_and_validate(None) {
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::NoPrecForToken(_),
..
Expand Down Expand Up @@ -737,7 +792,6 @@ mod test {
#[test]
fn token_rule_confusion_issue_557() {
use super::*;
use crate::yacc::*;
let ast_validity = ASTWithValidityInfo::new(
YaccKind::Original(YaccOriginalActionKind::GenericParseTree),
r#"
Expand Down Expand Up @@ -785,7 +839,6 @@ mod test {
#[test]
fn test_token_directives() {
use super::*;
use crate::yacc::*;

// Testing that `%token a` after `%left "a"` still ends up in
let ast_validity = ASTWithValidityInfo::new(
Expand Down Expand Up @@ -815,7 +868,6 @@ mod test {
#[test]
fn clone_ast_changing_start_rule() {
use super::*;
use crate::yacc::*;
let y_src = r#"
%start AStart
%token A B C
Expand All @@ -837,4 +889,101 @@ mod test {
Some(&bstart_rule.name)
);
}

#[test]
fn test_missing_actiont() {
use super::*;
let ast_validity = ASTWithValidityInfo::new(
YaccKind::Original(YaccOriginalActionKind::UserAction),
r#"
%token a
%%
start: "a";
"#,
);
assert_eq!(
ast_validity.errors(),
vec![YaccGrammarError {
kind: YaccGrammarErrorKind::MissingActionType,
spans: vec![Span::new(13, 18)],
}]
);

let ast_validity = ASTWithValidityInfo::new(
YaccKind::Original(YaccOriginalActionKind::UserAction),
r#"
%actiontype ()
%token a
%%
start: "a" { };
"#,
);
assert!(ast_validity.errors().is_empty());

let mut grm = GrammarAST::new();
let empty_span = Span::new(0, 0);
let rule_span = Span::new(255, 255);
grm.start = Some(("A".to_string(), empty_span));
grm.add_rule(("A".to_string(), rule_span), None);
grm.add_prod("A".to_string(), vec![], None, None, empty_span);
assert_eq!(
grm.complete_and_validate(Some(YaccKind::Grmtools)),
Err(YaccGrammarError {
kind: YaccGrammarErrorKind::MissingActionType,
spans: vec![rule_span],
})
);
}

#[test]
fn test_unrecognized_action_variable() {
use super::*;
let ast_validity = ASTWithValidityInfo::new(
YaccKind::Grmtools,
r#"
%token a
%%
start -> () : "a" { $foo; };
"#,
);
assert_eq!(
ast_validity.errors(),
vec![YaccGrammarError {
kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
spans: vec![Span::new(33, 37)],
}]
);

let ast_validity = ASTWithValidityInfo::new(
YaccKind::Grmtools,
r#"
%token a
%%
start -> () : "a" {$};
"#,
);
assert_eq!(
ast_validity.errors(),
vec![YaccGrammarError {
kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
spans: vec![Span::new(32, 33)],
}]
);

let ast_validity = ASTWithValidityInfo::new(
YaccKind::Grmtools,
r#"
%token a
%%
start -> () : "a" {$;;;; };
"#,
);
assert_eq!(
ast_validity.errors(),
vec![YaccGrammarError {
kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
spans: vec![Span::new(32, 33)],
}]
);
}
}
8 changes: 4 additions & 4 deletions cfgrammar/src/lib/yacc/grammar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1573,13 +1573,13 @@ mod test {
"%grmtools{yacckind: YaccKind::Original(yaccoriginalactionkind::useraction)}
%actiontype ()
%%
Start: ;",
Start: {};",
"%grmtools{yacckind: Original(YACCOriginalActionKind::NoAction)}
%%
Start: ;",
"%grmtools{yacckind: YaccKind::Grmtools}
%%
Start -> () : ;",
Start -> () : {};",
];
for src in srcs {
YaccGrammar::<u32>::from_str(src).unwrap();
Expand Down Expand Up @@ -1623,15 +1623,15 @@ mod test {
yacckind: YaccKind::Grmtools,
}
%%
Start -> () : ;
Start -> () : {};
"#;
YaccGrammar::<u32>::from_str(src).unwrap();
let src = r#"
%grmtools{
yacckind: YaccKind::Grmtools
}
%%
Start -> () : ;
Start -> () : {};
"#;
YaccGrammar::<u32>::from_str(src).unwrap();
}
Expand Down
Loading