From 648a9a85327de81bb5a5a6906d2baa4f4f637b2d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 24 Sep 2024 13:31:28 -0300 Subject: [PATCH 1/5] Introduce the `#[allow(...)]` attribute --- compiler/noirc_frontend/src/lexer/token.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index f7e0a85c79..b4f82a6818 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -804,6 +804,7 @@ impl Attribute { } ["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs), ["use_callers_scope"] => Attribute::Secondary(SecondaryAttribute::UseCallersScope), + ["allow", tag] => Attribute::Secondary(SecondaryAttribute::Allow(tag.to_string())), tokens => { tokens.iter().try_for_each(|token| validate(token))?; Attribute::Secondary(SecondaryAttribute::Custom(CustomAttribute { @@ -925,6 +926,9 @@ pub enum SecondaryAttribute { /// within the scope of the calling function/module rather than this one. /// This affects functions such as `Expression::resolve` or `Quoted::as_type`. UseCallersScope, + + /// Allow chosen warnings to happen so they are silenced. + Allow(String), } impl SecondaryAttribute { @@ -948,6 +952,7 @@ impl SecondaryAttribute { SecondaryAttribute::Abi(_) => Some("abi".to_string()), SecondaryAttribute::Varargs => Some("varargs".to_string()), SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()), + SecondaryAttribute::Allow(_) => Some("allow".to_string()), } } } @@ -966,6 +971,7 @@ impl fmt::Display for SecondaryAttribute { SecondaryAttribute::Abi(ref k) => write!(f, "#[abi({k})]"), SecondaryAttribute::Varargs => write!(f, "#[varargs]"), SecondaryAttribute::UseCallersScope => write!(f, "#[use_callers_scope]"), + SecondaryAttribute::Allow(ref k) => write!(f, "#[allow(#{k})]"), } } } @@ -1011,7 +1017,9 @@ impl AsRef for SecondaryAttribute { SecondaryAttribute::Deprecated(Some(string)) => string, SecondaryAttribute::Deprecated(None) => "", SecondaryAttribute::Custom(attribute) => &attribute.contents, - SecondaryAttribute::Field(string) | SecondaryAttribute::Abi(string) => string, + SecondaryAttribute::Field(string) + | SecondaryAttribute::Abi(string) + | SecondaryAttribute::Allow(string) => string, SecondaryAttribute::ContractLibraryMethod => "", SecondaryAttribute::Export => "", SecondaryAttribute::Varargs => "", From b023febeb4fe7f207c3656935a1a2b1095bd58eb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 24 Sep 2024 14:04:06 -0300 Subject: [PATCH 2/5] Allow silencing "unused variable" warning for a `let` statement --- compiler/noirc_frontend/src/ast/statement.rs | 5 +++- compiler/noirc_frontend/src/debug/mod.rs | 3 ++ .../src/elaborator/expressions.rs | 2 +- compiler/noirc_frontend/src/elaborator/mod.rs | 4 +-- .../noirc_frontend/src/elaborator/patterns.rs | 30 +++++++++++-------- .../src/elaborator/statements.rs | 6 +++- .../src/hir/comptime/hir_to_display_ast.rs | 2 +- .../src/hir/comptime/interpreter/builtin.rs | 1 + compiler/noirc_frontend/src/lexer/token.rs | 7 +++++ compiler/noirc_frontend/src/parser/parser.rs | 21 ++++++++----- .../src/parser/parser/traits.rs | 2 +- .../noirc_frontend/src/tests/unused_items.rs | 16 +++++++++- 12 files changed, 72 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 299c42b85c..38cf2cb1d8 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -141,13 +141,14 @@ impl StatementKind { pattern: Pattern, r#type: UnresolvedType, expression: Expression, + attributes: Vec, ) -> StatementKind { StatementKind::Let(LetStatement { pattern, r#type, expression, comptime: false, - attributes: vec![], + attributes, }) } @@ -814,6 +815,7 @@ impl ForRange { Pattern::Identifier(array_ident.clone()), UnresolvedTypeData::Unspecified.with_span(Default::default()), array, + vec![], ), span: array_span, }; @@ -858,6 +860,7 @@ impl ForRange { Pattern::Identifier(identifier), UnresolvedTypeData::Unspecified.with_span(Default::default()), Expression::new(loop_element, array_span), + vec![], ), span: array_span, }; diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index ed9265536f..66de265f86 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -146,6 +146,7 @@ impl DebugInstrumenter { ast::Pattern::Identifier(ident("__debug_expr", ret_expr.span)), ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), ret_expr.clone(), + vec![], ), span: ret_expr.span, }; @@ -249,6 +250,7 @@ impl DebugInstrumenter { }), span: let_stmt.expression.span, }, + vec![], ), span: *span, } @@ -274,6 +276,7 @@ impl DebugInstrumenter { ast::Pattern::Identifier(ident("__debug_expr", assign_stmt.expression.span)), ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), assign_stmt.expression.clone(), + vec![], ); let expression_span = assign_stmt.expression.span; let new_assign_stmt = match &assign_stmt.lvalue { diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index a3b71f3e21..9a74e77215 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -790,7 +790,7 @@ impl<'context> Elaborator<'context> { let parameter = DefinitionKind::Local(None); let typ = self.resolve_inferred_type(typ); arg_types.push(typ.clone()); - (self.elaborate_pattern(pattern, typ.clone(), parameter), typ) + (self.elaborate_pattern(pattern, typ.clone(), parameter, true), typ) }); let return_type = self.resolve_inferred_type(lambda.return_type); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index f9016b1ca6..31592b0225 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -362,8 +362,7 @@ impl<'context> Elaborator<'context> { if let Kind::Numeric(typ) = &generic.kind { let definition = DefinitionKind::GenericType(generic.type_var.clone()); let ident = Ident::new(generic.name.to_string(), generic.span); - let hir_ident = - self.add_variable_decl_inner(ident, false, false, false, definition); + let hir_ident = self.add_variable_decl(ident, false, false, false, definition); self.interner.push_definition_type(hir_ident.id, *typ.clone()); } } @@ -760,6 +759,7 @@ impl<'context> Elaborator<'context> { typ.clone(), DefinitionKind::Local(None), &mut parameter_idents, + true, None, ); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 7afa321556..46bf861867 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -26,6 +26,7 @@ impl<'context> Elaborator<'context> { pattern: Pattern, expected_type: Type, definition_kind: DefinitionKind, + warn_if_unused: bool, ) -> HirPattern { self.elaborate_pattern_mut( pattern, @@ -33,6 +34,7 @@ impl<'context> Elaborator<'context> { definition_kind, None, &mut Vec::new(), + warn_if_unused, None, ) } @@ -45,6 +47,7 @@ impl<'context> Elaborator<'context> { expected_type: Type, definition_kind: DefinitionKind, created_ids: &mut Vec, + warn_if_unused: bool, global_id: Option, ) -> HirPattern { self.elaborate_pattern_mut( @@ -53,10 +56,12 @@ impl<'context> Elaborator<'context> { definition_kind, None, created_ids, + warn_if_unused, global_id, ) } + #[allow(clippy::too_many_arguments)] fn elaborate_pattern_mut( &mut self, pattern: Pattern, @@ -64,6 +69,7 @@ impl<'context> Elaborator<'context> { definition: DefinitionKind, mutable: Option, new_definitions: &mut Vec, + warn_if_unused: bool, global_id: Option, ) -> HirPattern { match pattern { @@ -80,7 +86,13 @@ impl<'context> Elaborator<'context> { let location = Location::new(name.span(), self.file); HirIdent::non_trait_method(id, location) } else { - self.add_variable_decl(name, mutable.is_some(), true, definition) + self.add_variable_decl( + name, + mutable.is_some(), + true, + warn_if_unused, + definition, + ) }; self.interner.push_definition_type(ident.id, expected_type); new_definitions.push(ident.clone()); @@ -97,6 +109,7 @@ impl<'context> Elaborator<'context> { definition, Some(span), new_definitions, + warn_if_unused, global_id, ); let location = Location::new(span, self.file); @@ -128,6 +141,7 @@ impl<'context> Elaborator<'context> { definition.clone(), mutable, new_definitions, + warn_if_unused, global_id, ) }); @@ -151,6 +165,7 @@ impl<'context> Elaborator<'context> { definition, mutable, new_definitions, + warn_if_unused, global_id, ) } @@ -180,7 +195,7 @@ impl<'context> Elaborator<'context> { // shadowing here lets us avoid further errors if we define ERROR_IDENT // multiple times. let name = ERROR_IDENT.into(); - let identifier = this.add_variable_decl(name, false, true, definition.clone()); + let identifier = this.add_variable_decl(name, false, true, true, definition.clone()); HirPattern::Identifier(identifier) }; @@ -263,6 +278,7 @@ impl<'context> Elaborator<'context> { definition.clone(), mutable, new_definitions, + true, None, ); @@ -295,16 +311,6 @@ impl<'context> Elaborator<'context> { } pub(super) fn add_variable_decl( - &mut self, - name: Ident, - mutable: bool, - allow_shadowing: bool, - definition: DefinitionKind, - ) -> HirIdent { - self.add_variable_decl_inner(name, mutable, allow_shadowing, true, definition) - } - - pub fn add_variable_decl_inner( &mut self, name: Ident, mutable: bool, diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 2d46c4c634..a46c7df8bb 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -98,12 +98,16 @@ impl<'context> Elaborator<'context> { } } + let warn_if_unused = + !let_stmt.attributes.iter().any(|attr| attr.is_allow_unused_variables()); + let r#type = annotated_type; let pattern = self.elaborate_pattern_and_store_ids( let_stmt.pattern, r#type.clone(), definition, &mut Vec::new(), + warn_if_unused, global_id, ); @@ -215,7 +219,7 @@ impl<'context> Elaborator<'context> { // TODO: For loop variables are currently mutable by default since we haven't // yet implemented syntax for them to be optionally mutable. let kind = DefinitionKind::Local(None); - let identifier = self.add_variable_decl(identifier, false, true, kind); + let identifier = self.add_variable_decl(identifier, false, true, true, kind); // Check that start range and end range have the same types let range_span = start_span.merge(end_span); diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 972826f5b7..4344d19829 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -29,7 +29,7 @@ impl HirStatement { let pattern = let_stmt.pattern.to_display_ast(interner); let r#type = interner.id_type(let_stmt.expression).to_display_ast(); let expression = let_stmt.expression.to_display_ast(interner); - StatementKind::new_let(pattern, r#type, expression) + StatementKind::new_let(pattern, r#type, expression, let_stmt.attributes.clone()) } HirStatement::Constrain(constrain) => { let expr = constrain.0.to_display_ast(interner); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 4678d29a45..d716d55e1a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -2338,6 +2338,7 @@ fn function_def_set_parameters( parameter_type.clone(), DefinitionKind::Local(None), &mut parameter_idents, + true, None, ) }); diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index b4f82a6818..97faea4b44 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -955,6 +955,13 @@ impl SecondaryAttribute { SecondaryAttribute::Allow(_) => Some("allow".to_string()), } } + + pub(crate) fn is_allow_unused_variables(&self) -> bool { + match self { + SecondaryAttribute::Allow(string) => string == "unused_variables", + _ => false, + } + } } impl fmt::Display for SecondaryAttribute { diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index b007653062..1438753d62 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -54,7 +54,7 @@ use crate::ast::{ }; use crate::lexer::Lexer; use crate::parser::{force, ignore_then_commit, statement_recovery}; -use crate::token::{Keyword, Token, TokenKind}; +use crate::token::{Keyword, SecondaryAttribute, Token, TokenKind}; use acvm::AcirField; use chumsky::prelude::*; @@ -535,23 +535,30 @@ where fn let_statement<'a, P>( expr_parser: P, -) -> impl NoirParser<((Pattern, UnresolvedType), Expression)> + 'a +) -> impl NoirParser<(Vec, Pattern, UnresolvedType, Expression)> + 'a where P: ExprParser + 'a, { - let p = - ignore_then_commit(keyword(Keyword::Let).labelled(ParsingRuleLabel::Statement), pattern()); + let p = attributes(); + let p = p.then_ignore(keyword(Keyword::Let).labelled(ParsingRuleLabel::Statement)); + let p = then_commit(p, pattern()); let p = p.then(optional_type_annotation()); let p = then_commit_ignore(p, just(Token::Assign)); - then_commit(p, expr_parser) + let p = then_commit(p, expr_parser); + p.validate(|(((attributes, pattern), typ), expr), span, emit| { + let attributes = attributes::validate_secondary_attributes(attributes, span, emit); + + (attributes, pattern, typ, expr) + }) } fn declaration<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - let_statement(expr_parser) - .map(|((pattern, typ), expr)| StatementKind::new_let(pattern, typ, expr)) + let_statement(expr_parser).map(|(attributes, pattern, typ, expr)| { + StatementKind::new_let(pattern, typ, expr, attributes) + }) } pub fn pattern() -> impl NoirParser { diff --git a/compiler/noirc_frontend/src/parser/parser/traits.rs b/compiler/noirc_frontend/src/parser/parser/traits.rs index b95319f6da..ce8a201707 100644 --- a/compiler/noirc_frontend/src/parser/parser/traits.rs +++ b/compiler/noirc_frontend/src/parser/parser/traits.rs @@ -203,7 +203,7 @@ fn trait_implementation_body() -> impl NoirParser> .map(|(name, alias)| TraitImplItemKind::Type { name, alias }); let let_statement = let_statement(expression()).then_ignore(just(Token::Semicolon)).try_map( - |((pattern, typ), expr), span| match pattern { + |(_attributes, pattern, typ, expr), span| match pattern { Pattern::Identifier(ident) => Ok(TraitImplItemKind::Constant(ident, typ, expr)), _ => Err(ParserError::with_reason( ParserErrorReason::PatternInTraitFunctionParameter, diff --git a/compiler/noirc_frontend/src/tests/unused_items.rs b/compiler/noirc_frontend/src/tests/unused_items.rs index b49414d8b0..da0a40d93c 100644 --- a/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/compiler/noirc_frontend/src/tests/unused_items.rs @@ -1,4 +1,7 @@ -use crate::hir::{def_collector::dc_crate::CompilationError, resolution::errors::ResolverError}; +use crate::{ + hir::{def_collector::dc_crate::CompilationError, resolution::errors::ResolverError}, + tests::assert_no_errors, +}; use super::get_program_errors; @@ -157,3 +160,14 @@ fn errors_on_unused_trait() { assert_eq!(ident.to_string(), "Foo"); assert_eq!(*item_type, "trait"); } + +#[test] +fn silences_unused_variable_warning() { + let src = r#" + fn main() { + #[allow(unused_variables)] + let x = 1; + } + "#; + assert_no_errors(src); +} From 5369e9aaedea9bd8877499a54077450da9bd8906 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 24 Sep 2024 14:38:41 -0300 Subject: [PATCH 3/5] Suggest `allow(unused_variables)` in LSP completion --- compiler/noirc_frontend/src/ast/visitor.rs | 5 +++++ .../lsp/src/requests/completion/builtins.rs | 9 +++++++++ .../requests/completion/completion_items.rs | 4 ++++ tooling/lsp/src/requests/completion/tests.rs | 20 +++++++++++++++++++ 4 files changed, 38 insertions(+) diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 80442d2939..6ce8a70a0c 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -32,6 +32,7 @@ pub enum AttributeTarget { Struct, Trait, Function, + Let, } /// Implements the [Visitor pattern](https://en.wikipedia.org/wiki/Visitor_pattern) for Noir's AST. @@ -1097,6 +1098,10 @@ impl Statement { impl LetStatement { pub fn accept(&self, visitor: &mut impl Visitor) { + for attribute in &self.attributes { + attribute.accept(AttributeTarget::Let, visitor); + } + if visitor.visit_let_statement(self) { self.accept_children(visitor); } diff --git a/tooling/lsp/src/requests/completion/builtins.rs b/tooling/lsp/src/requests/completion/builtins.rs index 6812ebc135..cf2af4036f 100644 --- a/tooling/lsp/src/requests/completion/builtins.rs +++ b/tooling/lsp/src/requests/completion/builtins.rs @@ -116,6 +116,15 @@ impl<'a> NodeFinder<'a> { )); } } + AttributeTarget::Let => { + if name_matches("allow", prefix) || name_matches("unused_variables", prefix) { + self.completion_items.push(simple_completion_item( + "allow(unused_variables)", + CompletionItemKind::METHOD, + None, + )); + } + } } } } diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index c0155096dc..809988c34a 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -51,6 +51,10 @@ impl<'a> NodeFinder<'a> { AttributeTarget::Struct => Some(Type::Quoted(QuotedType::StructDefinition)), AttributeTarget::Trait => Some(Type::Quoted(QuotedType::TraitDefinition)), AttributeTarget::Function => Some(Type::Quoted(QuotedType::FunctionDefinition)), + AttributeTarget::Let => { + // No item can be suggested for a let statement attribute + return Vec::new(); + } } } else { None diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 45eb79bd1c..68cb3870f6 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -1942,6 +1942,26 @@ mod completion_tests { .await; } + #[test] + async fn test_suggests_built_in_let_attribute() { + let src = r#" + fn foo() { + #[allo>|<] + let x = 1; + } + "#; + + assert_completion_excluding_auto_import( + src, + vec![simple_completion_item( + "allow(unused_variables)", + CompletionItemKind::METHOD, + None, + )], + ) + .await; + } + #[test] async fn test_suggests_function_attribute() { let src = r#" From 42064896b2fd1b4f7c1cafc92e4466af78651d5c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 24 Sep 2024 16:35:52 -0300 Subject: [PATCH 4/5] Apply suggestions from code review --- compiler/noirc_frontend/src/elaborator/mod.rs | 9 +++++++-- compiler/noirc_frontend/src/elaborator/patterns.rs | 4 ++-- compiler/noirc_frontend/src/elaborator/statements.rs | 7 ++++++- .../src/hir/comptime/interpreter/builtin.rs | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 31592b0225..c36231bcf3 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -362,7 +362,12 @@ impl<'context> Elaborator<'context> { if let Kind::Numeric(typ) = &generic.kind { let definition = DefinitionKind::GenericType(generic.type_var.clone()); let ident = Ident::new(generic.name.to_string(), generic.span); - let hir_ident = self.add_variable_decl(ident, false, false, false, definition); + let hir_ident = self.add_variable_decl( + ident, false, // mutable + false, // allow_shadowing + false, // warn_if_unused + definition, + ); self.interner.push_definition_type(hir_ident.id, *typ.clone()); } } @@ -759,7 +764,7 @@ impl<'context> Elaborator<'context> { typ.clone(), DefinitionKind::Local(None), &mut parameter_idents, - true, + true, // warn_if_unused None, ); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 46bf861867..6ed59a61e4 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -89,7 +89,7 @@ impl<'context> Elaborator<'context> { self.add_variable_decl( name, mutable.is_some(), - true, + true, // allow_shadowing warn_if_unused, definition, ) @@ -278,7 +278,7 @@ impl<'context> Elaborator<'context> { definition.clone(), mutable, new_definitions, - true, + true, // warn_if_unused None, ); diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index a46c7df8bb..55b641ca3d 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -219,7 +219,12 @@ impl<'context> Elaborator<'context> { // TODO: For loop variables are currently mutable by default since we haven't // yet implemented syntax for them to be optionally mutable. let kind = DefinitionKind::Local(None); - let identifier = self.add_variable_decl(identifier, false, true, true, kind); + let identifier = self.add_variable_decl( + identifier, false, // mutable + true, // allow_shadowing + true, // warn_if_unused + kind, + ); // Check that start range and end range have the same types let range_span = start_span.merge(end_span); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index d716d55e1a..2e118eb4f0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -2338,7 +2338,7 @@ fn function_def_set_parameters( parameter_type.clone(), DefinitionKind::Local(None), &mut parameter_idents, - true, + true, // warn_if_unused None, ) }); From 5db563ff5421ecc9baf7bf3560b9709ad7210170 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 24 Sep 2024 16:39:38 -0300 Subject: [PATCH 5/5] Don't parse attributes in trait let statement --- compiler/noirc_frontend/src/parser/parser.rs | 25 ++++++++----------- .../src/parser/parser/traits.rs | 2 +- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 1438753d62..0eb49ae975 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -54,7 +54,7 @@ use crate::ast::{ }; use crate::lexer::Lexer; use crate::parser::{force, ignore_then_commit, statement_recovery}; -use crate::token::{Keyword, SecondaryAttribute, Token, TokenKind}; +use crate::token::{Keyword, Token, TokenKind}; use acvm::AcirField; use chumsky::prelude::*; @@ -535,30 +535,27 @@ where fn let_statement<'a, P>( expr_parser: P, -) -> impl NoirParser<(Vec, Pattern, UnresolvedType, Expression)> + 'a +) -> impl NoirParser<((Pattern, UnresolvedType), Expression)> + 'a where P: ExprParser + 'a, { - let p = attributes(); - let p = p.then_ignore(keyword(Keyword::Let).labelled(ParsingRuleLabel::Statement)); - let p = then_commit(p, pattern()); + let p = + ignore_then_commit(keyword(Keyword::Let).labelled(ParsingRuleLabel::Statement), pattern()); let p = p.then(optional_type_annotation()); let p = then_commit_ignore(p, just(Token::Assign)); - let p = then_commit(p, expr_parser); - p.validate(|(((attributes, pattern), typ), expr), span, emit| { - let attributes = attributes::validate_secondary_attributes(attributes, span, emit); - - (attributes, pattern, typ, expr) - }) + then_commit(p, expr_parser) } fn declaration<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - let_statement(expr_parser).map(|(attributes, pattern, typ, expr)| { - StatementKind::new_let(pattern, typ, expr, attributes) - }) + attributes().then(let_statement(expr_parser)).validate( + |(attributes, ((pattern, typ), expr)), span, emit| { + let attributes = attributes::validate_secondary_attributes(attributes, span, emit); + StatementKind::new_let(pattern, typ, expr, attributes) + }, + ) } pub fn pattern() -> impl NoirParser { diff --git a/compiler/noirc_frontend/src/parser/parser/traits.rs b/compiler/noirc_frontend/src/parser/parser/traits.rs index ce8a201707..b95319f6da 100644 --- a/compiler/noirc_frontend/src/parser/parser/traits.rs +++ b/compiler/noirc_frontend/src/parser/parser/traits.rs @@ -203,7 +203,7 @@ fn trait_implementation_body() -> impl NoirParser> .map(|(name, alias)| TraitImplItemKind::Type { name, alias }); let let_statement = let_statement(expression()).then_ignore(just(Token::Semicolon)).try_map( - |(_attributes, pattern, typ, expr), span| match pattern { + |((pattern, typ), expr), span| match pattern { Pattern::Identifier(ident) => Ok(TraitImplItemKind::Constant(ident, typ, expr)), _ => Err(ParserError::with_reason( ParserErrorReason::PatternInTraitFunctionParameter,