diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index bc36849127d..3b57d9af2b9 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -5,7 +5,7 @@ use noirc_errors::Location; use crate::{ ast::{ - EnumVariant, Expression, ExpressionKind, FunctionKind, Literal, NoirEnumeration, + EnumVariant, Expression, ExpressionKind, FunctionKind, Ident, Literal, NoirEnumeration, StatementKind, UnresolvedType, Visibility, }, elaborator::path_resolution::PathResolutionItem, @@ -271,7 +271,8 @@ impl Elaborator<'_> { let rows = vecmap(rules, |(pattern, branch)| { self.push_scope(); - let pattern = self.expression_to_pattern(pattern, &expected_pattern_type); + let pattern = + self.expression_to_pattern(pattern, &expected_pattern_type, &mut Vec::new()); let columns = vec![Column::new(variable_to_match, pattern)]; let guard = None; @@ -293,7 +294,12 @@ impl Elaborator<'_> { } /// Convert an expression into a Pattern, defining any variables within. - fn expression_to_pattern(&mut self, expression: Expression, expected_type: &Type) -> Pattern { + fn expression_to_pattern( + &mut self, + expression: Expression, + expected_type: &Type, + variables_defined: &mut Vec, + ) -> Pattern { let expr_location = expression.type_location(); let unify_with_expected_type = |this: &mut Self, actual| { this.unify(actual, expected_type, expr_location.file, || { @@ -333,13 +339,25 @@ impl Elaborator<'_> { Vec::new(), expected_type, location, + variables_defined, ), Err(_) if path_len == 1 => { // Define the variable let kind = DefinitionKind::Local(None); - // TODO: `allow_shadowing` is false while I'm too lazy to add a check that we - // don't define the same name multiple times in one pattern. - let id = self.add_variable_decl(last_ident, false, false, true, kind).id; + + if let Some(existing) = + variables_defined.iter().find(|elem| *elem == &last_ident) + { + let error = ResolverError::VariableAlreadyDefinedInPattern { + existing: existing.clone(), + new_span: last_ident.span(), + }; + self.push_err(error, self.file); + } else { + variables_defined.push(last_ident.clone()); + } + + let id = self.add_variable_decl(last_ident, false, true, true, kind).id; self.interner.push_definition_type(id, expected_type.clone()); Pattern::Binding(id) } @@ -352,9 +370,12 @@ impl Elaborator<'_> { } } } - ExpressionKind::Call(call) => { - self.expression_to_constructor(*call.func, call.arguments, expected_type) - } + ExpressionKind::Call(call) => self.expression_to_constructor( + *call.func, + call.arguments, + expected_type, + variables_defined, + ), ExpressionKind::Constructor(_) => todo!("handle constructors"), ExpressionKind::Tuple(fields) => { let field_types = vecmap(0..fields.len(), |_| self.interner.next_type_variable()); @@ -363,21 +384,23 @@ impl Elaborator<'_> { let fields = vecmap(fields.into_iter().enumerate(), |(i, field)| { let expected = field_types.get(i).unwrap_or(&Type::Error); - self.expression_to_pattern(field, expected) + self.expression_to_pattern(field, expected, variables_defined) }); Pattern::Constructor(Constructor::Tuple(field_types.clone()), fields) } - ExpressionKind::Parenthesized(expr) => self.expression_to_pattern(*expr, expected_type), + ExpressionKind::Parenthesized(expr) => { + self.expression_to_pattern(*expr, expected_type, variables_defined) + } ExpressionKind::Interned(id) => { let kind = self.interner.get_expression_kind(id); let expr = Expression::new(kind.clone(), expression.location); - self.expression_to_pattern(expr, expected_type) + self.expression_to_pattern(expr, expected_type, variables_defined) } ExpressionKind::InternedStatement(id) => { if let StatementKind::Expression(expr) = self.interner.get_statement_kind(id) { - self.expression_to_pattern(expr.clone(), expected_type) + self.expression_to_pattern(expr.clone(), expected_type, variables_defined) } else { panic!("Invalid expr kind {expression}") } @@ -413,6 +436,7 @@ impl Elaborator<'_> { name: Expression, args: Vec, expected_type: &Type, + variables_defined: &mut Vec, ) -> Pattern { match name.kind { ExpressionKind::Variable(path) => { @@ -424,6 +448,7 @@ impl Elaborator<'_> { args, expected_type, location, + variables_defined, ), Err(error) => { self.push_err(error, location.file); @@ -433,16 +458,21 @@ impl Elaborator<'_> { } } ExpressionKind::Parenthesized(expr) => { - self.expression_to_constructor(*expr, args, expected_type) + self.expression_to_constructor(*expr, args, expected_type, variables_defined) } ExpressionKind::Interned(id) => { let kind = self.interner.get_expression_kind(id); let expr = Expression::new(kind.clone(), name.location); - self.expression_to_constructor(expr, args, expected_type) + self.expression_to_constructor(expr, args, expected_type, variables_defined) } ExpressionKind::InternedStatement(id) => { if let StatementKind::Expression(expr) = self.interner.get_statement_kind(id) { - self.expression_to_constructor(expr.clone(), args, expected_type) + self.expression_to_constructor( + expr.clone(), + args, + expected_type, + variables_defined, + ) } else { panic!("Invalid expr kind {name}") } @@ -457,6 +487,7 @@ impl Elaborator<'_> { args: Vec, expected_type: &Type, location: Location, + variables_defined: &mut Vec, ) -> Pattern { let span = location.span; @@ -515,7 +546,7 @@ impl Elaborator<'_> { let args = args.into_iter().zip(expected_arg_types); let args = vecmap(args, |(arg, expected_arg_type)| { - self.expression_to_pattern(arg, &expected_arg_type) + self.expression_to_pattern(arg, &expected_arg_type, variables_defined) }); let constructor = Constructor::Variant(actual_type, variant_index); Pattern::Constructor(constructor, args) diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 5af0cf41a62..f5ed1475087 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -182,6 +182,8 @@ pub enum ResolverError { LoopNotYetSupported { span: Span }, #[error("Expected a trait but found {found}")] ExpectedTrait { found: String, span: Span }, + #[error("Variable '{existing}' was already defined in the same match pattern")] + VariableAlreadyDefinedInPattern { existing: Ident, new_span: Span }, } impl ResolverError { @@ -691,8 +693,14 @@ impl<'a> From<&'a ResolverError> for Diagnostic { format!("Expected a trait, found {found}"), String::new(), *span) - } + ResolverError::VariableAlreadyDefinedInPattern { existing, new_span } => { + let message = format!("Variable `{existing}` was already defined in the same match pattern"); + let secondary = format!("`{existing}` redefined here"); + let mut error = Diagnostic::simple_error(message, secondary, *new_span); + error.add_secondary(format!("`{existing}` was previously defined here"), existing.span()); + error + }, } } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 7b1ffabd4e9..07a54c45702 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4475,3 +4475,22 @@ fn errors_on_unspecified_unstable_match() { assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))); } + +#[test] +fn errors_on_repeated_match_variables_in_pattern() { + let src = r#" + fn main() { + match (1, 2) { + (_x, _x) => (), + } + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::VariableAlreadyDefinedInPattern { .. }) + )); +}