From c2723e1bb021f69fa062d928af7b12e74715baaf Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 24 Jan 2024 16:53:32 -0500 Subject: [PATCH 1/5] distinguish parser from lexer unexpected token, fix panic on empty module, replace map(|_| ..) with to(..), remove redundant or_not on zero-or-more attributes() parser --- compiler/noirc_frontend/src/lexer/errors.rs | 2 +- compiler/noirc_frontend/src/parser/errors.rs | 2 +- compiler/noirc_frontend/src/parser/parser.rs | 19 ++++++++++--------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/lexer/errors.rs b/compiler/noirc_frontend/src/lexer/errors.rs index a2a4056f1d0..ee08dcf1a9d 100644 --- a/compiler/noirc_frontend/src/lexer/errors.rs +++ b/compiler/noirc_frontend/src/lexer/errors.rs @@ -64,7 +64,7 @@ impl LexerErrorKind { ( "An unexpected character was found".to_string(), - format!("Expected {expected}, but found {found}"), + format!("Lexer: expected {expected}, but found {found}"), *span, ) }, diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index 5c869ff4719..94605749999 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -117,7 +117,7 @@ impl std::fmt::Display for ParserError { let vowel = "aeiou".contains(first.chars().next().unwrap()); write!( f, - "Expected a{} {} but found {}", + "Parser: expected a{} {} but found {}", if vowel { "n" } else { "" }, first, self.found diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index cdfdc570949..cee5188ddfd 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -61,12 +61,12 @@ pub fn parse_program(source_program: &str) -> (ParsedModule, Vec) { parsing_errors.extend(lexing_errors.into_iter().map(Into::into)); - (module.unwrap(), parsing_errors) + (module.unwrap_or(ParsedModule { items: vec![] }), parsing_errors) } /// program: module EOF fn program() -> impl NoirParser { - module().then_ignore(force(just(Token::EOF))) + module().then_ignore(just(Token::EOF)) } /// module: top_level_statement module @@ -74,7 +74,7 @@ fn program() -> impl NoirParser { fn module() -> impl NoirParser { recursive(|module_parser| { empty() - .map(|_| ParsedModule::default()) + .to(ParsedModule::default()) .then(spanned(top_level_statement(module_parser)).repeated()) .foldl(|mut program, (statement, span)| { let mut push_item = |kind| program.items.push(Item { kind, span }); @@ -164,7 +164,7 @@ fn contract(module_parser: impl NoirParser) -> impl NoirParser impl NoirParser { attributes() - .or_not() + .map(Option::Some) .then(function_modifiers()) .then_ignore(keyword(Keyword::Fn)) .then(ident()) @@ -224,6 +224,7 @@ fn function_modifiers() -> impl NoirParser<(bool, bool, bool, bool, bool)> { ) }) } + fn is_pub_crate() -> impl NoirParser { (keyword(Keyword::Pub) .then_ignore(just(Token::LeftParen)) @@ -260,10 +261,10 @@ fn struct_definition() -> impl NoirParser { [(LeftParen, RightParen), (LeftBracket, RightBracket)], |_| vec![], )) - .or(just(Semicolon).map(|_| Vec::new())); + .or(just(Semicolon).to(Vec::new())); attributes() - .or_not() + .map(Option::Some) .then_ignore(keyword(Struct)) .then(ident()) .then(generics()) @@ -448,7 +449,7 @@ fn trait_constant_declaration() -> impl NoirParser { /// trait_function_declaration: 'fn' ident generics '(' declaration_parameters ')' function_return_type fn trait_function_declaration() -> impl NoirParser { let trait_function_body_or_semicolon = - block(fresh_statement()).map(Option::from).or(just(Token::Semicolon).map(|_| Option::None)); + block(fresh_statement()).map(Option::from).or(just(Token::Semicolon).to(Option::None)); keyword(Keyword::Fn) .ignore_then(ident()) @@ -976,7 +977,7 @@ fn assign_operator() -> impl NoirParser { // Since >> is lexed as two separate "greater-than"s, >>= is lexed as > >=, so // we need to account for that case here as well. let right_shift_fix = - just(Token::Greater).then(just(Token::GreaterEqual)).map(|_| Token::ShiftRight); + just(Token::Greater).then(just(Token::GreaterEqual)).to(Token::ShiftRight); let shorthand_syntax = shorthand_syntax.or(right_shift_fix); just(Token::Assign).or(shorthand_syntax) @@ -1337,7 +1338,7 @@ fn create_infix_expression(lhs: Expression, (operator, rhs): (BinaryOp, Expressi // to parse nested generic types. For normal expressions however, it means we have to manually // parse two greater-than tokens as a single right-shift here. fn right_shift_operator() -> impl NoirParser { - just(Token::Greater).then(just(Token::Greater)).map(|_| Token::ShiftRight) + just(Token::Greater).then(just(Token::Greater)).to(Token::ShiftRight) } fn operator_with_precedence(precedence: Precedence) -> impl NoirParser> { From cd7c1e78f9e7f43243453c311c8c341ab38c9fe8 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 25 Jan 2024 12:24:40 -0500 Subject: [PATCH 2/5] remove unused Option --- compiler/noirc_frontend/src/parser/parser.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index cee5188ddfd..b0ecc20d01c 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -164,7 +164,6 @@ fn contract(module_parser: impl NoirParser) -> impl NoirParser impl NoirParser { attributes() - .map(Option::Some) .then(function_modifiers()) .then_ignore(keyword(Keyword::Fn)) .then(ident()) @@ -264,7 +263,6 @@ fn struct_definition() -> impl NoirParser { .or(just(Semicolon).to(Vec::new())); attributes() - .map(Option::Some) .then_ignore(keyword(Struct)) .then(ident()) .then(generics()) @@ -464,20 +462,14 @@ fn trait_function_declaration() -> impl NoirParser { } fn validate_attributes( - attributes: Option>, + attributes: Vec, span: Span, emit: &mut dyn FnMut(ParserError), ) -> Attributes { - if attributes.is_none() { - return Attributes::empty(); - } - - let attrs = attributes.unwrap(); - let mut primary = None; let mut secondary = Vec::new(); - for attribute in attrs { + for attribute in attributes { match attribute { Attribute::Function(attr) => { if primary.is_some() { @@ -496,14 +488,13 @@ fn validate_attributes( } fn validate_struct_attributes( - attributes: Option>, + attributes: Vec, span: Span, emit: &mut dyn FnMut(ParserError), ) -> Vec { - let attrs = attributes.unwrap_or_default(); let mut struct_attributes = vec![]; - for attribute in attrs { + for attribute in attributes { match attribute { Attribute::Function(..) => { emit(ParserError::with_reason( From d12ba1f3c8d569517881a18baf6a0df58cdbe457 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 25 Jan 2024 12:27:57 -0500 Subject: [PATCH 3/5] remove 'parse/lexer prefix from 'expected' errors --- compiler/noirc_frontend/src/lexer/errors.rs | 2 +- compiler/noirc_frontend/src/parser/errors.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/lexer/errors.rs b/compiler/noirc_frontend/src/lexer/errors.rs index ee08dcf1a9d..a2a4056f1d0 100644 --- a/compiler/noirc_frontend/src/lexer/errors.rs +++ b/compiler/noirc_frontend/src/lexer/errors.rs @@ -64,7 +64,7 @@ impl LexerErrorKind { ( "An unexpected character was found".to_string(), - format!("Lexer: expected {expected}, but found {found}"), + format!("Expected {expected}, but found {found}"), *span, ) }, diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index 94605749999..57313ca828e 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -117,7 +117,7 @@ impl std::fmt::Display for ParserError { let vowel = "aeiou".contains(first.chars().next().unwrap()); write!( f, - "Parser: expected a{} {} but found {}", + "Expected a{} {} but found {}.", if vowel { "n" } else { "" }, first, self.found From bfba50077865e4841514c1a2110324104712e0b6 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Thu, 25 Jan 2024 15:49:49 -0500 Subject: [PATCH 4/5] Update compiler/noirc_frontend/src/parser/errors.rs Co-authored-by: jfecher --- compiler/noirc_frontend/src/parser/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index 57313ca828e..5c869ff4719 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -117,7 +117,7 @@ impl std::fmt::Display for ParserError { let vowel = "aeiou".contains(first.chars().next().unwrap()); write!( f, - "Expected a{} {} but found {}.", + "Expected a{} {} but found {}", if vowel { "n" } else { "" }, first, self.found From da2d7f8b3492d8c0f0b22f032d1acf468986b321 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 25 Jan 2024 15:55:41 -0500 Subject: [PATCH 5/5] cargo fmt --- compiler/noirc_frontend/src/parser/parser.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index b0ecc20d01c..f82ce95c718 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -262,15 +262,12 @@ fn struct_definition() -> impl NoirParser { )) .or(just(Semicolon).to(Vec::new())); - attributes() - .then_ignore(keyword(Struct)) - .then(ident()) - .then(generics()) - .then(fields) - .validate(|(((raw_attributes, name), generics), fields), span, emit| { + attributes().then_ignore(keyword(Struct)).then(ident()).then(generics()).then(fields).validate( + |(((raw_attributes, name), generics), fields), span, emit| { let attributes = validate_struct_attributes(raw_attributes, span, emit); TopLevelStatement::Struct(NoirStruct { name, attributes, generics, fields, span }) - }) + }, + ) } fn type_alias_definition() -> impl NoirParser {