From b3f78875de2f56867df3232e691b0f382ddd79d9 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 21 Feb 2025 15:09:29 -0600 Subject: [PATCH 1/2] Support constructors in match patterns --- .../noirc_frontend/src/elaborator/enums.rs | 75 ++++++++++++++++++- .../compile_success_empty/enums/src/main.nr | 27 ++++++- 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index bc36849127d..b4ed96b40fa 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use fm::FileId; use fxhash::FxHashMap as HashMap; use iter_extended::{try_vecmap, vecmap}; @@ -5,8 +7,8 @@ use noirc_errors::Location; use crate::{ ast::{ - EnumVariant, Expression, ExpressionKind, FunctionKind, Literal, NoirEnumeration, - StatementKind, UnresolvedType, Visibility, + ConstructorExpression, EnumVariant, Expression, ExpressionKind, FunctionKind, Ident, + Literal, NoirEnumeration, StatementKind, UnresolvedType, Visibility, }, elaborator::path_resolution::PathResolutionItem, hir::{comptime::Value, resolution::errors::ResolverError, type_check::TypeCheckError}, @@ -355,7 +357,7 @@ impl Elaborator<'_> { ExpressionKind::Call(call) => { self.expression_to_constructor(*call.func, call.arguments, expected_type) } - ExpressionKind::Constructor(_) => todo!("handle constructors"), + ExpressionKind::Constructor(constructor) => self.constructor_to_pattern(*constructor), ExpressionKind::Tuple(fields) => { let field_types = vecmap(0..fields.len(), |_| self.interner.next_type_variable()); let actual = Type::Tuple(field_types.clone()); @@ -408,6 +410,54 @@ impl Elaborator<'_> { } } + fn constructor_to_pattern(&mut self, constructor: ConstructorExpression) -> Pattern { + let location = constructor.typ.location; + let typ = match constructor.struct_type { + Some(id) => { + let typ = self.interner.get_type(id); + let generics = typ.borrow().instantiate(&mut self.interner); + Type::DataType(typ, generics) + } + None => self.resolve_type(constructor.typ), + }; + + let Some((struct_name, mut expected_field_types)) = + self.struct_name_and_field_types(&typ, location) + else { + return Pattern::Error; + }; + + let mut fields = BTreeMap::default(); + for (field_name, field) in constructor.fields { + let Some(field_index) = + expected_field_types.iter().position(|(name, _)| *name == field_name.0.contents) + else { + let error = if fields.contains_key(&field_name.0.contents) { + ResolverError::DuplicateField { field: field_name } + } else { + let struct_definition = struct_name.clone(); + ResolverError::NoSuchField { field: field_name, struct_definition } + }; + self.push_err(error, self.file); + continue; + }; + + let (field_name, expected_field_type) = expected_field_types.swap_remove(field_index); + fields.insert(field_name, self.expression_to_pattern(field, &expected_field_type)); + } + + if !expected_field_types.is_empty() { + let struct_definition = struct_name; + let span = location.span; + let missing_fields = vecmap(expected_field_types, |(name, _)| name); + let error = ResolverError::MissingFields { span, missing_fields, struct_definition }; + self.push_err(error, self.file); + } + + let args = vecmap(fields, |(_name, field)| field); + Pattern::Constructor(Constructor::Variant(typ, 0), args) + } + fn expression_to_constructor( &mut self, name: Expression, @@ -521,6 +571,23 @@ impl Elaborator<'_> { Pattern::Constructor(constructor, args) } + fn struct_name_and_field_types( + &mut self, + typ: &Type, + location: Location, + ) -> Option<(Ident, Vec<(String, Type)>)> { + if let Type::DataType(typ, generics) = typ.follow_bindings_shallow().as_ref() { + if let Some(fields) = typ.borrow().get_fields(generics) { + return Some((typ.borrow().name.clone(), fields)); + } + } + + let error = + ResolverError::NonStructUsedInConstructor { typ: typ.to_string(), span: location.span }; + self.push_err(error, location.file); + None + } + /// Compiles the rows of a match expression, outputting a decision tree for the match. /// /// This is an adaptation of https://github.com/yorickpeterse/pattern-matching-in-rust/tree/main/jacobs2021 @@ -900,6 +967,8 @@ enum Pattern { /// 1 <= n < 20. #[allow(unused)] Range(SignedField, SignedField), + + Error, } #[derive(Clone)] diff --git a/test_programs/compile_success_empty/enums/src/main.nr b/test_programs/compile_success_empty/enums/src/main.nr index 03fc10a1c35..e862c2bc54f 100644 --- a/test_programs/compile_success_empty/enums/src/main.nr +++ b/test_programs/compile_success_empty/enums/src/main.nr @@ -3,6 +3,7 @@ fn main() { foo_tests(); option_tests(); abc_tests(); + match_on_structs(); } fn primitive_tests() { @@ -103,7 +104,7 @@ fn abc_tests() { // Mut is only to throw the optimizer off a bit so we can see // the `eq`s that get generated before they're removed because each of these are constant let mut tuple = (ABC::A, ABC::B); - match tuple { + let _ = match tuple { (ABC::A, _) => 1, (_, ABC::A) => 2, (_, ABC::B) => 3, @@ -114,3 +115,27 @@ fn abc_tests() { _ => 0, }; } + +fn match_on_structs() { + let foo = MyStruct { x: 10, y: 20 }; + match foo { + MyStruct { x, y } => { + assert_eq(x, 10); + assert_eq(y, 20); + }, + } + + match MyOption::Some(foo) { + MyOption::Some(MyStruct { x: x2, y: y2 }) => { + assert_eq(x2, 10); + assert_eq(y2, 20); + }, + MyOption::None => fail(), + MyOption::Maybe => fail(), + } +} + +struct MyStruct { + x: i32, + y: Field, +} From 97a692621515e800fa2df8b8c53c1687f47c010b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 24 Feb 2025 09:44:36 -0600 Subject: [PATCH 2/2] Add tests and allow '_' to be redefined --- .../noirc_frontend/src/elaborator/enums.rs | 22 +++--- compiler/noirc_frontend/src/tests.rs | 75 +++++++++++++++++++ 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 4346bee5544..b09deb58b15 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -358,11 +358,14 @@ impl Elaborator<'_> { 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); + // Allow redefinition of `_` only, to ignore variables + if last_ident.0.contents != "_" { + 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()); } @@ -444,14 +447,7 @@ impl Elaborator<'_> { variables_defined: &mut Vec, ) -> Pattern { let location = constructor.typ.location; - let typ = match constructor.struct_type { - Some(id) => { - let typ = self.interner.get_type(id); - let generics = typ.borrow().instantiate(self.interner); - Type::DataType(typ, generics) - } - None => self.resolve_type(constructor.typ), - }; + let typ = self.resolve_type(constructor.typ); let Some((struct_name, mut expected_field_types)) = self.struct_name_and_field_types(&typ, location) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 07a54c45702..a238d7d6bb5 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4494,3 +4494,78 @@ fn errors_on_repeated_match_variables_in_pattern() { CompilationError::ResolverError(ResolverError::VariableAlreadyDefinedInPattern { .. }) )); } + +#[test] +fn duplicate_field_in_match_struct_pattern() { + let src = r#" +fn main() { + let foo = Foo { x: 10, y: 20 }; + match foo { + Foo { x: _, x: _, y: _ } => {} + } +} + +struct Foo { + x: i32, + y: Field, +} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::DuplicateField { .. }) + )); +} + +#[test] +fn missing_field_in_match_struct_pattern() { + let src = r#" +fn main() { + let foo = Foo { x: 10, y: 20 }; + match foo { + Foo { x: _ } => {} + } +} + +struct Foo { + x: i32, + y: Field, +} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::MissingFields { .. }) + )); +} + +#[test] +fn no_such_field_in_match_struct_pattern() { + let src = r#" +fn main() { + let foo = Foo { x: 10, y: 20 }; + match foo { + Foo { x: _, y: _, z: _ } => {} + } +} + +struct Foo { + x: i32, + y: Field, +} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::NoSuchField { .. }) + )); +}