From 492b0741f6be4e6e9dfe3a85c321c5ec303f86db Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 8 Aug 2026 01:47:07 -0700 Subject: [PATCH 01/10] Move actiont checking from codegen to ast validation. --- cfgrammar/src/lib/yacc/ast.rs | 103 ++++++++++++++++++++++++------- cfgrammar/src/lib/yacc/parser.rs | 3 + lrpar/src/lib/ctbuilder.rs | 18 ++---- 3 files changed, 89 insertions(+), 35 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index b860385c0..90d40f607 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -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. @@ -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 { @@ -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 { @@ -307,9 +312,17 @@ 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. + pub(crate) fn complete_and_validate( + &mut self, + yacc_kind: Option, + ) -> Result<(), YaccGrammarError> { + let kind_requires_actiont = matches!( + yacc_kind, + Some(YaccKind::Original(YaccOriginalActionKind::UserAction)) | Some(YaccKind::Grmtools) + ); + match self.start { None => { return Err(YaccGrammarError { @@ -327,6 +340,12 @@ impl GrammarAST { } } for rule in self.rules.values() { + if kind_requires_actiont && 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 let Some(ref n) = prod.precedence { @@ -510,7 +529,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, .. @@ -526,7 +545,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(_), .. @@ -542,7 +561,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] @@ -554,7 +573,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] @@ -564,7 +583,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(_), .. @@ -581,7 +600,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] @@ -594,7 +613,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] @@ -604,7 +623,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(_), .. @@ -626,7 +645,7 @@ mod test { None, Span::new(0, 2), ); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(_), .. @@ -644,7 +663,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, @@ -677,7 +696,7 @@ mod test { None, empty_span, ); - assert!(grm.complete_and_validate().is_ok()); + assert!(grm.complete_and_validate(None).is_ok()); } #[test] @@ -693,7 +712,7 @@ mod test { None, empty_span, ); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(_), .. @@ -701,7 +720,7 @@ mod test { _ => 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(_), .. @@ -737,7 +756,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#" @@ -785,7 +803,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( @@ -815,7 +832,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 @@ -837,4 +853,49 @@ 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], + }) + ); + } } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index c452d4ae9..9721fc4de 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -36,6 +36,7 @@ pub enum YaccGrammarErrorKind { IncompleteRule, IncompleteComment, IncompleteAction, + MissingActionType, MissingColon, MissingRightArrow, MismatchedBrace, @@ -97,6 +98,7 @@ impl fmt::Display for YaccGrammarErrorKind { YaccGrammarErrorKind::IncompleteRule => "Incomplete rule", YaccGrammarErrorKind::IncompleteComment => "Incomplete comment", YaccGrammarErrorKind::IncompleteAction => "Incomplete action", + YaccGrammarErrorKind::MissingActionType => "Missing action type", YaccGrammarErrorKind::MissingColon => "Missing ':'", YaccGrammarErrorKind::MissingRightArrow => "Missing '->'", YaccGrammarErrorKind::MismatchedBrace => "Mismatched brace", @@ -245,6 +247,7 @@ impl Spanned for YaccGrammarError { | YaccGrammarErrorKind::IncompleteRule | YaccGrammarErrorKind::IncompleteComment | YaccGrammarErrorKind::IncompleteAction + | YaccGrammarErrorKind::MissingActionType | YaccGrammarErrorKind::MissingColon | YaccGrammarErrorKind::MissingRightArrow | YaccGrammarErrorKind::MismatchedBrace diff --git a/lrpar/src/lib/ctbuilder.rs b/lrpar/src/lib/ctbuilder.rs index 830558116..c2b401234 100644 --- a/lrpar/src/lib/ctbuilder.rs +++ b/lrpar/src/lib/ctbuilder.rs @@ -1649,20 +1649,10 @@ where for i in 0..grm.prod(pidx).len() { let argt = match grm.prod(pidx)[i] { Symbol::Rule(ref_ridx) => { - if let Some(action_type) = grm.actiontype(ref_ridx).as_ref() { - str::parse::(action_type)? - } else { - let mut s = String::from("\n"); - let rule_span = grm.rule_name_span(ref_ridx); - s.push_str(&diag.file_location_msg("Error", Some(rule_span))); - s.push('\n'); - s.push_str(&diag.underline_span_with_text( - rule_span, - "Rule missing action type".to_string(), - '^', - )); - return Err(ErrorString(s).into()); - } + let action_type = grm.actiontype(ref_ridx) + .as_ref() + .expect("actiontype should have been checked during complete_and_validate for this YaccKind"); + str::parse::(action_type)? } Symbol::Token(_) => { let lexemet = From 9e956c2d6dacd60f06e60bde81555ba0d0240b1d Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 8 Aug 2026 18:56:36 -0700 Subject: [PATCH 02/10] Move check that there is action code to complete_and_validate. --- cfgrammar/src/lib/yacc/ast.rs | 15 +++++++++++---- cfgrammar/src/lib/yacc/grammar.rs | 8 ++++---- cfgrammar/src/lib/yacc/parser.rs | 3 +++ lrpar/src/lib/ctbuilder.rs | 13 +------------ 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 90d40f607..7eb68d384 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -313,12 +313,13 @@ impl GrammarAST { /// 4) If a production has a precedence token, then it references a declared token /// 5) Every token declared with %epp matches a known token /// 6) If `yacc_kind` is specified, perform any kind specific validation. - /// * If the kind requires an action type, check that each rule has one. + /// * If the kind requires an action type, check that each rule has one + /// * That each production has action code pub(crate) fn complete_and_validate( &mut self, yacc_kind: Option, ) -> Result<(), YaccGrammarError> { - let kind_requires_actiont = matches!( + let kind_requires_action_checks = matches!( yacc_kind, Some(YaccKind::Original(YaccOriginalActionKind::UserAction)) | Some(YaccKind::Grmtools) ); @@ -340,7 +341,7 @@ impl GrammarAST { } } for rule in self.rules.values() { - if kind_requires_actiont && rule.actiont.is_none() { + if kind_requires_action_checks && rule.actiont.is_none() { return Err(YaccGrammarError { kind: YaccGrammarErrorKind::MissingActionType, spans: vec![rule.name.1], @@ -348,6 +349,12 @@ impl GrammarAST { } for &pidx in &rule.pidxs { let prod = &self.prods[pidx]; + if kind_requires_action_checks && prod.action.is_none() { + 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 { @@ -879,7 +886,7 @@ start: "a"; %actiontype () %token a %% -start: "a"; +start: "a" { }; "#, ); assert!(ast_validity.errors().is_empty()); diff --git a/cfgrammar/src/lib/yacc/grammar.rs b/cfgrammar/src/lib/yacc/grammar.rs index 2bfae2efc..a03010ee7 100644 --- a/cfgrammar/src/lib/yacc/grammar.rs +++ b/cfgrammar/src/lib/yacc/grammar.rs @@ -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::::from_str(src).unwrap(); @@ -1623,7 +1623,7 @@ mod test { yacckind: YaccKind::Grmtools, } %% - Start -> () : ; + Start -> () : {}; "#; YaccGrammar::::from_str(src).unwrap(); let src = r#" @@ -1631,7 +1631,7 @@ mod test { yacckind: YaccKind::Grmtools } %% - Start -> () : ; + Start -> () : {}; "#; YaccGrammar::::from_str(src).unwrap(); } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 9721fc4de..1fe88b704 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -36,6 +36,7 @@ pub enum YaccGrammarErrorKind { IncompleteRule, IncompleteComment, IncompleteAction, + MissingActionCode, MissingActionType, MissingColon, MissingRightArrow, @@ -98,6 +99,7 @@ impl fmt::Display for YaccGrammarErrorKind { YaccGrammarErrorKind::IncompleteRule => "Incomplete rule", YaccGrammarErrorKind::IncompleteComment => "Incomplete comment", YaccGrammarErrorKind::IncompleteAction => "Incomplete action", + YaccGrammarErrorKind::MissingActionCode => "Production is missing action code", YaccGrammarErrorKind::MissingActionType => "Missing action type", YaccGrammarErrorKind::MissingColon => "Missing ':'", YaccGrammarErrorKind::MissingRightArrow => "Missing '->'", @@ -247,6 +249,7 @@ impl Spanned for YaccGrammarError { | YaccGrammarErrorKind::IncompleteRule | YaccGrammarErrorKind::IncompleteComment | YaccGrammarErrorKind::IncompleteAction + | YaccGrammarErrorKind::MissingActionCode | YaccGrammarErrorKind::MissingActionType | YaccGrammarErrorKind::MissingColon | YaccGrammarErrorKind::MissingRightArrow diff --git a/lrpar/src/lib/ctbuilder.rs b/lrpar/src/lib/ctbuilder.rs index c2b401234..a4cf21d96 100644 --- a/lrpar/src/lib/ctbuilder.rs +++ b/lrpar/src/lib/ctbuilder.rs @@ -1690,18 +1690,7 @@ where // Iterate over all $-arguments and replace them with their respective // element from the argument vector (e.g. $1 is replaced by args[0]). - let pre_action = grm.action(pidx).as_ref().ok_or_else(|| { - let mut s = String::from("\n"); - let span = grm.prod_span(pidx); - s.push_str(&diag.file_location_msg("Error", Some(span))); - s.push('\n'); - s.push_str(&diag.underline_span_with_text( - span, - "Production is missing action code".to_string(), - '^', - )); - ErrorString(s) - })?; + let pre_action = grm.action(pidx).as_ref().expect("action code should have been checked during complete_and_validate for this YaccKind"); let mut last = 0; let mut outs = String::new(); loop { From 6313b9653f2e1e907126391f1fb7db467fd58fc4 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 8 Aug 2026 19:28:48 -0700 Subject: [PATCH 03/10] Move action '$' variable checks to complete_and_validate --- cfgrammar/src/lib/yacc/ast.rs | 34 +++++++++++++++++++++++++++----- cfgrammar/src/lib/yacc/parser.rs | 5 +++++ lrpar/src/lib/ctbuilder.rs | 25 ++++------------------- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 7eb68d384..e9cbac832 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -349,12 +349,36 @@ impl GrammarAST { } for &pidx in &rule.pidxs { let prod = &self.prods[pidx]; - if kind_requires_action_checks && prod.action.is_none() { - return Err(YaccGrammarError { - kind: YaccGrammarErrorKind::MissingActionCode, - spans: vec![prod.prod_span] - }); + 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()))) + { + return Err(YaccGrammarError { + kind: YaccGrammarErrorKind::UnrecognizedDollarVariable, + spans: vec![Span::new( + action_span.start() + last + off + "$".len(), + action_span.end(), + )], + }); + } else { + last = last + off + "$$".len(); + } + } + } 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 { diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 1fe88b704..7bb45df81 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -59,6 +59,7 @@ pub enum YaccGrammarErrorKind { InvalidString, NoStartRule, UnknownSymbol, + UnrecognizedDollarVariable, InvalidStartRule(String), UnknownRuleRef(String), UnknownToken(String), @@ -111,6 +112,9 @@ impl fmt::Display for YaccGrammarErrorKind { YaccGrammarErrorKind::UnknownDeclaration => "Unknown declaration", YaccGrammarErrorKind::DuplicatePrecedence => "Token has multiple precedences specified", YaccGrammarErrorKind::PrecNotFollowedByToken => "%prec not followed by token name", + YaccGrammarErrorKind::UnrecognizedDollarVariable => { + "Unrecognized action variable following '$'" + } YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration => { "Duplicated %avoid_insert declaration" } @@ -264,6 +268,7 @@ impl Spanned for YaccGrammarError { | YaccGrammarErrorKind::InvalidString | YaccGrammarErrorKind::NoStartRule | YaccGrammarErrorKind::UnknownSymbol + | YaccGrammarErrorKind::UnrecognizedDollarVariable | YaccGrammarErrorKind::InvalidStartRule(_) | YaccGrammarErrorKind::UnknownRuleRef(_) | YaccGrammarErrorKind::UnknownToken(_) diff --git a/lrpar/src/lib/ctbuilder.rs b/lrpar/src/lib/ctbuilder.rs index a4cf21d96..4ef940758 100644 --- a/lrpar/src/lib/ctbuilder.rs +++ b/lrpar/src/lib/ctbuilder.rs @@ -23,7 +23,7 @@ use crate::{ use crate::unstable_api::UnstableApi; use cfgrammar::{ - Location, RIdx, Span, Symbol, + Location, RIdx, Symbol, header::{ GrmtoolsSectionParser, Header, HeaderError, HeaderErrorKind, HeaderValue, Namespaced, Setting, Value, @@ -954,7 +954,6 @@ where &derived_mod_name, outp, &format!("/* CACHE INFORMATION {} */\n", cache), - &yacc_diag, )?; let conflicts = if stable.conflicts().is_some() { Some((sgraph, stable)) @@ -1083,14 +1082,13 @@ where mod_name: &str, outp_rs: P, cache: &str, - diag: &SpannedDiagnosticFormatter, ) -> Result<(), Box> { let visibility = self.visibility.clone(); let user_actions = if let Some( YaccKind::Original(YaccOriginalActionKind::UserAction) | YaccKind::Grmtools, ) = self.yacckind { - Some(self.gen_user_actions(grm, diag)?) + Some(self.gen_user_actions(grm)?) } else { None }; @@ -1611,11 +1609,7 @@ where } /// Generate the user action functions (if any). - fn gen_user_actions( - &self, - grm: &YaccGrammar, - diag: &SpannedDiagnosticFormatter, - ) -> Result> { + fn gen_user_actions(&self, grm: &YaccGrammar) -> Result> { let programs = grm .programs() .as_ref() @@ -1714,18 +1708,7 @@ where write!(outs, "{prefix}arg_", prefix = ACTION_PREFIX).ok(); last = last + off + "$".len(); } else { - let span = grm.action_span(pidx).unwrap(); - let inner_span = - Span::new(span.start() + last + off + "$".len(), span.end()); - let mut s = String::from("\n"); - s.push_str(&diag.file_location_msg("Error", Some(inner_span))); - s.push('\n'); - s.push_str(&diag.underline_span_with_text( - inner_span, - "Unknown text following '$'".to_string(), - '^', - )); - return Err(ErrorString(s).into()); + unreachable!("action variables checked during complete_and_validate"); } } None => { From ca3a9b1317ff5e3064d2dcfaf681992c6c35f5b4 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 8 Aug 2026 20:22:52 -0700 Subject: [PATCH 04/10] Add test for unrecognized action variables --- cfgrammar/src/lib/yacc/ast.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index e9cbac832..b288acd38 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -929,4 +929,24 @@ start: "a" { }; }) ); } + + #[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::UnrecognizedDollarVariable, + spans: vec![Span::new(33, 37)], + }] + ); + } } From ff5a2d6fb98e0eaf1fde3b4fb907b54f171e6c74 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 8 Aug 2026 21:42:40 -0700 Subject: [PATCH 05/10] Add docs for action variable check --- cfgrammar/src/lib/yacc/ast.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index b288acd38..a23ff2387 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -315,6 +315,7 @@ impl GrammarAST { /// 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, From c2d027c1c253a835a97c387bf4d60ad2eeb97dc9 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 8 Aug 2026 21:42:50 -0700 Subject: [PATCH 06/10] Rename error kind --- cfgrammar/src/lib/yacc/ast.rs | 4 ++-- cfgrammar/src/lib/yacc/parser.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index a23ff2387..4fe9cc0d6 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -362,7 +362,7 @@ impl GrammarAST { .starts_with(|c: char| c.is_numeric()))) { return Err(YaccGrammarError { - kind: YaccGrammarErrorKind::UnrecognizedDollarVariable, + kind: YaccGrammarErrorKind::UnrecognisedActionVariable, spans: vec![Span::new( action_span.start() + last + off + "$".len(), action_span.end(), @@ -945,7 +945,7 @@ start -> () : "a" { $foo; }; assert_eq!( ast_validity.errors(), vec![YaccGrammarError { - kind: YaccGrammarErrorKind::UnrecognizedDollarVariable, + kind: YaccGrammarErrorKind::UnrecognisedActionVariable, spans: vec![Span::new(33, 37)], }] ); diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 7bb45df81..c9389a952 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -59,7 +59,7 @@ pub enum YaccGrammarErrorKind { InvalidString, NoStartRule, UnknownSymbol, - UnrecognizedDollarVariable, + UnrecognisedActionVariable, InvalidStartRule(String), UnknownRuleRef(String), UnknownToken(String), @@ -112,7 +112,7 @@ impl fmt::Display for YaccGrammarErrorKind { YaccGrammarErrorKind::UnknownDeclaration => "Unknown declaration", YaccGrammarErrorKind::DuplicatePrecedence => "Token has multiple precedences specified", YaccGrammarErrorKind::PrecNotFollowedByToken => "%prec not followed by token name", - YaccGrammarErrorKind::UnrecognizedDollarVariable => { + YaccGrammarErrorKind::UnrecognisedActionVariable => { "Unrecognized action variable following '$'" } YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration => { @@ -268,7 +268,7 @@ impl Spanned for YaccGrammarError { | YaccGrammarErrorKind::InvalidString | YaccGrammarErrorKind::NoStartRule | YaccGrammarErrorKind::UnknownSymbol - | YaccGrammarErrorKind::UnrecognizedDollarVariable + | YaccGrammarErrorKind::UnrecognisedActionVariable | YaccGrammarErrorKind::InvalidStartRule(_) | YaccGrammarErrorKind::UnknownRuleRef(_) | YaccGrammarErrorKind::UnknownToken(_) From d244993fcfca5fb7cc79f0923c02a19208924d9b Mon Sep 17 00:00:00 2001 From: matt rice Date: Mon, 10 Aug 2026 00:17:06 -0700 Subject: [PATCH 07/10] Fix the UnrecognisedActionVariable span calculation --- cfgrammar/src/lib/yacc/ast.rs | 44 +++++++++++++++++++++++++++++--- cfgrammar/src/lib/yacc/parser.rs | 6 ++--- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 4fe9cc0d6..8449bc348 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -352,6 +352,9 @@ impl GrammarAST { let prod = &self.prods[pidx]; if kind_requires_action_checks { if let Some((action_code, action_span)) = prod.action.as_ref() { + let leading_bytes = action_code.len() - action_code.trim_start().len(); + let trailing_bytes = action_code.len() - action_code.trim_end().len(); + let action_code = action_code.trim(); let mut last = 0; while let Some(off) = action_code[last..].find('$') { if !(action_code[last + off..].starts_with("$$") @@ -364,12 +367,12 @@ impl GrammarAST { return Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnrecognisedActionVariable, spans: vec![Span::new( - action_span.start() + last + off + "$".len(), - action_span.end(), + action_span.start() + leading_bytes + last + off, + action_span.end() - trailing_bytes, )], }); } else { - last = last + off + "$$".len(); + last = last + off + 2; } } } else { @@ -946,7 +949,40 @@ start -> () : "a" { $foo; }; ast_validity.errors(), vec![YaccGrammarError { kind: YaccGrammarErrorKind::UnrecognisedActionVariable, - spans: vec![Span::new(33, 37)], + spans: vec![Span::new(33, 38)], + }] + ); + + 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)], + }] + ); + + // We expect the error to extend to the end of the action code minus any whitespace + 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, 37)], }] ); } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index c9389a952..539231b5e 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -853,7 +853,7 @@ impl YaccParser<'_> { Err(self.mk_error(YaccGrammarErrorKind::IncompleteAction, i)) } else { debug_assert!(self.lookahead_is("}", j).is_some()); - let s = self.src[i + '{'.len_utf8()..j].trim().to_string(); + let s = self.src[i + '{'.len_utf8()..j].to_string(); Ok((j + '}'.len_utf8(), s)) } } @@ -2298,12 +2298,12 @@ x" ", ) .unwrap(); - let action_str = "println!(\"test\");".to_string(); + let action_str = " println!(\"test\"); ".to_string(); assert_eq!( grm.prods[grm.rules["A"].pidxs[0]].action, Some((action_str.clone(), Span::new(34, 34 + action_str.len()))) ); - let action_str = "add($1, $2);".to_string(); + let action_str = " add($1, $2); ".to_string(); assert_eq!( grm.prods[grm.rules["B"].pidxs[0]].action, Some((action_str.clone(), Span::new(90, 90 + action_str.len()))) From 9f559936d9723fbce3f6de7ff36b4958b6bfb40c Mon Sep 17 00:00:00 2001 From: matt rice Date: Mon, 10 Aug 2026 01:18:34 -0700 Subject: [PATCH 08/10] Improve UnrecognisedActionSpan --- cfgrammar/src/lib/yacc/ast.rs | 13 ++++++++----- cfgrammar/src/lib/yacc/parser.rs | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 8449bc348..a1c9953b2 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -353,7 +353,6 @@ impl GrammarAST { if kind_requires_action_checks { if let Some((action_code, action_span)) = prod.action.as_ref() { let leading_bytes = action_code.len() - action_code.trim_start().len(); - let trailing_bytes = action_code.len() - action_code.trim_end().len(); let action_code = action_code.trim(); let mut last = 0; while let Some(off) = action_code[last..].find('$') { @@ -364,11 +363,15 @@ impl GrammarAST { && 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 start_pos = action_span.start() + leading_bytes + last + off; + let var_end_pos = m.map(|m| start_pos + 1 + m.end()).unwrap_or(start_pos + 1); return Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnrecognisedActionVariable, spans: vec![Span::new( - action_span.start() + leading_bytes + last + off, - action_span.end() - trailing_bytes, + start_pos, + var_end_pos, )], }); } else { @@ -949,7 +952,7 @@ start -> () : "a" { $foo; }; ast_validity.errors(), vec![YaccGrammarError { kind: YaccGrammarErrorKind::UnrecognisedActionVariable, - spans: vec![Span::new(33, 38)], + spans: vec![Span::new(33, 37)], }] ); @@ -982,7 +985,7 @@ start -> () : "a" {$;;;; }; ast_validity.errors(), vec![YaccGrammarError { kind: YaccGrammarErrorKind::UnrecognisedActionVariable, - spans: vec![Span::new(32, 37)], + spans: vec![Span::new(32, 33)], }] ); } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 539231b5e..6d76e0f8a 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -297,7 +297,7 @@ pub(crate) struct YaccParser<'a> { global_actiontype: Option<(String, Span)>, } -static RE_NAME: LazyLock = +pub(crate) static RE_NAME: LazyLock = LazyLock::new(|| Regex::new(r"^[a-zA-Z_.][a-zA-Z0-9_.]*").unwrap()); static RE_TOKEN: LazyLock = LazyLock::new(|| Regex::new("^(?:(\".+?\")|('.+?')|([a-zA-Z_][a-zA-Z_0-9]*))").unwrap()); From d148d1be7d9ec18192bd902597f69005cb9e1975 Mon Sep 17 00:00:00 2001 From: matt rice Date: Mon, 10 Aug 2026 01:48:07 -0700 Subject: [PATCH 09/10] Remove now out of date comment --- cfgrammar/src/lib/yacc/ast.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index a1c9953b2..311239c81 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -972,7 +972,6 @@ start -> () : "a" {$}; }] ); - // We expect the error to extend to the end of the action code minus any whitespace let ast_validity = ASTWithValidityInfo::new( YaccKind::Grmtools, r#" From a840a91d7d078b4203801f30fff5d8e005c938a3 Mon Sep 17 00:00:00 2001 From: matt rice Date: Mon, 10 Aug 2026 04:32:10 -0700 Subject: [PATCH 10/10] Remove extraneous string trimming --- cfgrammar/src/lib/yacc/ast.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 311239c81..288c860da 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -352,8 +352,6 @@ impl GrammarAST { let prod = &self.prods[pidx]; if kind_requires_action_checks { if let Some((action_code, action_span)) = prod.action.as_ref() { - let leading_bytes = action_code.len() - action_code.trim_start().len(); - let action_code = action_code.trim(); let mut last = 0; while let Some(off) = action_code[last..].find('$') { if !(action_code[last + off..].starts_with("$$") @@ -364,15 +362,15 @@ impl GrammarAST { .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 start_pos = action_span.start() + leading_bytes + last + off; - let var_end_pos = m.map(|m| start_pos + 1 + m.end()).unwrap_or(start_pos + 1); + 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( - start_pos, - var_end_pos, - )], + spans: vec![Span::new(var_start_pos, var_end_pos)], }); } else { last = last + off + 2;