From 20351dfa13984436e51f7ad775fbe1ed132003b2 Mon Sep 17 00:00:00 2001 From: matt rice Date: Fri, 22 Dec 2023 23:39:58 -0800 Subject: [PATCH 1/7] first attempt at using channels for error sending --- cfgrammar/src/lib/yacc/ast.rs | 275 ++++++++++++++----- cfgrammar/src/lib/yacc/grammar.rs | 2 +- cfgrammar/src/lib/yacc/parser.rs | 432 +++++++++++++++++------------- 3 files changed, 457 insertions(+), 252 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 04c269130..5a9d85c2f 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -6,35 +6,160 @@ use std::{ use indexmap::{IndexMap, IndexSet}; use super::{ - parser::YaccParser, Precedence, YaccGrammarError, YaccGrammarErrorKind, YaccGrammarWarning, - YaccGrammarWarningKind, YaccKind, + parser::YaccGrammarConstructionFailure, parser::YaccParser, Precedence, YaccGrammarError, + YaccGrammarErrorKind, YaccGrammarWarning, YaccGrammarWarningKind, YaccKind, }; - use crate::Span; +use std::path; +use std::sync::mpsc; /// Contains a `GrammarAST` structure produced from a grammar source file. /// As well as any errors which occurred during the construction of the AST. pub struct ASTWithValidityInfo { pub(crate) ast: GrammarAST, - pub(crate) errs: Vec, + pub(crate) error_receiver: Option>, + pub(crate) error_sender: ErrorSender, +} + +#[allow(dead_code)] +pub struct AstBuilder<'a> { + kind: YaccKind, + grammar_src: Option<&'a str>, + grammar_path: Option, + error_receiver: Option>, + error_sender: ErrorSender, } -impl ASTWithValidityInfo { +pub struct ErrorSender { + error_occurred: bool, + sender: Option>, +} + +#[allow(dead_code)] +impl ErrorSender { + pub(crate) fn new(sender: mpsc::Sender) -> Self { + Self { + error_occurred: false, + sender: Some(sender), + } + } + pub(crate) fn close(&mut self) { + drop(self.sender.take()) + } + + pub(crate) fn send( + &mut self, + e: YaccGrammarError, + ) -> Result<(), mpsc::SendError> { + self.error_occurred = true; + if let Some(sender) = &self.sender { + sender.send(e) + } else { + Err(mpsc::SendError(e)) + } + } + pub(crate) fn error_occurred(&self) -> bool { + self.error_occurred + } +} + +#[allow(dead_code)] +impl<'a> AstBuilder<'a> { + fn path(mut self, path: &path::Path) -> Self { + self.grammar_path = Some(path.to_owned()); + self + } + fn yacc_kind(mut self, kind: YaccKind) -> Self { + self.kind = kind; + self + } + + fn error_receiver(&mut self) -> mpsc::Receiver { + self.error_receiver.take().unwrap() + } + + fn build(self) -> ASTWithValidityInfo { + ASTWithValidityInfo::new_with_error_channel( + self.kind, + self.grammar_src.unwrap(), + self.error_sender, + self.error_receiver, + ) + } +} + +impl<'a> ASTWithValidityInfo { + pub fn builder() -> AstBuilder<'a> { + let (error_sender, error_receiver) = std::sync::mpsc::channel(); + AstBuilder { + kind: YaccKind::Grmtools, + grammar_src: None, + grammar_path: None, + error_receiver: Some(error_receiver), + error_sender: ErrorSender::new(error_sender), + } + } + + pub fn new_with_error_channel( + yacc_kind: YaccKind, + s: &str, + mut error_sender: ErrorSender, + error_receiver: Option>, + ) -> Self { + let ast = match yacc_kind { + YaccKind::Original(_) | YaccKind::Grmtools | YaccKind::Eco => { + let mut yp = YaccParser::new(yacc_kind, s.to_string()); + yp.parse_with_error_channel(&mut error_sender).unwrap(); + let mut ast = yp.ast(); + ast.complete_and_validate_with_error_channel(&mut error_sender) + .ok(); + ast + } + }; + error_sender.close(); + ASTWithValidityInfo { + ast, + error_receiver, + error_sender, + } + } + + #[cfg(test)] + pub(crate) fn validate_ast(mut ast: GrammarAST) -> Self { + let (error_sender, error_receiver) = mpsc::channel(); + let mut error_sender = ErrorSender::new(error_sender); + ast.complete_and_validate_with_error_channel(&mut error_sender) + .ok(); + error_sender.close(); + ASTWithValidityInfo { + ast, + error_receiver: Some(error_receiver), + error_sender, + } + } + /// Parses a source file into an AST, returning an ast and any errors that were /// encountered during the construction of it. The `ASTWithValidityInfo` can be /// then unused to construct a `YaccGrammar`, which will either produce an /// `Ok(YaccGrammar)` or an `Err` which includes these errors. pub fn new(yacc_kind: YaccKind, s: &str) -> Self { - let mut errs = Vec::new(); + let (error_sender, error_receiver) = mpsc::channel(); + let mut error_sender = ErrorSender::new(error_sender); let ast = match yacc_kind { YaccKind::Original(_) | YaccKind::Grmtools | YaccKind::Eco => { let mut yp = YaccParser::new(yacc_kind, s.to_string()); - yp.parse().map_err(|e| errs.extend(e)).ok(); + let _ = yp.parse_with_error_channel(&mut error_sender).ok(); let mut ast = yp.ast(); - ast.complete_and_validate().map_err(|e| errs.push(e)).ok(); + ast.complete_and_validate_with_error_channel(&mut error_sender) + .ok(); + error_sender.close(); ast } }; - ASTWithValidityInfo { ast, errs } + ASTWithValidityInfo { + ast, + error_receiver: Some(error_receiver), + error_sender, + } } /// Returns a `GrammarAST` constructed as the result of parsing a source file. @@ -48,12 +173,16 @@ impl ASTWithValidityInfo { /// Returns whether any errors where encountered during the /// parsing and validation of the AST during it's construction. pub fn is_valid(&self) -> bool { - self.errs.is_empty() + !self.error_sender.error_occurred() } /// Returns all errors which were encountered during AST construction. - pub fn errors(&self) -> &[YaccGrammarError] { - self.errs.as_slice() + pub fn errors(&self) -> Vec { + let errs = self + .error_receiver + .as_ref() + .map_or(vec![], |errs| errs.iter().collect::>()); + errs } } @@ -209,20 +338,25 @@ 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 /// If the validation succeeds, None is returned. - pub(crate) fn complete_and_validate(&mut self) -> Result<(), YaccGrammarError> { + pub(crate) fn complete_and_validate_with_error_channel( + &mut self, + errs: &mut ErrorSender, + ) -> Result<(), YaccGrammarConstructionFailure> { match self.start { None => { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::NoStartRule, spans: vec![Span::new(0, 0)], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } Some((ref s, span)) => { if !self.rules.contains_key(s) { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::InvalidStartRule(s.clone()), spans: vec![span], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } } @@ -231,34 +365,38 @@ impl GrammarAST { let prod = &self.prods[pidx]; if let Some(ref n) = prod.precedence { if !self.tokens.contains(n) { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(n.clone()), spans: vec![Span::new(0, 0)], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } if !self.precs.contains_key(n) { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::NoPrecForToken(n.clone()), spans: vec![Span::new(0, 0)], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } for sym in &prod.symbols { match *sym { Symbol::Rule(ref name, span) => { if !self.rules.contains_key(name) { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(name.clone()), spans: vec![span], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } Symbol::Token(ref name, span) => { if !self.tokens.contains(name) { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(name.clone()), spans: vec![span], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } } @@ -275,28 +413,31 @@ impl GrammarAST { continue; } } - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownEPP(k.clone()), spans: vec![*sp], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } for sym in &self.expect_unused { match sym { Symbol::Rule(sym_name, sym_span) => { if self.get_rule(sym_name).is_none() { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(sym_name.clone()), spans: vec![*sym_span], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } Symbol::Token(sym_name, sym_span) => { if !self.has_token(sym_name) { - return Err(YaccGrammarError { + errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(sym_name.clone()), spans: vec![*sym_span], - }); + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } } @@ -396,7 +537,7 @@ impl GrammarAST { mod test { use super::{ super::{AssocKind, Precedence}, - GrammarAST, Span, Symbol, YaccGrammarError, YaccGrammarErrorKind, + ASTWithValidityInfo, GrammarAST, Span, Symbol, YaccGrammarError, YaccGrammarErrorKind, }; fn rule(n: &str) -> Symbol { @@ -409,12 +550,12 @@ mod test { #[test] fn test_empty_grammar() { - let mut grm = GrammarAST::new(); - match grm.complete_and_validate() { - Err(YaccGrammarError { + let grm = GrammarAST::new(); + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::NoStartRule, .. - }) => (), + }] => (), _ => panic!("Validation error"), } } @@ -426,11 +567,12 @@ 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); - match grm.complete_and_validate() { - Err(YaccGrammarError { + + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::InvalidStartRule(_), .. - }) => (), + }] => (), _ => panic!("Validation error"), } } @@ -442,7 +584,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); - assert!(grm.complete_and_validate().is_ok()); + assert!(ASTWithValidityInfo::validate_ast(grm).is_valid()); } #[test] @@ -454,7 +596,7 @@ mod test { grm.add_rule(("B".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![rule("B")], None, None); grm.add_prod("B".to_string(), vec![], None, None); - assert!(grm.complete_and_validate().is_ok()); + assert!(ASTWithValidityInfo::validate_ast(grm).is_valid()); } #[test] @@ -464,11 +606,11 @@ 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); - match grm.complete_and_validate() { - Err(YaccGrammarError { + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(_), .. - }) => (), + }] => (), _ => panic!("Validation error"), } } @@ -481,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); - assert!(grm.complete_and_validate().is_ok()); + assert!(ASTWithValidityInfo::validate_ast(grm).is_valid()); } #[test] @@ -494,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![rule("b")], None, None); - assert!(grm.complete_and_validate().is_err()); + assert!(!ASTWithValidityInfo::validate_ast(grm).is_valid()); } #[test] @@ -504,11 +646,11 @@ 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); - match grm.complete_and_validate() { - Err(YaccGrammarError { + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(_), .. - }) => (), + }] => (), _ => panic!("Validation error"), } } @@ -520,11 +662,11 @@ 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"), token("b")], None, None); - match grm.complete_and_validate() { - Err(YaccGrammarError { + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(_), .. - }) => (), + }] => (), _ => panic!("Validation error"), } } @@ -538,11 +680,11 @@ mod test { grm.add_prod("A".to_string(), vec![], None, None); grm.epp .insert("k".to_owned(), (empty_span, ("v".to_owned(), empty_span))); - match grm.complete_and_validate() { - Err(YaccGrammarError { + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::UnknownEPP(_), spans, - }) if spans.len() == 1 && spans[0] == Span::new(2, 3) => (), + }] if spans.len() == 1 && spans[0] == Span::new(2, 3) => (), _ => panic!("Validation error"), } } @@ -570,7 +712,7 @@ mod test { Some("b".to_string()), None, ); - assert!(grm.complete_and_validate().is_ok()); + assert!(ASTWithValidityInfo::validate_ast(grm).is_valid()); } #[test] @@ -585,19 +727,28 @@ mod test { Some("b".to_string()), None, ); - match grm.complete_and_validate() { - Err(YaccGrammarError { + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(_), .. - }) => (), + }] => (), _ => panic!("Validation error"), } + let mut grm = GrammarAST::new(); + 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")], + Some("b".to_string()), + None, + ); grm.tokens.insert("b".to_string()); - match grm.complete_and_validate() { - Err(YaccGrammarError { + match ASTWithValidityInfo::validate_ast(grm).errors().as_slice() { + [YaccGrammarError { kind: YaccGrammarErrorKind::NoPrecForToken(_), .. - }) => (), + }] => (), _ => panic!("Validation error"), } } diff --git a/cfgrammar/src/lib/yacc/grammar.rs b/cfgrammar/src/lib/yacc/grammar.rs index 7f3e0870c..c155bf82e 100644 --- a/cfgrammar/src/lib/yacc/grammar.rs +++ b/cfgrammar/src/lib/yacc/grammar.rs @@ -115,7 +115,7 @@ where ast_validation: &ast::ASTWithValidityInfo, ) -> YaccGrammarResult { if !ast_validation.is_valid() { - return Err(ast_validation.errs.clone()); + return Err(ast_validation.errors()); } let ast = &ast_validation.ast; // Check that StorageT is big enough to hold RIdx/PIdx/SIdx/TIdx values; after these diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index fc98307b2..68ad58d41 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -10,6 +10,7 @@ use std::{ error::Error, fmt, str::FromStr, + sync::mpsc, }; pub type YaccGrammarResult = Result>; @@ -17,10 +18,31 @@ pub type YaccGrammarResult = Result>; use crate::{Span, Spanned}; use super::{ - ast::{GrammarAST, Symbol}, + ast::{ErrorSender, GrammarAST, Symbol}, AssocKind, Precedence, YaccKind, }; +#[derive(Debug)] +pub enum YaccGrammarConstructionFailure { + ConstructionFailure, + ErrorChannel(mpsc::SendError), +} + +impl From> for YaccGrammarConstructionFailure { + fn from(it: mpsc::SendError) -> Self { + Self::ErrorChannel(it) + } +} + +impl fmt::Display for YaccGrammarConstructionFailure { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::ConstructionFailure => write!(f, "Failure to send an error over a channel."), + Self::ErrorChannel(e) => write!(f, "Error constructing YaccGrammar: {}", e), + } + } +} + /// The various different possible Yacc parser errors. #[derive(Debug, PartialEq, Eq, Clone)] pub enum YaccGrammarErrorKind { @@ -269,27 +291,6 @@ lazy_static! { Regex::new("^(?:(\".+?\")|('.+?')|([a-zA-Z_][a-zA-Z_0-9]*))").unwrap(); } -fn add_duplicate_occurrence( - errs: &mut Vec, - kind: YaccGrammarErrorKind, - orig_span: Span, - dup_span: Span, -) { - if !errs.iter_mut().any(|e| { - if e.kind == kind && e.spans[0] == orig_span { - e.spans.push(dup_span); - true - } else { - false - } - }) { - errs.push(YaccGrammarError { - kind, - spans: vec![orig_span, dup_span], - }); - } -} - /// The actual parser is intended to be entirely opaque from outside users. impl YaccParser { pub(crate) fn new(yacc_kind: YaccKind, src: String) -> YaccParser { @@ -302,37 +303,35 @@ impl YaccParser { } } - pub(crate) fn parse(&mut self) -> YaccGrammarResult { - let mut errs: Vec = Vec::new(); + pub(crate) fn parse_with_error_channel( + &mut self, + errs: &mut ErrorSender, + ) -> Result { // We pass around an index into the *bytes* of self.src. We guarantee that at all times // this points to the beginning of a UTF-8 character (since multibyte characters exist, not // every byte within the string is also a valid character). - let mut result = self.parse_declarations(0, &mut errs); - result = self.parse_rules(match result { - Ok(i) => i, - Err(e) => { - errs.push(e); - return Err(errs); - } - }); - result = self.parse_programs( - match result { - Ok(i) => i, - Err(e) => { - errs.push(e); - return Err(errs); - } - }, - &mut errs, - ); + let mut i = self.parse_declarations(0, errs)?; + i = self.parse_rules(i, errs)?; + i = self.parse_programs(i, errs)?; + + if errs.error_occurred() { + Err(YaccGrammarConstructionFailure::ConstructionFailure) + } else { + Ok(i) + } + } + #[cfg(test)] + pub(crate) fn parse(&mut self) -> Result> { + let (error_sender, error_receiver) = mpsc::channel(); + let mut error_sender = ErrorSender::new(error_sender); + let result = self.parse_with_error_channel(&mut error_sender); match result { - Ok(i) if errs.is_empty() => Ok(i), - Err(e) => { - errs.push(e); - Err(errs) + Ok(x) => Ok(x), + Err(_) => { + drop(error_sender); + Err(error_receiver.iter().collect()) } - _ => Err(errs), } } @@ -343,213 +342,207 @@ impl YaccParser { fn parse_declarations( &mut self, mut i: usize, - errs: &mut Vec, - ) -> Result { - i = self.parse_ws(i, true)?; + errs: &mut ErrorSender, + ) -> Result { + i = self.parse_ws(i, true, errs)?; let mut prec_level = 0; while i < self.src.len() { if self.lookahead_is("%%", i).is_some() { return Ok(i); } if let Some(j) = self.lookahead_is("%token", i) { - i = self.parse_ws(j, false)?; + i = self.parse_ws(j, false, errs)?; while i < self.src.len() && self.lookahead_is("%", i).is_none() { - let (j, n, span) = self.parse_token(i)?; + let (j, n, span) = self.parse_token(i, errs)?; if self.ast.tokens.insert(n) { self.ast.spans.push(span); } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; } continue; } if let YaccKind::Original(_) = self.yacc_kind { if let Some(j) = self.lookahead_is("%actiontype", i) { - i = self.parse_ws(j, false)?; - let (j, n) = self.parse_to_eol(i)?; + i = self.parse_ws(j, false, errs)?; + let (j, n) = self.parse_to_eol(i, errs)?; let span = Span::new(i, j); if let Some((_, orig_span)) = self.global_actiontype { - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateActiontypeDeclaration, - orig_span, - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateActiontypeDeclaration, + spans: vec![orig_span, span], + })? } else { self.global_actiontype = Some((n, span)); } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } } if let Some(j) = self.lookahead_is("%start", i) { - i = self.parse_ws(j, false)?; - let (j, n) = self.parse_name(i)?; + i = self.parse_ws(j, false, errs)?; + let result = self.try_parse_name(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, n) = result.unwrap(); let span = Span::new(i, j); if let Some((_, orig_span)) = self.ast.start { - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateStartDeclaration, - orig_span, - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateStartDeclaration, + spans: vec![orig_span, span], + })? } else { self.ast.start = Some((n, span)); } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } if let Some(j) = self.lookahead_is("%epp", i) { - i = self.parse_ws(j, false)?; - let (j, n, _) = self.parse_token(i)?; + i = self.parse_ws(j, false, errs)?; + let (j, n, _) = self.parse_token(i, errs)?; let span = Span::new(i, j); - i = self.parse_ws(j, false)?; - let (j, v) = self.parse_string(i)?; + i = self.parse_ws(j, false, errs)?; + let (j, v) = self.parse_string(i, errs)?; let vspan = Span::new(i, j); match self.ast.epp.entry(n) { Entry::Occupied(orig) => { let (orig_span, _) = orig.get(); - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateEPP, - *orig_span, - span, - ) + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateEPP, + spans: vec![*orig_span, span], + })?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } Entry::Vacant(epp) => { epp.insert((span, (v, vspan))); } } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } if let Some(j) = self.lookahead_is("%expect-rr", i) { - i = self.parse_ws(j, false)?; - let (j, n) = self.parse_int(i)?; + i = self.parse_ws(j, false, errs)?; + let (j, n) = self.parse_int(i, errs)?; let span = Span::new(i, j); if let Some((_, orig_span)) = self.ast.expectrr { - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateExpectRRDeclaration, - orig_span, - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateExpectRRDeclaration, + spans: vec![orig_span, span], + })?; } else { self.ast.expectrr = Some((n, span)); } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } if let Some(j) = self.lookahead_is("%expect-unused", i) { - i = self.parse_ws(j, false)?; + i = self.parse_ws(j, false, errs)?; while i < self.src.len() && self.lookahead_is("%", i).is_none() { - let j = match self.parse_name(i) { + let j = match self.try_parse_name(i) { Ok((j, n)) => { self.ast .expect_unused .push(Symbol::Rule(n, Span::new(i, j))); j } - Err(_) => match self.parse_token(i) { + Err(_) => match self.parse_token(i, errs) { Ok((j, n, span)) => { self.ast.expect_unused.push(Symbol::Token(n, span)); j } Err(_) => { - return Err(self.mk_error(YaccGrammarErrorKind::UnknownSymbol, i)) + errs.send(self.mk_error(YaccGrammarErrorKind::UnknownSymbol, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } }, }; - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; } continue; } if let Some(j) = self.lookahead_is("%expect", i) { - i = self.parse_ws(j, false)?; - let (j, n) = self.parse_int(i)?; + i = self.parse_ws(j, false, errs)?; + let (j, n) = self.parse_int(i, errs)?; let span = Span::new(i, j); if let Some((_, orig_span)) = self.ast.expect { - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateExpectDeclaration, - orig_span, - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateExpectDeclaration, + spans: vec![orig_span, span], + })?; } else { self.ast.expect = Some((n, span)); } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } if let Some(j) = self.lookahead_is("%avoid_insert", i) { - i = self.parse_ws(j, false)?; + i = self.parse_ws(j, false, errs)?; let num_newlines = self.num_newlines; if self.ast.avoid_insert.is_none() { self.ast.avoid_insert = Some(HashMap::new()); } while j < self.src.len() && self.num_newlines == num_newlines { - let (j, n, span) = self.parse_token(i)?; + let (j, n, span) = self.parse_token(i, errs)?; if self.ast.tokens.insert(n.clone()) { self.ast.spans.push(span); } match self.ast.avoid_insert.as_mut().unwrap().entry(n) { Entry::Occupied(occupied) => { - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration, - *occupied.get(), - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration, + spans: vec![*occupied.get(), span], + })?; } Entry::Vacant(vacant) => { vacant.insert(span); } } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; } continue; } if let Some(j) = self.lookahead_is("%parse-param", i) { - i = self.parse_ws(j, false)?; - let (j, name) = self.parse_to_single_colon(i)?; + i = self.parse_ws(j, false, errs)?; + let (j, name) = self.parse_to_single_colon(i, errs)?; match self.lookahead_is(":", j) { - Some(j) => i = self.parse_ws(j, false)?, + Some(j) => i = self.parse_ws(j, false, errs)?, None => { - return Err(self.mk_error(YaccGrammarErrorKind::MissingColon, j)); + errs.send(self.mk_error(YaccGrammarErrorKind::MissingColon, j))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } - let (j, ty) = self.parse_to_eol(i)?; + let (j, ty) = self.parse_to_eol(i, errs)?; self.ast.parse_param = Some((name, ty)); - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } if let YaccKind::Eco = self.yacc_kind { if let Some(j) = self.lookahead_is("%implicit_tokens", i) { - i = self.parse_ws(j, false)?; + i = self.parse_ws(j, false, errs)?; let num_newlines = self.num_newlines; if self.ast.implicit_tokens.is_none() { self.ast.implicit_tokens = Some(HashMap::new()); } while j < self.src.len() && self.num_newlines == num_newlines { - let (j, n, span) = self.parse_token(i)?; + let (j, n, span) = self.parse_token(i, errs)?; if self.ast.tokens.insert(n.clone()) { self.ast.spans.push(span); } match self.ast.implicit_tokens.as_mut().unwrap().entry(n) { Entry::Occupied(entry) => { let orig_span = *entry.get(); - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicateImplicitTokensDeclaration, - orig_span, - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicateImplicitTokensDeclaration, + spans: vec![orig_span, span], + })?; } Entry::Vacant(entry) => { entry.insert(span); } } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; } continue; } @@ -567,22 +560,21 @@ impl YaccParser { kind = AssocKind::Nonassoc; k = j; } else { - return Err(self.mk_error(YaccGrammarErrorKind::UnknownDeclaration, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::UnknownDeclaration, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } - i = self.parse_ws(k, false)?; + i = self.parse_ws(k, false, errs)?; let num_newlines = self.num_newlines; while i < self.src.len() && num_newlines == self.num_newlines { - let (j, n, span) = self.parse_token(i)?; + let (j, n, span) = self.parse_token(i, errs)?; match self.ast.precs.entry(n) { Entry::Occupied(orig) => { let (_, orig_span) = orig.get(); - add_duplicate_occurrence( - errs, - YaccGrammarErrorKind::DuplicatePrecedence, - *orig_span, - span, - ); + errs.send(YaccGrammarError { + kind: YaccGrammarErrorKind::DuplicatePrecedence, + spans: vec![*orig_span, span], + })?; } Entry::Vacant(entry) => { let prec = Precedence { @@ -593,28 +585,42 @@ impl YaccParser { } } - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; } prec_level += 1; } } debug_assert!(i == self.src.len()); - Err(self.mk_error(YaccGrammarErrorKind::PrematureEnd, i)) + errs.send(self.mk_error(YaccGrammarErrorKind::PrematureEnd, i))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) } - fn parse_rules(&mut self, mut i: usize) -> Result { + fn parse_rules( + &mut self, + mut i: usize, + errs: &mut ErrorSender, + ) -> Result { // self.parse_declarations should have left the input at '%%' i = self.lookahead_is("%%", i).unwrap(); - i = self.parse_ws(i, true)?; + i = self.parse_ws(i, true, errs)?; while i < self.src.len() && self.lookahead_is("%%", i).is_none() { - i = self.parse_rule(i)?; - i = self.parse_ws(i, true)?; + i = self.parse_rule(i, errs)?; + i = self.parse_ws(i, true, errs)?; } Ok(i) } - fn parse_rule(&mut self, mut i: usize) -> Result { - let (j, rn) = self.parse_name(i)?; + fn parse_rule( + &mut self, + mut i: usize, + errs: &mut ErrorSender, + ) -> Result { + let result = self.try_parse_name(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, rn) = result.unwrap(); let span = Span::new(i, j); if self.ast.start.is_none() { self.ast.start = Some((rn.clone(), span)); @@ -630,38 +636,40 @@ impl YaccParser { i = j; } YaccKind::Grmtools => { - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; if let Some(j) = self.lookahead_is("->", i) { i = j; } else { - return Err(self.mk_error(YaccGrammarErrorKind::MissingRightArrow, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::MissingRightArrow, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } - i = self.parse_ws(i, true)?; - let (j, actiont) = self.parse_to_single_colon(i)?; + i = self.parse_ws(i, true, errs)?; + let (j, actiont) = self.parse_to_single_colon(i, errs)?; if self.ast.get_rule(&rn).is_none() { self.ast.add_rule((rn.clone(), span), Some(actiont)); } i = j; } } - i = self.parse_ws(i, true)?; + i = self.parse_ws(i, true, errs)?; match self.lookahead_is(":", i) { Some(j) => i = j, None => { - return Err(self.mk_error(YaccGrammarErrorKind::MissingColon, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::MissingColon, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } let mut syms = Vec::new(); let mut prec = None; let mut action = None; - i = self.parse_ws(i, true)?; + i = self.parse_ws(i, true, errs)?; while i < self.src.len() { if let Some(j) = self.lookahead_is("|", i) { self.ast.add_prod(rn.clone(), syms, prec, action); syms = Vec::new(); prec = None; action = None; - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; continue; } else if let Some(j) = self.lookahead_is(";", i) { self.ast.add_prod(rn, syms, prec, action); @@ -669,27 +677,28 @@ impl YaccParser { } if self.lookahead_is("\"", i).is_some() || self.lookahead_is("'", i).is_some() { - let (j, sym, span) = self.parse_token(i)?; - i = self.parse_ws(j, true)?; + let (j, sym, span) = self.parse_token(i, errs)?; + i = self.parse_ws(j, true, errs)?; if self.ast.tokens.insert(sym.clone()) { self.ast.spans.push(span); } syms.push(Symbol::Token(sym, span)); } else if let Some(j) = self.lookahead_is("%prec", i) { - i = self.parse_ws(j, true)?; - let (k, sym, _) = self.parse_token(i)?; + i = self.parse_ws(j, true, errs)?; + let (k, sym, _) = self.parse_token(i, errs)?; if self.ast.tokens.contains(&sym) { prec = Some(sym); } else { - return Err(self.mk_error(YaccGrammarErrorKind::PrecNotFollowedByToken, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::PrecNotFollowedByToken, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } i = k; } else if self.lookahead_is("{", i).is_some() { - let (j, a) = self.parse_action(i)?; + let (j, a) = self.parse_action(i, errs)?; i = j; action = Some(a); } else if let Some(mut j) = self.lookahead_is("%empty", i) { - j = self.parse_ws(j, true)?; + j = self.parse_ws(j, true, errs)?; // %empty could be followed by all sorts of weird syntax errors: all we try and do // is say "does this production look like it's finished" and trust that the other // errors will be caught by other parts of the parser. @@ -698,11 +707,12 @@ impl YaccParser { || self.lookahead_is(";", j).is_some() || self.lookahead_is("{", j).is_some()) { - return Err(self.mk_error(YaccGrammarErrorKind::NonEmptyProduction, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::NonEmptyProduction, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } i = j; } else { - let (j, sym, span) = self.parse_token(i)?; + let (j, sym, span) = self.parse_token(i, errs)?; if self.ast.tokens.contains(&sym) { syms.push(Symbol::Token(sym, span)); } else { @@ -710,12 +720,13 @@ impl YaccParser { } i = j; } - i = self.parse_ws(i, true)?; + i = self.parse_ws(i, true, errs)?; } - Err(self.mk_error(YaccGrammarErrorKind::IncompleteRule, i)) + errs.send(self.mk_error(YaccGrammarErrorKind::IncompleteRule, i))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) } - fn parse_name(&self, i: usize) -> Result<(usize, String), YaccGrammarError> { + fn try_parse_name(&self, i: usize) -> Result<(usize, String), YaccGrammarError> { match RE_NAME.find(&self.src[i..]) { Some(m) => { assert_eq!(m.start(), 0); @@ -725,7 +736,11 @@ impl YaccParser { } } - fn parse_token(&self, i: usize) -> Result<(usize, String, Span), YaccGrammarError> { + fn parse_token( + &self, + i: usize, + errs: &mut ErrorSender, + ) -> Result<(usize, String, Span), YaccGrammarConstructionFailure> { match RE_TOKEN.find(&self.src[i..]) { Some(m) => { assert!(m.start() == 0 && m.end() > 0); @@ -747,11 +762,18 @@ impl YaccParser { )), } } - None => Err(self.mk_error(YaccGrammarErrorKind::IllegalString, i)), + None => { + errs.send(self.mk_error(YaccGrammarErrorKind::IllegalString, i))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) + } } } - fn parse_action(&mut self, i: usize) -> Result<(usize, String), YaccGrammarError> { + fn parse_action( + &mut self, + i: usize, + errs: &mut ErrorSender, + ) -> Result<(usize, String), YaccGrammarConstructionFailure> { debug_assert!(self.lookahead_is("{", i).is_some()); let mut j = i; let mut c = 0; // Count braces @@ -772,7 +794,8 @@ impl YaccParser { j += ch.len_utf8(); } if c > 0 { - Err(self.mk_error(YaccGrammarErrorKind::IncompleteAction, j)) + errs.send(self.mk_error(YaccGrammarErrorKind::IncompleteAction, j))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) } else { debug_assert!(self.lookahead_is("}", j).is_some()); let s = self.src[i + '{'.len_utf8()..j].trim().to_string(); @@ -783,10 +806,10 @@ impl YaccParser { fn parse_programs( &mut self, mut i: usize, - _: &mut Vec, - ) -> Result { + errs: &mut ErrorSender, + ) -> Result { if let Some(j) = self.lookahead_is("%%", i) { - i = self.parse_ws(j, true)?; + i = self.parse_ws(j, true, errs)?; let prog = self.src[i..].to_string(); i += prog.len(); self.ast.set_programs(prog); @@ -795,7 +818,11 @@ impl YaccParser { } /// Parse up to (but do not include) the end of line (or, if it comes sooner, the end of file). - fn parse_to_eol(&mut self, i: usize) -> Result<(usize, String), YaccGrammarError> { + fn parse_to_eol( + &mut self, + i: usize, + _: &mut ErrorSender, + ) -> Result<(usize, String), YaccGrammarConstructionFailure> { let mut j = i; while j < self.src.len() { let c = self.src[j..].chars().next().unwrap(); @@ -809,7 +836,11 @@ impl YaccParser { /// Parse up to (but do not include) a single colon (double colons are allowed so that strings /// like `a::b::c:` treat `a::b::c` as a single name. Errors if EOL encountered. - fn parse_to_single_colon(&mut self, i: usize) -> Result<(usize, String), YaccGrammarError> { + fn parse_to_single_colon( + &mut self, + i: usize, + errs: &mut ErrorSender, + ) -> Result<(usize, String), YaccGrammarConstructionFailure> { let mut j = i; while j < self.src.len() { let c = self.src[j..].chars().next().unwrap(); @@ -828,14 +859,16 @@ impl YaccParser { _ => j += c.len_utf8(), } } - Err(self.mk_error(YaccGrammarErrorKind::ReachedEOL, j)) + errs.send(self.mk_error(YaccGrammarErrorKind::ReachedEOL, j))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) } /// Parse a quoted string, allowing escape characters. fn parse_int( &mut self, i: usize, - ) -> Result<(usize, T), YaccGrammarError> { + errs: &mut ErrorSender, + ) -> Result<(usize, T), YaccGrammarConstructionFailure> { let mut j = i; while j < self.src.len() { let c = self.src[j..].chars().next().unwrap(); @@ -846,18 +879,26 @@ impl YaccParser { } match self.src[i..j].parse::() { Ok(x) => Ok((j, x)), - Err(_) => Err(self.mk_error(YaccGrammarErrorKind::IllegalInteger, i)), + Err(_) => { + errs.send(self.mk_error(YaccGrammarErrorKind::IllegalInteger, i))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) + } } } /// Parse a quoted string, allowing escape characters. - fn parse_string(&mut self, mut i: usize) -> Result<(usize, String), YaccGrammarError> { + fn parse_string( + &mut self, + mut i: usize, + errs: &mut ErrorSender, + ) -> Result<(usize, String), YaccGrammarConstructionFailure> { let qc = if self.lookahead_is("'", i).is_some() { '\'' } else if self.lookahead_is("\"", i).is_some() { '"' } else { - return Err(self.mk_error(YaccGrammarErrorKind::InvalidString, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); }; debug_assert!('"'.len_utf8() == 1 && '\''.len_utf8() == 1); @@ -872,7 +913,8 @@ impl YaccParser { let c = self.src[j..].chars().next().unwrap(); match c { '\n' | '\r' => { - return Err(self.mk_error(YaccGrammarErrorKind::InvalidString, j)); + errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } x if x == qc => { s.push_str(&self.src[i..j]); @@ -887,26 +929,34 @@ impl YaccParser { j += 2; } _ => { - return Err(self.mk_error(YaccGrammarErrorKind::InvalidString, j)); + errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } } } _ => j += c.len_utf8(), } } - Err(self.mk_error(YaccGrammarErrorKind::InvalidString, j)) + errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?; + Err(YaccGrammarConstructionFailure::ConstructionFailure) } /// Skip whitespace from `i` onwards. If `inc_newlines` is `false`, will return `Err` if a /// newline is encountered; otherwise newlines are consumed and skipped. - fn parse_ws(&mut self, mut i: usize, inc_newlines: bool) -> Result { + fn parse_ws( + &mut self, + mut i: usize, + inc_newlines: bool, + errs: &mut ErrorSender, + ) -> Result { while i < self.src.len() { let c = self.src[i..].chars().next().unwrap(); match c { ' ' | '\t' => i += c.len_utf8(), '\n' | '\r' => { if !inc_newlines { - return Err(self.mk_error(YaccGrammarErrorKind::ReachedEOL, i)); + errs.send(self.mk_error(YaccGrammarErrorKind::ReachedEOL, i))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } self.num_newlines += 1; i += c.len_utf8(); @@ -939,10 +989,11 @@ impl YaccParser { match c { '\n' | '\r' => { if !inc_newlines { - return Err(self.mk_error( + errs.send(self.mk_error( YaccGrammarErrorKind::ReachedEOL, i, - )); + ))?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); } self.num_newlines += 1; } @@ -959,8 +1010,11 @@ impl YaccParser { } } if !found { + errs.send( + self.mk_error(YaccGrammarErrorKind::IncompleteComment, i), + )?; return Err( - self.mk_error(YaccGrammarErrorKind::IncompleteComment, i) + YaccGrammarConstructionFailure::ConstructionFailure, ); } } @@ -2343,7 +2397,7 @@ x" } #[test] - fn test_only_one_type() { + fn test_duplicate_actiontype() { let src = " %actiontype T1 %actiontype T2 From a9b432a0975e107d713da5a27d91f18baa13ade6 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 23 Dec 2023 00:34:27 -0800 Subject: [PATCH 2/7] handle parse_token specially --- cfgrammar/src/lib/yacc/parser.rs | 69 +++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 18 deletions(-) diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 68ad58d41..53bcaca82 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -353,7 +353,12 @@ impl YaccParser { if let Some(j) = self.lookahead_is("%token", i) { i = self.parse_ws(j, false, errs)?; while i < self.src.len() && self.lookahead_is("%", i).is_none() { - let (j, n, span) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, n, span) = result.unwrap(); if self.ast.tokens.insert(n) { self.ast.spans.push(span); } @@ -400,7 +405,12 @@ impl YaccParser { } if let Some(j) = self.lookahead_is("%epp", i) { i = self.parse_ws(j, false, errs)?; - let (j, n, _) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, n, _) = result.unwrap(); let span = Span::new(i, j); i = self.parse_ws(j, false, errs)?; let (j, v) = self.parse_string(i, errs)?; @@ -446,7 +456,7 @@ impl YaccParser { .push(Symbol::Rule(n, Span::new(i, j))); j } - Err(_) => match self.parse_token(i, errs) { + Err(_) => match self.parse_token(i) { Ok((j, n, span)) => { self.ast.expect_unused.push(Symbol::Token(n, span)); j @@ -483,7 +493,12 @@ impl YaccParser { self.ast.avoid_insert = Some(HashMap::new()); } while j < self.src.len() && self.num_newlines == num_newlines { - let (j, n, span) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, n, span) = result.unwrap(); if self.ast.tokens.insert(n.clone()) { self.ast.spans.push(span); } @@ -526,7 +541,12 @@ impl YaccParser { self.ast.implicit_tokens = Some(HashMap::new()); } while j < self.src.len() && self.num_newlines == num_newlines { - let (j, n, span) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, n, span) = result.unwrap(); if self.ast.tokens.insert(n.clone()) { self.ast.spans.push(span); } @@ -567,7 +587,12 @@ impl YaccParser { i = self.parse_ws(k, false, errs)?; let num_newlines = self.num_newlines; while i < self.src.len() && num_newlines == self.num_newlines { - let (j, n, span) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, n, span) = result.unwrap(); match self.ast.precs.entry(n) { Entry::Occupied(orig) => { let (_, orig_span) = orig.get(); @@ -677,7 +702,12 @@ impl YaccParser { } if self.lookahead_is("\"", i).is_some() || self.lookahead_is("'", i).is_some() { - let (j, sym, span) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, sym, span) = result.unwrap(); i = self.parse_ws(j, true, errs)?; if self.ast.tokens.insert(sym.clone()) { self.ast.spans.push(span); @@ -685,7 +715,12 @@ impl YaccParser { syms.push(Symbol::Token(sym, span)); } else if let Some(j) = self.lookahead_is("%prec", i) { i = self.parse_ws(j, true, errs)?; - let (k, sym, _) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (k, sym, _) = result.unwrap(); if self.ast.tokens.contains(&sym) { prec = Some(sym); } else { @@ -712,7 +747,12 @@ impl YaccParser { } i = j; } else { - let (j, sym, span) = self.parse_token(i, errs)?; + let result = self.parse_token(i); + if let Err(e) = result { + errs.send(e)?; + return Err(YaccGrammarConstructionFailure::ConstructionFailure); + } + let (j, sym, span) = result.unwrap(); if self.ast.tokens.contains(&sym) { syms.push(Symbol::Token(sym, span)); } else { @@ -736,11 +776,7 @@ impl YaccParser { } } - fn parse_token( - &self, - i: usize, - errs: &mut ErrorSender, - ) -> Result<(usize, String, Span), YaccGrammarConstructionFailure> { + fn parse_token(&self, i: usize) -> Result<(usize, String, Span), YaccGrammarError> { match RE_TOKEN.find(&self.src[i..]) { Some(m) => { assert!(m.start() == 0 && m.end() > 0); @@ -762,10 +798,7 @@ impl YaccParser { )), } } - None => { - errs.send(self.mk_error(YaccGrammarErrorKind::IllegalString, i))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) - } + None => Err(self.mk_error(YaccGrammarErrorKind::IllegalString, i)), } } From 507b1ad6c59358876f17fff81dd6b2a6ef30a18e Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 23 Dec 2023 16:05:07 -0800 Subject: [PATCH 3/7] Fix DuplicateEPP implementation, and tests now that we don't squash errors. --- cfgrammar/src/lib/yacc/parser.rs | 154 +++++++++++++++++++++---------- 1 file changed, 103 insertions(+), 51 deletions(-) diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 53bcaca82..cc7c635ee 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -422,7 +422,6 @@ impl YaccParser { kind: YaccGrammarErrorKind::DuplicateEPP, spans: vec![*orig_span, span], })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); } Entry::Vacant(epp) => { epp.insert((span, (v, vspan))); @@ -1221,27 +1220,26 @@ mod test { src: &str, expected: &mut dyn Iterator)>, ) { - match self { - Ok(_) => panic!("Parsed ok while expecting error"), - Err(errs) - if errs - .iter() - .map(|e| { - // Check that it is valid to slice the source with the spans. - for span in e.spans() { - let _ = &src[span.start()..span.end()]; - } - ( - e.kind.clone(), - e.spans() - .iter() - .map(|span| line_col!(src, span)) - .collect::>(), - ) - }) - .eq(expected) => {} - Err(errs) => incorrect_errs!(src, errs), - } + let errs = self.expect_err("Parsed ok while expecting error"); + let kinds_lines_cols = &errs + .iter() + .map(|e| { + ( + e.kind.clone(), + e.spans() + .iter() + .map(|span| line_col!(src, span)) + .collect::>(), + ) + }) + .collect::>(); + errs.iter().for_each(|e| { + for span in e.spans() { + let _ = &src[span.start()..span.end()]; + } + }); + let expected = expected.collect::>(); + assert_eq!(kinds_lines_cols, &expected); } } @@ -1774,7 +1772,7 @@ x" } #[test] - fn test_multiple_dup_precs() { + fn test_multiple_duplicate_precs() { let src = " %left 'x' %left 'x' @@ -1794,11 +1792,23 @@ x" &mut [ ( YaccGrammarErrorKind::DuplicatePrecedence, - vec![(2, 18), (3, 18), (4, 19), (5, 22)], + vec![(2, 18), (3, 18)], + ), + ( + YaccGrammarErrorKind::DuplicatePrecedence, + vec![(2, 18), (4, 19)], + ), + ( + YaccGrammarErrorKind::DuplicatePrecedence, + vec![(2, 18), (5, 22)], + ), + ( + YaccGrammarErrorKind::DuplicatePrecedence, + vec![(6, 18), (7, 22)], ), ( YaccGrammarErrorKind::DuplicatePrecedence, - vec![(6, 18), (7, 22), (8, 19)], + vec![(6, 18), (8, 19)], ), ] .into_iter(), @@ -2111,10 +2121,13 @@ x" %epp A \"a\" %% "; - parse(YaccKind::Eco, src).expect_error_at_lines_cols( + parse(YaccKind::Eco, src).expect_multiple_errors( src, - YaccGrammarErrorKind::DuplicateEPP, - &mut [(2, 14), (3, 14), (4, 14)].into_iter(), + &mut [ + (YaccGrammarErrorKind::DuplicateEPP, vec![(2, 14), (3, 14)]), + (YaccGrammarErrorKind::DuplicateEPP, vec![(2, 14), (4, 14)]), + ] + .into_iter(), ); } @@ -2132,14 +2145,10 @@ x" parse(YaccKind::Eco, src).expect_multiple_errors( src, &mut [ - ( - YaccGrammarErrorKind::DuplicateEPP, - vec![(2, 14), (3, 14), (4, 14)], - ), - ( - YaccGrammarErrorKind::DuplicateEPP, - vec![(5, 14), (6, 14), (7, 14)], - ), + (YaccGrammarErrorKind::DuplicateEPP, vec![(2, 14), (3, 14)]), + (YaccGrammarErrorKind::DuplicateEPP, vec![(2, 14), (4, 14)]), + (YaccGrammarErrorKind::DuplicateEPP, vec![(5, 14), (6, 14)]), + (YaccGrammarErrorKind::DuplicateEPP, vec![(5, 14), (7, 14)]), ] .into_iter(), ); @@ -2202,11 +2211,20 @@ x" YaccKind::Original(YaccOriginalActionKind::GenericParseTree), src, ) - .expect_error_at_lines_cols( + .expect_multiple_errors( src, - YaccGrammarErrorKind::DuplicateExpectDeclaration, - &mut [(2, 19), (3, 19), (4, 19)].into_iter(), - ) + &mut [ + ( + YaccGrammarErrorKind::DuplicateExpectDeclaration, + vec![(2, 19), (3, 19)], + ), + ( + YaccGrammarErrorKind::DuplicateExpectDeclaration, + vec![(2, 19), (4, 19)], + ), + ] + .into_iter(), + ); } #[test] @@ -2226,7 +2244,11 @@ x" &mut [ ( YaccGrammarErrorKind::DuplicateExpectDeclaration, - vec![(2, 19), (3, 19), (4, 19)], + vec![(2, 19), (3, 19)], + ), + ( + YaccGrammarErrorKind::DuplicateExpectDeclaration, + vec![(2, 19), (4, 19)], ), (YaccGrammarErrorKind::MissingColon, vec![(6, 13)]), ] @@ -2246,10 +2268,19 @@ x" YaccKind::Original(YaccOriginalActionKind::GenericParseTree), src, ) - .expect_error_at_lines_cols( + .expect_multiple_errors( src, - YaccGrammarErrorKind::DuplicateExpectRRDeclaration, - &mut [(2, 22), (3, 22), (4, 22)].into_iter(), + &mut [ + ( + YaccGrammarErrorKind::DuplicateExpectRRDeclaration, + vec![(2, 22), (3, 22)], + ), + ( + YaccGrammarErrorKind::DuplicateExpectRRDeclaration, + vec![(2, 22), (4, 22)], + ), + ] + .into_iter(), ); } @@ -2270,7 +2301,11 @@ x" &mut [ ( YaccGrammarErrorKind::DuplicateExpectRRDeclaration, - vec![(2, 22), (3, 22), (4, 22)], + vec![(2, 22), (3, 22)], + ), + ( + YaccGrammarErrorKind::DuplicateExpectRRDeclaration, + vec![(2, 22), (4, 22)], ), (YaccGrammarErrorKind::IllegalName, vec![(6, 11)]), ] @@ -2441,11 +2476,20 @@ x" YaccKind::Original(YaccOriginalActionKind::GenericParseTree), src, ) - .expect_error_at_lines_cols( + .expect_multiple_errors( src, - YaccGrammarErrorKind::DuplicateActiontypeDeclaration, - &mut [(2, 22), (3, 22), (4, 22)].into_iter(), - ); + &mut [ + ( + YaccGrammarErrorKind::DuplicateActiontypeDeclaration, + vec![(2, 22), (3, 22)], + ), + ( + YaccGrammarErrorKind::DuplicateActiontypeDeclaration, + vec![(2, 22), (4, 22)], + ), + ] + .into_iter(), + ) } #[test] @@ -2463,7 +2507,11 @@ x" &mut [ ( YaccGrammarErrorKind::DuplicateActiontypeDeclaration, - vec![(2, 22), (3, 22), (4, 22)], + vec![(2, 22), (3, 22)], + ), + ( + YaccGrammarErrorKind::DuplicateActiontypeDeclaration, + vec![(2, 22), (4, 22)], ), (YaccGrammarErrorKind::PrematureEnd, vec![(4, 24)]), ] @@ -2528,7 +2576,11 @@ B"; &mut [ ( YaccGrammarErrorKind::DuplicateStartDeclaration, - vec![(1, 8), (2, 8), (3, 8)], + vec![(1, 8), (2, 8)], + ), + ( + YaccGrammarErrorKind::DuplicateStartDeclaration, + vec![(1, 8), (3, 8)], ), (YaccGrammarErrorKind::MissingRightArrow, vec![(6, 2)]), ] From 9393e7fef02a04b3ddda1c7fe50e004d8be8bca5 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 23 Dec 2023 16:10:33 -0800 Subject: [PATCH 4/7] Remove path from ASTBuilder if there is no means to use it. --- cfgrammar/src/lib/yacc/ast.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 5a9d85c2f..edae82ab3 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -10,7 +10,6 @@ use super::{ YaccGrammarErrorKind, YaccGrammarWarning, YaccGrammarWarningKind, YaccKind, }; use crate::Span; -use std::path; use std::sync::mpsc; /// Contains a `GrammarAST` structure produced from a grammar source file. /// As well as any errors which occurred during the construction of the AST. @@ -24,7 +23,6 @@ pub struct ASTWithValidityInfo { pub struct AstBuilder<'a> { kind: YaccKind, grammar_src: Option<&'a str>, - grammar_path: Option, error_receiver: Option>, error_sender: ErrorSender, } @@ -64,10 +62,6 @@ impl ErrorSender { #[allow(dead_code)] impl<'a> AstBuilder<'a> { - fn path(mut self, path: &path::Path) -> Self { - self.grammar_path = Some(path.to_owned()); - self - } fn yacc_kind(mut self, kind: YaccKind) -> Self { self.kind = kind; self @@ -93,7 +87,6 @@ impl<'a> ASTWithValidityInfo { AstBuilder { kind: YaccKind::Grmtools, grammar_src: None, - grammar_path: None, error_receiver: Some(error_receiver), error_sender: ErrorSender::new(error_sender), } From 1389628a06d216b7d98103de965bfe03a9bfebf6 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 23 Dec 2023 16:11:23 -0800 Subject: [PATCH 5/7] make new_with_error_channel pub(crate) for now --- cfgrammar/src/lib/yacc/ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index edae82ab3..2ad0e19c0 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -92,7 +92,7 @@ impl<'a> ASTWithValidityInfo { } } - pub fn new_with_error_channel( + pub(crate) fn new_with_error_channel( yacc_kind: YaccKind, s: &str, mut error_sender: ErrorSender, From 8bc8050a1116425e8341b5bbfbdd5e6078b35724 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 23 Dec 2023 21:52:59 -0800 Subject: [PATCH 6/7] Simplify code by returning construction failure from error.send --- cfgrammar/src/lib/yacc/ast.rs | 52 ++++++------- cfgrammar/src/lib/yacc/parser.rs | 129 +++++++++++++------------------ 2 files changed, 77 insertions(+), 104 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 2ad0e19c0..3c02205d4 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -47,12 +47,13 @@ impl ErrorSender { pub(crate) fn send( &mut self, e: YaccGrammarError, - ) -> Result<(), mpsc::SendError> { + ) -> Result { self.error_occurred = true; if let Some(sender) = &self.sender { - sender.send(e) + sender.send(e)?; + Ok(YaccGrammarConstructionFailure::ConstructionFailure) } else { - Err(mpsc::SendError(e)) + Err(mpsc::SendError(e).into()) } } pub(crate) fn error_occurred(&self) -> bool { @@ -337,19 +338,17 @@ impl GrammarAST { ) -> Result<(), YaccGrammarConstructionFailure> { match self.start { None => { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::NoStartRule, spans: vec![Span::new(0, 0)], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } Some((ref s, span)) => { if !self.rules.contains_key(s) { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::InvalidStartRule(s.clone()), spans: vec![span], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } } } @@ -358,38 +357,34 @@ impl GrammarAST { let prod = &self.prods[pidx]; if let Some(ref n) = prod.precedence { if !self.tokens.contains(n) { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(n.clone()), spans: vec![Span::new(0, 0)], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } if !self.precs.contains_key(n) { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::NoPrecForToken(n.clone()), spans: vec![Span::new(0, 0)], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } } for sym in &prod.symbols { match *sym { Symbol::Rule(ref name, span) => { if !self.rules.contains_key(name) { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(name.clone()), spans: vec![span], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } } Symbol::Token(ref name, span) => { if !self.tokens.contains(name) { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(name.clone()), spans: vec![span], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } } } @@ -406,31 +401,28 @@ impl GrammarAST { continue; } } - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownEPP(k.clone()), spans: vec![*sp], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } for sym in &self.expect_unused { match sym { Symbol::Rule(sym_name, sym_span) => { if self.get_rule(sym_name).is_none() { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(sym_name.clone()), spans: vec![*sym_span], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } } Symbol::Token(sym_name, sym_span) => { if !self.has_token(sym_name) { - errs.send(YaccGrammarError { + return Err(errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(sym_name.clone()), spans: vec![*sym_span], - })?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + })?); } } } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index cc7c635ee..6a3dde70d 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -355,8 +355,7 @@ impl YaccParser { while i < self.src.len() && self.lookahead_is("%", i).is_none() { let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, n, span) = result.unwrap(); if self.ast.tokens.insert(n) { @@ -372,10 +371,10 @@ impl YaccParser { let (j, n) = self.parse_to_eol(i, errs)?; let span = Span::new(i, j); if let Some((_, orig_span)) = self.global_actiontype { - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateActiontypeDeclaration, spans: vec![orig_span, span], - })? + })?; } else { self.global_actiontype = Some((n, span)); } @@ -387,16 +386,15 @@ impl YaccParser { i = self.parse_ws(j, false, errs)?; let result = self.try_parse_name(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, n) = result.unwrap(); let span = Span::new(i, j); if let Some((_, orig_span)) = self.ast.start { - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateStartDeclaration, spans: vec![orig_span, span], - })? + })?; } else { self.ast.start = Some((n, span)); } @@ -407,8 +405,7 @@ impl YaccParser { i = self.parse_ws(j, false, errs)?; let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, n, _) = result.unwrap(); let span = Span::new(i, j); @@ -418,7 +415,7 @@ impl YaccParser { match self.ast.epp.entry(n) { Entry::Occupied(orig) => { let (orig_span, _) = orig.get(); - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateEPP, spans: vec![*orig_span, span], })?; @@ -435,7 +432,7 @@ impl YaccParser { let (j, n) = self.parse_int(i, errs)?; let span = Span::new(i, j); if let Some((_, orig_span)) = self.ast.expectrr { - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateExpectRRDeclaration, spans: vec![orig_span, span], })?; @@ -461,8 +458,9 @@ impl YaccParser { j } Err(_) => { - errs.send(self.mk_error(YaccGrammarErrorKind::UnknownSymbol, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send( + self.mk_error(YaccGrammarErrorKind::UnknownSymbol, i), + )?); } }, }; @@ -475,7 +473,7 @@ impl YaccParser { let (j, n) = self.parse_int(i, errs)?; let span = Span::new(i, j); if let Some((_, orig_span)) = self.ast.expect { - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateExpectDeclaration, spans: vec![orig_span, span], })?; @@ -494,8 +492,7 @@ impl YaccParser { while j < self.src.len() && self.num_newlines == num_newlines { let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, n, span) = result.unwrap(); if self.ast.tokens.insert(n.clone()) { @@ -504,7 +501,7 @@ impl YaccParser { match self.ast.avoid_insert.as_mut().unwrap().entry(n) { Entry::Occupied(occupied) => { - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration, spans: vec![*occupied.get(), span], })?; @@ -523,8 +520,9 @@ impl YaccParser { match self.lookahead_is(":", j) { Some(j) => i = self.parse_ws(j, false, errs)?, None => { - errs.send(self.mk_error(YaccGrammarErrorKind::MissingColon, j))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err( + errs.send(self.mk_error(YaccGrammarErrorKind::MissingColon, j))? + ); } } let (j, ty) = self.parse_to_eol(i, errs)?; @@ -542,8 +540,7 @@ impl YaccParser { while j < self.src.len() && self.num_newlines == num_newlines { let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, n, span) = result.unwrap(); if self.ast.tokens.insert(n.clone()) { @@ -552,7 +549,7 @@ impl YaccParser { match self.ast.implicit_tokens.as_mut().unwrap().entry(n) { Entry::Occupied(entry) => { let orig_span = *entry.get(); - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicateImplicitTokensDeclaration, spans: vec![orig_span, span], })?; @@ -579,8 +576,9 @@ impl YaccParser { kind = AssocKind::Nonassoc; k = j; } else { - errs.send(self.mk_error(YaccGrammarErrorKind::UnknownDeclaration, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err( + errs.send(self.mk_error(YaccGrammarErrorKind::UnknownDeclaration, i))? + ); } i = self.parse_ws(k, false, errs)?; @@ -588,14 +586,13 @@ impl YaccParser { while i < self.src.len() && num_newlines == self.num_newlines { let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, n, span) = result.unwrap(); match self.ast.precs.entry(n) { Entry::Occupied(orig) => { let (_, orig_span) = orig.get(); - errs.send(YaccGrammarError { + let _ = errs.send(YaccGrammarError { kind: YaccGrammarErrorKind::DuplicatePrecedence, spans: vec![*orig_span, span], })?; @@ -615,8 +612,7 @@ impl YaccParser { } } debug_assert!(i == self.src.len()); - errs.send(self.mk_error(YaccGrammarErrorKind::PrematureEnd, i))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) + Err(errs.send(self.mk_error(YaccGrammarErrorKind::PrematureEnd, i))?) } fn parse_rules( @@ -641,8 +637,7 @@ impl YaccParser { ) -> Result { let result = self.try_parse_name(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, rn) = result.unwrap(); let span = Span::new(i, j); @@ -664,8 +659,9 @@ impl YaccParser { if let Some(j) = self.lookahead_is("->", i) { i = j; } else { - errs.send(self.mk_error(YaccGrammarErrorKind::MissingRightArrow, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err( + errs.send(self.mk_error(YaccGrammarErrorKind::MissingRightArrow, i))? + ); } i = self.parse_ws(i, true, errs)?; let (j, actiont) = self.parse_to_single_colon(i, errs)?; @@ -679,8 +675,7 @@ impl YaccParser { match self.lookahead_is(":", i) { Some(j) => i = j, None => { - errs.send(self.mk_error(YaccGrammarErrorKind::MissingColon, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(self.mk_error(YaccGrammarErrorKind::MissingColon, i))?); } } let mut syms = Vec::new(); @@ -703,8 +698,7 @@ impl YaccParser { if self.lookahead_is("\"", i).is_some() || self.lookahead_is("'", i).is_some() { let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, sym, span) = result.unwrap(); i = self.parse_ws(j, true, errs)?; @@ -716,15 +710,15 @@ impl YaccParser { i = self.parse_ws(j, true, errs)?; let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (k, sym, _) = result.unwrap(); if self.ast.tokens.contains(&sym) { prec = Some(sym); } else { - errs.send(self.mk_error(YaccGrammarErrorKind::PrecNotFollowedByToken, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err( + errs.send(self.mk_error(YaccGrammarErrorKind::PrecNotFollowedByToken, i))? + ); } i = k; } else if self.lookahead_is("{", i).is_some() { @@ -741,15 +735,15 @@ impl YaccParser { || self.lookahead_is(";", j).is_some() || self.lookahead_is("{", j).is_some()) { - errs.send(self.mk_error(YaccGrammarErrorKind::NonEmptyProduction, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err( + errs.send(self.mk_error(YaccGrammarErrorKind::NonEmptyProduction, i))? + ); } i = j; } else { let result = self.parse_token(i); if let Err(e) = result { - errs.send(e)?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(e)?); } let (j, sym, span) = result.unwrap(); if self.ast.tokens.contains(&sym) { @@ -761,8 +755,7 @@ impl YaccParser { } i = self.parse_ws(i, true, errs)?; } - errs.send(self.mk_error(YaccGrammarErrorKind::IncompleteRule, i))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) + Err(errs.send(self.mk_error(YaccGrammarErrorKind::IncompleteRule, i))?) } fn try_parse_name(&self, i: usize) -> Result<(usize, String), YaccGrammarError> { @@ -826,8 +819,7 @@ impl YaccParser { j += ch.len_utf8(); } if c > 0 { - errs.send(self.mk_error(YaccGrammarErrorKind::IncompleteAction, j))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) + Err(errs.send(self.mk_error(YaccGrammarErrorKind::IncompleteAction, j))?) } else { debug_assert!(self.lookahead_is("}", j).is_some()); let s = self.src[i + '{'.len_utf8()..j].trim().to_string(); @@ -891,8 +883,7 @@ impl YaccParser { _ => j += c.len_utf8(), } } - errs.send(self.mk_error(YaccGrammarErrorKind::ReachedEOL, j))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) + Err(errs.send(self.mk_error(YaccGrammarErrorKind::ReachedEOL, j))?) } /// Parse a quoted string, allowing escape characters. @@ -911,10 +902,7 @@ impl YaccParser { } match self.src[i..j].parse::() { Ok(x) => Ok((j, x)), - Err(_) => { - errs.send(self.mk_error(YaccGrammarErrorKind::IllegalInteger, i))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) - } + Err(_) => Err(errs.send(self.mk_error(YaccGrammarErrorKind::IllegalInteger, i))?), } } @@ -929,8 +917,7 @@ impl YaccParser { } else if self.lookahead_is("\"", i).is_some() { '"' } else { - errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, i))?); }; debug_assert!('"'.len_utf8() == 1 && '\''.len_utf8() == 1); @@ -945,8 +932,7 @@ impl YaccParser { let c = self.src[j..].chars().next().unwrap(); match c { '\n' | '\r' => { - errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?); } x if x == qc => { s.push_str(&self.src[i..j]); @@ -961,16 +947,16 @@ impl YaccParser { j += 2; } _ => { - errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err( + errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))? + ); } } } _ => j += c.len_utf8(), } } - errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?; - Err(YaccGrammarConstructionFailure::ConstructionFailure) + Err(errs.send(self.mk_error(YaccGrammarErrorKind::InvalidString, j))?) } /// Skip whitespace from `i` onwards. If `inc_newlines` is `false`, will return `Err` if a @@ -987,8 +973,7 @@ impl YaccParser { ' ' | '\t' => i += c.len_utf8(), '\n' | '\r' => { if !inc_newlines { - errs.send(self.mk_error(YaccGrammarErrorKind::ReachedEOL, i))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + return Err(errs.send(self.mk_error(YaccGrammarErrorKind::ReachedEOL, i))?); } self.num_newlines += 1; i += c.len_utf8(); @@ -1021,11 +1006,10 @@ impl YaccParser { match c { '\n' | '\r' => { if !inc_newlines { - errs.send(self.mk_error( + return Err(errs.send(self.mk_error( YaccGrammarErrorKind::ReachedEOL, i, - ))?; - return Err(YaccGrammarConstructionFailure::ConstructionFailure); + ))?); } self.num_newlines += 1; } @@ -1042,12 +1026,9 @@ impl YaccParser { } } if !found { - errs.send( + return Err(errs.send( self.mk_error(YaccGrammarErrorKind::IncompleteComment, i), - )?; - return Err( - YaccGrammarConstructionFailure::ConstructionFailure, - ); + )?); } } _ => break, From d5c73ea5d7e5cf4721832a690da61e5bdf20c370 Mon Sep 17 00:00:00 2001 From: matt rice Date: Sat, 23 Dec 2023 22:17:37 -0800 Subject: [PATCH 7/7] Fix swapped error strings, add error for closed channels --- cfgrammar/src/lib/yacc/ast.rs | 2 +- cfgrammar/src/lib/yacc/parser.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index 3c02205d4..fa5d09499 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -53,7 +53,7 @@ impl ErrorSender { sender.send(e)?; Ok(YaccGrammarConstructionFailure::ConstructionFailure) } else { - Err(mpsc::SendError(e).into()) + Err(YaccGrammarConstructionFailure::ErrorChannelClosed(e)) } } pub(crate) fn error_occurred(&self) -> bool { diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 6a3dde70d..caaab7a23 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -25,6 +25,7 @@ use super::{ #[derive(Debug)] pub enum YaccGrammarConstructionFailure { ConstructionFailure, + ErrorChannelClosed(YaccGrammarError), ErrorChannel(mpsc::SendError), } @@ -37,8 +38,9 @@ impl From> for YaccGrammarConstructionFailure impl fmt::Display for YaccGrammarConstructionFailure { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Self::ConstructionFailure => write!(f, "Failure to send an error over a channel."), - Self::ErrorChannel(e) => write!(f, "Error constructing YaccGrammar: {}", e), + Self::ErrorChannelClosed(e) => write!(f, "Attempt to send error {} over closed channel", e), + Self::ConstructionFailure => write!(f, "Error constructing YaccGrammar: "), + Self::ErrorChannel(e) => write!(f, "Failure to send an error {} over a channel.", e), } } }