From 23ed74bc94ec4da8dbd35da0ae39b26c7ef601e5 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 17 Sep 2024 14:07:19 -0500 Subject: [PATCH] fix: Always parse all tokens from quoted token streams (#6064) # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/5803 Resolves https://github.com/noir-lang/noir/issues/6059 ## Summary\* If we ever had a token stream with a subset of valid input we'd parse just that subset instead of erroring that the full input doesn't parse. ## Additional Context We don't have EOF tokens in token streams so I used the `end()` primitive instead. ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/hir/comptime/interpreter/builtin.rs | 37 ++++++------------- .../interpreter/builtin/builtin_helpers.rs | 11 ++++++ .../noirc_frontend/src/hir/comptime/value.rs | 1 + .../comptime_parse_all_tokens/Nargo.toml | 7 ++++ .../comptime_parse_all_tokens/src/main.nr | 9 +++++ 5 files changed, 39 insertions(+), 26 deletions(-) create mode 100644 test_programs/compile_failure/comptime_parse_all_tokens/Nargo.toml create mode 100644 test_programs/compile_failure/comptime_parse_all_tokens/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 7298514b53c..9833cf27565 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -10,7 +10,7 @@ use builtin_helpers::{ mutate_func_meta_type, parse, quote_ident, replace_func_meta_parameters, replace_func_meta_return_type, }; -use chumsky::{chain::Chain, prelude::choice, Parser}; +use chumsky::{chain::Chain, prelude::choice, primitive::just, Parser}; use im::Vector; use iter_extended::{try_vecmap, vecmap}; use noirc_errors::Location; @@ -33,15 +33,14 @@ use crate::{ def_map::ModuleId, }, hir_def::function::FunctionBody, - lexer::Lexer, macros_api::{HirExpression, HirLiteral, Ident, ModuleDefId, NodeInterner, Signedness}, node_interner::{DefinitionKind, TraitImplKind}, - parser::{self}, + parser, token::{Attribute, SecondaryAttribute, Token}, Kind, QuotedType, ResolvedGeneric, Shared, Type, TypeVariable, }; -use self::builtin_helpers::{eq_item, get_array, get_ctstring, get_str, get_u8, hash_item}; +use self::builtin_helpers::{eq_item, get_array, get_ctstring, get_str, get_u8, hash_item, lex}; use super::Interpreter; pub(crate) mod builtin_helpers; @@ -330,10 +329,7 @@ fn struct_def_add_attribute( let attribute_location = attribute.1; let attribute = get_str(interner, attribute)?; - let mut tokens = Lexer::lex(&format!("#[{}]", attribute)).0 .0; - if let Some(Token::EOF) = tokens.last().map(|token| token.token()) { - tokens.pop(); - } + let mut tokens = lex(&format!("#[{}]", attribute)); if tokens.len() != 1 { return Err(InterpreterError::InvalidAttribute { attribute: attribute.to_string(), @@ -341,7 +337,7 @@ fn struct_def_add_attribute( }); } - let token = tokens.into_iter().next().unwrap().into_token(); + let token = tokens.remove(0); let Token::Attribute(attribute) = token else { return Err(InterpreterError::InvalidAttribute { attribute: attribute.to_string(), @@ -374,11 +370,7 @@ fn struct_def_add_generic( let generic_location = generic.1; let generic = get_str(interner, generic)?; - let mut tokens = Lexer::lex(&generic).0 .0; - if let Some(Token::EOF) = tokens.last().map(|token| token.token()) { - tokens.pop(); - } - + let mut tokens = lex(&generic); if tokens.len() != 1 { return Err(InterpreterError::GenericNameShouldBeAnIdent { name: generic, @@ -386,7 +378,7 @@ fn struct_def_add_generic( }); } - let Token::Ident(generic_name) = tokens.pop().unwrap().into_token() else { + let Token::Ident(generic_name) = tokens.remove(0) else { return Err(InterpreterError::GenericNameShouldBeAnIdent { name: generic, location: generic_location, @@ -690,6 +682,7 @@ fn quoted_as_expr( let statement_parser = parser::fresh_statement().map(Value::statement); let lvalue_parser = parser::lvalue(parser::expression()).map(Value::lvalue); let parser = choice((expr_parser, statement_parser, lvalue_parser)); + let parser = parser.then_ignore(just(Token::Semicolon).or_not()); let expr = parse(argument, parser, "an expression").ok(); @@ -2059,12 +2052,7 @@ fn fmtstr_quoted_contents( ) -> IResult { let self_argument = check_one_argument(arguments, location)?; let (string, _) = get_format_string(interner, self_argument)?; - let (tokens, _) = Lexer::lex(&string); - let mut tokens: Vec<_> = tokens.0.into_iter().map(|token| token.into_token()).collect(); - if let Some(Token::EOF) = tokens.last() { - tokens.pop(); - } - + let tokens = lex(&string); Ok(Value::Quoted(Rc::new(tokens))) } @@ -2083,10 +2071,7 @@ fn function_def_add_attribute( let attribute_location = attribute.1; let attribute = get_str(interpreter.elaborator.interner, attribute)?; - let mut tokens = Lexer::lex(&format!("#[{}]", attribute)).0 .0; - if let Some(Token::EOF) = tokens.last().map(|token| token.token()) { - tokens.pop(); - } + let mut tokens = lex(&format!("#[{}]", attribute)); if tokens.len() != 1 { return Err(InterpreterError::InvalidAttribute { attribute: attribute.to_string(), @@ -2094,7 +2079,7 @@ fn function_def_add_attribute( }); } - let token = tokens.into_iter().next().unwrap().into_token(); + let token = tokens.remove(0); let Token::Attribute(attribute) = token else { return Err(InterpreterError::InvalidAttribute { attribute: attribute.to_string(), diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index db42d6c4170..5fe5447da6a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -4,6 +4,7 @@ use std::{hash::Hasher, rc::Rc}; use acvm::FieldElement; use noirc_errors::Location; +use crate::lexer::Lexer; use crate::{ ast::{ BlockExpression, ExpressionKind, Ident, IntegerBitSize, LValue, Pattern, Signedness, @@ -392,11 +393,21 @@ pub(super) fn check_function_not_yet_resolved( } } +pub(super) fn lex(input: &str) -> Vec { + let (tokens, _) = Lexer::lex(input); + let mut tokens: Vec<_> = tokens.0.into_iter().map(|token| token.into_token()).collect(); + if let Some(Token::EOF) = tokens.last() { + tokens.pop(); + } + tokens +} + pub(super) fn parse( (value, location): (Value, Location), parser: impl NoirParser, rule: &'static str, ) -> IResult { + let parser = parser.then_ignore(chumsky::primitive::end()); let tokens = get_quoted((value, location))?; let quoted = add_token_spans(tokens.clone(), location.span); parse_tokens(tokens, quoted, location, parser, rule) diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 34be0e9f49e..31971fc63b7 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -554,6 +554,7 @@ fn parse_tokens( parser: impl NoirParser, location: Location, ) -> IResult { + let parser = parser.then_ignore(chumsky::primitive::end()); match parser.parse(add_token_spans(tokens.clone(), location.span)) { Ok(expr) => Ok(expr), Err(mut errors) => { diff --git a/test_programs/compile_failure/comptime_parse_all_tokens/Nargo.toml b/test_programs/compile_failure/comptime_parse_all_tokens/Nargo.toml new file mode 100644 index 00000000000..2d9b78c9c38 --- /dev/null +++ b/test_programs/compile_failure/comptime_parse_all_tokens/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_parse_all_tokens" +type = "bin" +authors = [""] +compiler_version = ">=0.34.0" + +[dependencies] diff --git a/test_programs/compile_failure/comptime_parse_all_tokens/src/main.nr b/test_programs/compile_failure/comptime_parse_all_tokens/src/main.nr new file mode 100644 index 00000000000..8e9a607f44a --- /dev/null +++ b/test_programs/compile_failure/comptime_parse_all_tokens/src/main.nr @@ -0,0 +1,9 @@ +#[foo] +fn main() {} + +comptime fn foo(_f: FunctionDefinition) -> Quoted { + let t = quote { Field }.as_type(); + // We parse 0 or more top level expressions from attribute output + // so for invalid input we previously "successfully" parsed 0 items. + quote { use $t; } +}