From 9934115e31c88176758d3f50193b5b9ec32d718b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 24 Feb 2025 13:37:25 -0600 Subject: [PATCH 1/5] Pause work since we error on INT_MIN globals --- .../noirc_frontend/src/elaborator/enums.rs | 81 +++++++++++++++++-- compiler/noirc_frontend/src/elaborator/mod.rs | 14 ++++ .../src/hir/resolution/errors.rs | 7 ++ .../compile_success_empty/enums/src/main.nr | 12 +++ 4 files changed, 107 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index b09deb58b15..049e45f89ea 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; +use acvm::{AcirField, FieldElement}; use fm::FileId; use fxhash::FxHashMap as HashMap; use iter_extended::{try_vecmap, vecmap}; @@ -316,8 +317,8 @@ impl Elaborator<'_> { // We want the actual expression's span here, not the innermost one from `type_location()` let span = expression.location.span; let syntax_error = |this: &mut Self| { - let errors = ResolverError::InvalidSyntaxInPattern { span }; - this.push_err(errors, this.file); + let error = ResolverError::InvalidSyntaxInPattern { span }; + this.push_err(error, this.file); Pattern::Error }; @@ -495,6 +496,12 @@ impl Elaborator<'_> { expected_type: &Type, variables_defined: &mut Vec, ) -> Pattern { + let syntax_error = |this: &mut Self| { + let error = ResolverError::InvalidSyntaxInPattern { span: name.location.span }; + this.push_err(error, this.file); + Pattern::Error + }; + match name.kind { ExpressionKind::Variable(path) => { let location = path.location; @@ -531,10 +538,10 @@ impl Elaborator<'_> { variables_defined, ) } else { - panic!("Invalid expr kind {name}") + syntax_error(self) } } - other => todo!("invalid constructor `{other}`"), + _ => syntax_error(self), } } @@ -551,10 +558,20 @@ impl Elaborator<'_> { let (actual_type, expected_arg_types, variant_index) = match name { PathResolutionItem::Global(id) => { // variant constant + self.elaborate_global_if_unresolved(&id); let global = self.interner.get_global(id); - let variant_index = match global.value { - GlobalValue::Resolved(Value::Enum(tag, ..)) => tag, - _ => todo!("Value is not an enum constant"), + let variant_index = match &global.value { + GlobalValue::Resolved(Value::Enum(tag, ..)) => *tag, + // This may be a global constant. Treat it like a normal constant + GlobalValue::Resolved(value) => { + let value = value.clone(); + return self.global_constant_to_integer_constructor( + value, + expected_type, + location, + ); + } + other => todo!("Value `{other:?}` is not an enum constant"), }; let global_type = self.interner.definition_type(global.definition_id); @@ -609,6 +626,56 @@ impl Elaborator<'_> { Pattern::Constructor(constructor, args) } + fn global_constant_to_integer_constructor( + &mut self, + constant: Value, + expected_type: &Type, + location: Location, + ) -> Pattern { + let actual_type = constant.get_type(); + self.unify(&actual_type, expected_type, location.file, || TypeCheckError::TypeMismatch { + expected_typ: expected_type.to_string(), + expr_typ: actual_type.to_string(), + expr_span: location.span, + }); + + // Convert a signed integer type like i32 to SignedField + macro_rules! signed_to_signed_field { + ($value:expr) => {{ + let negative = $value < 0; + // Widen the value so that SignedType::MIN does not wrap to 0 when negated below + let mut widened = $value as i128; + if negative { + widened = -widened; + } + SignedField::new(widened.into(), negative) + }}; + } + + let value = match constant { + Value::Bool(value) => SignedField::new(value.into(), false), + Value::Field(value) => SignedField::new(value, false), + Value::I8(value) => signed_to_signed_field!(value), + Value::I16(value) => signed_to_signed_field!(value), + Value::I32(value) => signed_to_signed_field!(value), + Value::I64(value) => signed_to_signed_field!(value), + Value::U1(value) => SignedField::new(value.into(), false), + Value::U8(value) => SignedField::new((value as u128).into(), false), + Value::U16(value) => SignedField::new((value as u128).into(), false), + Value::U32(value) => SignedField::new(value.into(), false), + Value::U64(value) => SignedField::new(value.into(), false), + Value::U128(value) => SignedField::new(value.into(), false), + Value::Zeroed(_) => SignedField::new(FieldElement::zero(), false), + _ => { + let error = ResolverError::NonIntegerGlobalUsedInPattern { location }; + self.push_err(error, self.file); + return Pattern::Error; + } + }; + + Pattern::Int(value) + } + fn struct_name_and_field_types( &mut self, typ: &Type, diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 7b553b88ac9..22d1ddcaf4f 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1983,6 +1983,8 @@ impl<'context> Elaborator<'context> { let old_file = std::mem::replace(&mut self.file, global.file_id); let old_item = self.current_item.take(); + eprintln!("Elaborating {global:?}"); + let global_id = global.global_id; self.current_item = Some(DependencyId::Global(global_id)); let let_stmt = global.stmt_def; @@ -2006,13 +2008,18 @@ impl<'context> Elaborator<'context> { self.push_err(ResolverError::MutableGlobal { span }, location.file); } + dbg!(); + let (let_statement, _typ) = self .elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))); + dbg!(); let statement_id = self.interner.get_global(global_id).let_statement; self.interner.replace_statement(statement_id, let_statement); + dbg!(); self.elaborate_comptime_global(global_id); + dbg!(); if let Some(name) = name { self.interner.register_global( @@ -2034,22 +2041,27 @@ impl<'context> Elaborator<'context> { .interner .get_global_let_statement(global_id) .expect("Let statement of global should be set by elaborate_global_let"); + dbg!(); let global = self.interner.get_global(global_id); let definition_id = global.definition_id; let location = global.location; let mut interpreter = self.setup_interpreter(); + dbg!(); if let Err(error) = interpreter.evaluate_let(let_statement) { + dbg!(&error); let (error, file) = error.into_compilation_error_pair(); self.push_err(error, file); } else { + dbg!(); let value = interpreter .lookup_id(definition_id, location) .expect("The global should be defined since evaluate_let did not error"); self.debug_comptime(location, |interner| value.display(interner).to_string()); + eprintln!(" set {global_id:?}.value = Resolved({value:?})"); self.interner.get_global_mut(global_id).value = GlobalValue::Resolved(value); } } @@ -2057,9 +2069,11 @@ impl<'context> Elaborator<'context> { /// If the given global is unresolved, elaborate it and return true fn elaborate_global_if_unresolved(&mut self, global_id: &GlobalId) -> bool { if let Some(global) = self.unresolved_globals.remove(global_id) { + eprintln!("Elaborating {global_id:?}: {global:?}"); self.elaborate_global(global); true } else { + eprintln!("Not elaborating {global_id:?}"); false } } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 1e6237788c7..dd7ac8f91c1 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -186,6 +186,8 @@ pub enum ResolverError { InvalidSyntaxInPattern { span: Span }, #[error("Variable '{existing}' was already defined in the same match pattern")] VariableAlreadyDefinedInPattern { existing: Ident, new_span: Span }, + #[error("Only integer globals can be used in match patterns")] + NonIntegerGlobalUsedInPattern { location: Location }, } impl ResolverError { @@ -709,6 +711,11 @@ impl<'a> From<&'a ResolverError> for Diagnostic { error.add_secondary(format!("`{existing}` was previously defined here"), existing.span()); error }, + ResolverError::NonIntegerGlobalUsedInPattern { location } => { + let message = "Only integer or boolean globals can be used in match patterns".to_string(); + let secondary = "This global is not an integer or boolean".to_string(); + Diagnostic::simple_error(message, secondary, location.span) + }, } } } diff --git a/test_programs/compile_success_empty/enums/src/main.nr b/test_programs/compile_success_empty/enums/src/main.nr index e862c2bc54f..3b12af1c15d 100644 --- a/test_programs/compile_success_empty/enums/src/main.nr +++ b/test_programs/compile_success_empty/enums/src/main.nr @@ -20,8 +20,20 @@ fn primitive_tests() { false => fail(), true => (), } + + // Ensure we can match on MIN and use globals as constants + let i64_min = I64_MIN; + match i64_min { + 9_223_372_036_854_775_807 => fail(), + -9_223_372_036_854_775_807 => fail(), + 0 => fail(), + I64_MIN => (), + _ => fail(), + } } +global I64_MIN: i64 = -9_223_372_036_854_775_808; + enum Foo { A(Field, Field), B(u32), From 98d1ccbdcd34531598ceac944abf5cfeec30d983 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 25 Feb 2025 14:55:44 -0600 Subject: [PATCH 2/5] Replace panic with silent error --- compiler/noirc_frontend/src/elaborator/enums.rs | 4 +++- compiler/noirc_frontend/src/elaborator/mod.rs | 14 -------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 049e45f89ea..21785291731 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -571,7 +571,9 @@ impl Elaborator<'_> { location, ); } - other => todo!("Value `{other:?}` is not an enum constant"), + // We tried to resolve this value above so there must have been an error + // in doing so. Avoid reporting an additional error. + _ => return Pattern::Error, }; let global_type = self.interner.definition_type(global.definition_id); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 22d1ddcaf4f..7b553b88ac9 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1983,8 +1983,6 @@ impl<'context> Elaborator<'context> { let old_file = std::mem::replace(&mut self.file, global.file_id); let old_item = self.current_item.take(); - eprintln!("Elaborating {global:?}"); - let global_id = global.global_id; self.current_item = Some(DependencyId::Global(global_id)); let let_stmt = global.stmt_def; @@ -2008,18 +2006,13 @@ impl<'context> Elaborator<'context> { self.push_err(ResolverError::MutableGlobal { span }, location.file); } - dbg!(); - let (let_statement, _typ) = self .elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))); - dbg!(); let statement_id = self.interner.get_global(global_id).let_statement; self.interner.replace_statement(statement_id, let_statement); - dbg!(); self.elaborate_comptime_global(global_id); - dbg!(); if let Some(name) = name { self.interner.register_global( @@ -2041,27 +2034,22 @@ impl<'context> Elaborator<'context> { .interner .get_global_let_statement(global_id) .expect("Let statement of global should be set by elaborate_global_let"); - dbg!(); let global = self.interner.get_global(global_id); let definition_id = global.definition_id; let location = global.location; let mut interpreter = self.setup_interpreter(); - dbg!(); if let Err(error) = interpreter.evaluate_let(let_statement) { - dbg!(&error); let (error, file) = error.into_compilation_error_pair(); self.push_err(error, file); } else { - dbg!(); let value = interpreter .lookup_id(definition_id, location) .expect("The global should be defined since evaluate_let did not error"); self.debug_comptime(location, |interner| value.display(interner).to_string()); - eprintln!(" set {global_id:?}.value = Resolved({value:?})"); self.interner.get_global_mut(global_id).value = GlobalValue::Resolved(value); } } @@ -2069,11 +2057,9 @@ impl<'context> Elaborator<'context> { /// If the given global is unresolved, elaborate it and return true fn elaborate_global_if_unresolved(&mut self, global_id: &GlobalId) -> bool { if let Some(global) = self.unresolved_globals.remove(global_id) { - eprintln!("Elaborating {global_id:?}: {global:?}"); self.elaborate_global(global); true } else { - eprintln!("Not elaborating {global_id:?}"); false } } From 6527c9964b19da5f2252392471d93b05fcc5feec Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 26 Feb 2025 11:06:09 -0600 Subject: [PATCH 3/5] Replace remaining panics with errors --- .../noirc_frontend/src/elaborator/enums.rs | 150 +++++++----- .../src/hir/resolution/errors.rs | 18 ++ compiler/noirc_frontend/src/tests.rs | 161 +------------ compiler/noirc_frontend/src/tests/enums.rs | 228 ++++++++++++++++++ 4 files changed, 343 insertions(+), 214 deletions(-) create mode 100644 compiler/noirc_frontend/src/tests/enums.rs diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 21785291731..d1a18044ab1 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -340,44 +340,30 @@ impl Elaborator<'_> { // - Possible diagnostics improvement: warn if `a` is defined as a variable // when there is a matching enum variant with name `Foo::a` which can // be imported. The user likely intended to reference the enum variant. - let path_len = path.segments.len(); let location = path.location; let last_ident = path.last_ident(); + // Setting this to `Some` allows us to shadow globals with the same name. + // We should avoid this if there is a `::` in the path since that means the + // user is trying to resolve to a non-local item. + let shadow_existing = path.is_ident().then_some(last_ident); + match self.resolve_path_or_error(path) { Ok(resolution) => self.path_resolution_to_constructor( resolution, + shadow_existing, Vec::new(), expected_type, location, variables_defined, ), - Err(_) if path_len == 1 => { - // Define the variable - let kind = DefinitionKind::Local(None); - - if let Some(existing) = - variables_defined.iter().find(|elem| *elem == &last_ident) - { - // 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); - } + Err(error) => { + if let Some(name) = shadow_existing { + self.define_pattern_variable(name, expected_type, variables_defined) } else { - variables_defined.push(last_ident.clone()); + self.push_err(error, location.file); + Pattern::Error } - - 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) - } - Err(error) => { - self.push_err(error, location.file); - Pattern::Error } } } @@ -442,6 +428,33 @@ impl Elaborator<'_> { } } + fn define_pattern_variable( + &mut self, + name: Ident, + expected_type: &Type, + variables_defined: &mut Vec, + ) -> Pattern { + // Define the variable + let kind = DefinitionKind::Local(None); + + if let Some(existing) = variables_defined.iter().find(|elem| *elem == &name) { + // Allow redefinition of `_` only, to ignore variables + if name.0.contents != "_" { + let error = ResolverError::VariableAlreadyDefinedInPattern { + existing: existing.clone(), + new_span: name.span(), + }; + self.push_err(error, self.file); + } + } else { + variables_defined.push(name.clone()); + } + + let id = self.add_variable_decl(name, false, true, true, kind).id; + self.interner.push_definition_type(id, expected_type.clone()); + Pattern::Binding(id) + } + fn constructor_to_pattern( &mut self, constructor: ConstructorExpression, @@ -507,8 +520,11 @@ impl Elaborator<'_> { let location = path.location; match self.resolve_path_or_error(path) { + // Use None for `name` here - we don't want to define a variable if this + // resolves to an existing item. Ok(resolution) => self.path_resolution_to_constructor( resolution, + None, args, expected_type, location, @@ -545,9 +561,16 @@ impl Elaborator<'_> { } } + /// Convert a PathResolutionItem - usually an enum variant or global - to a Constructor. + /// If `name` is `Some`, we'll define a Pattern::Binding instead of erroring if the + /// item doesn't resolve to a variant or global. This would shadow an existing + /// value such as a free function. Generally this is desired unless the variable was + /// a path with multiple components such as `foo::bar` which should always be treated as + /// a path to an existing item. fn path_resolution_to_constructor( &mut self, - name: PathResolutionItem, + resolution: PathResolutionItem, + name: Option, args: Vec, expected_type: &Type, location: Location, @@ -555,11 +578,11 @@ impl Elaborator<'_> { ) -> Pattern { let span = location.span; - let (actual_type, expected_arg_types, variant_index) = match name { + let (actual_type, expected_arg_types, variant_index) = match &resolution { PathResolutionItem::Global(id) => { // variant constant - self.elaborate_global_if_unresolved(&id); - let global = self.interner.get_global(id); + self.elaborate_global_if_unresolved(id); + let global = self.interner.get_global(*id); let variant_index = match &global.value { GlobalValue::Resolved(Value::Enum(tag, ..)) => *tag, // This may be a global constant. Treat it like a normal constant @@ -583,7 +606,13 @@ impl Elaborator<'_> { PathResolutionItem::Method(_type_id, _type_turbofish, func_id) => { // TODO(#7430): Take type_turbofish into account when instantiating the function's type let meta = self.interner.function_meta(&func_id); - let Some(variant_index) = meta.enum_variant_index else { todo!("not a variant") }; + let Some(variant_index) = meta.enum_variant_index else { + let item = resolution.description(); + let span = location.span; + let error = ResolverError::UnexpectedItemInPattern { span, item }; + self.push_err(error, self.file); + return Pattern::Error; + }; let (actual_type, expected_arg_types) = match meta.typ.instantiate(self.interner).0 { @@ -593,18 +622,23 @@ impl Elaborator<'_> { (actual_type, expected_arg_types, variant_index) } - PathResolutionItem::Module(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::Type(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::TypeAlias(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::Trait(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::ModuleFunction(_) => { - todo!("path_resolution_to_constructor {name:?}") - } - PathResolutionItem::TypeAliasFunction(_, _, _) => { - todo!("path_resolution_to_constructor {name:?}") - } - PathResolutionItem::TraitFunction(_, _, _) => { - todo!("path_resolution_to_constructor {name:?}") + PathResolutionItem::Module(_) + | PathResolutionItem::Type(_) + | PathResolutionItem::TypeAlias(_) + | PathResolutionItem::Trait(_) + | PathResolutionItem::ModuleFunction(_) + | PathResolutionItem::TypeAliasFunction(_, _, _) + | PathResolutionItem::TraitFunction(_, _, _) => { + // This variable refers to an existing item + if let Some(name) = name { + // If name is set, shadow the existing item + return self.define_pattern_variable(name, expected_type, variables_defined); + } else { + let item = resolution.description(); + let error = ResolverError::UnexpectedItemInPattern { span, item }; + self.push_err(error, self.file); + return Pattern::Error; + } } }; @@ -617,7 +651,11 @@ impl Elaborator<'_> { }); if args.len() != expected_arg_types.len() { - // error expected N args, found M? + let expected = expected_arg_types.len(); + let found = args.len(); + let error = TypeCheckError::ArityMisMatch { expected, found, span }; + self.push_err(error, self.file); + return Pattern::Error; } let args = args.into_iter().zip(expected_arg_types); @@ -740,8 +778,6 @@ impl Elaborator<'_> { Ok(HirMatch::Switch(branch_var, cases, Some(fallback))) } - Type::Array(_, _) => todo!(), - Type::Slice(_) => todo!(), Type::Bool => { let cases = vec![ (Constructor::False, Vec::new(), Vec::new()), @@ -792,12 +828,17 @@ impl Elaborator<'_> { } else { drop(def); let typ = Type::DataType(type_def, generics); - todo!("Cannot match on type {typ}") + let error = ResolverError::TypeUnsupportedInMatch { typ, span: location.span }; + Err((error, location.file)) } } - typ @ (Type::Alias(_, _) - | Type::TypeVariable(_) + // We could match on these types in the future + typ @ (Type::Array(_, _) + | Type::Slice(_) | Type::String(_) + // But we'll never be able to match on these + | Type::Alias(_, _) + | Type::TypeVariable(_) | Type::FmtString(_, _) | Type::TraitAsType(_, _, _) | Type::NamedGeneric(_, _) @@ -808,7 +849,10 @@ impl Elaborator<'_> { | Type::Constant(_, _) | Type::Quoted(_) | Type::InfixExpr(_, _, _, _) - | Type::Error) => todo!("Cannot match on type {typ:?}"), + | Type::Error) => { + let error = ResolverError::TypeUnsupportedInMatch { typ, span: location.span }; + Err((error, location.file)) + }, } } @@ -846,11 +890,9 @@ impl Elaborator<'_> { let (key, cons) = match col.pattern { Pattern::Int(val) => ((val, val), Constructor::Int(val)), Pattern::Range(start, stop) => ((start, stop), Constructor::Range(start, stop)), - Pattern::Error => continue, - pattern => { - eprintln!("Unexpected pattern for integer type: {pattern:?}"); - continue; - } + // Any other pattern shouldn't have an integer type and we expect a type + // check error to already have been issued. + _ => continue, }; if let Some(index) = tested.get(&key) { diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index dd7ac8f91c1..5a4904c6a75 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -188,6 +188,10 @@ pub enum ResolverError { VariableAlreadyDefinedInPattern { existing: Ident, new_span: Span }, #[error("Only integer globals can be used in match patterns")] NonIntegerGlobalUsedInPattern { location: Location }, + #[error("Cannot match on values of type `{typ}`")] + TypeUnsupportedInMatch { typ: Type, span: Span }, + #[error("Expected a struct, enum, or literal value in pattern, but found a {item}")] + UnexpectedItemInPattern { span: noirc_errors::Span, item: &'static str }, } impl ResolverError { @@ -716,6 +720,20 @@ impl<'a> From<&'a ResolverError> for Diagnostic { let secondary = "This global is not an integer or boolean".to_string(); Diagnostic::simple_error(message, secondary, location.span) }, + ResolverError::TypeUnsupportedInMatch { typ, span } => { + Diagnostic::simple_error( + format!("Cannot match on values of type `{typ}`"), + String::new(), + *span, + ) + }, + ResolverError::UnexpectedItemInPattern { item, span } => { + Diagnostic::simple_error( + format!("Expected a struct, enum, or literal pattern, but found a {item}"), + String::new(), + *span, + ) + }, } } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index a238d7d6bb5..d4b87780b5f 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3,6 +3,7 @@ mod aliases; mod arithmetic_generics; mod bound_checks; +mod enums; mod imports; mod metaprogramming; mod name_shadowing; @@ -4235,28 +4236,6 @@ fn regression_7088() { assert_no_errors(src); } -#[test] -fn error_with_duplicate_enum_variant() { - let src = r#" - enum Foo { - Bar(i32), - Bar(u8), - } - - fn main() {} - "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - assert!(matches!( - &errors[0].0, - CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { .. }) - )); - assert!(matches!( - &errors[1].0, - CompilationError::ResolverError(ResolverError::UnusedItem { .. }) - )); -} - #[test] fn errors_on_empty_loop_no_break() { let src = r#" @@ -4431,141 +4410,3 @@ fn errors_if_while_body_type_is_not_unit() { panic!("Expected a TypeMismatch error"); }; } - -#[test] -fn errors_on_unspecified_unstable_enum() { - // Enums are experimental - this will need to be updated when they are stabilized - let src = r#" - enum Foo { Bar } - - fn main() { - let _x = Foo::Bar; - } - "#; - - let no_features = &[]; - let errors = get_program_using_features(src, no_features).2; - assert_eq!(errors.len(), 1); - - let CompilationError::ParseError(error) = &errors[0].0 else { - panic!("Expected a ParseError experimental feature error"); - }; - - assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))); -} - -#[test] -fn errors_on_unspecified_unstable_match() { - // Enums are experimental - this will need to be updated when they are stabilized - let src = r#" - fn main() { - match 3 { - _ => (), - } - } - "#; - - let no_features = &[]; - let errors = get_program_using_features(src, no_features).2; - assert_eq!(errors.len(), 1); - - let CompilationError::ParseError(error) = &errors[0].0 else { - panic!("Expected a ParseError experimental feature error"); - }; - - 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 { .. }) - )); -} - -#[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 { .. }) - )); -} diff --git a/compiler/noirc_frontend/src/tests/enums.rs b/compiler/noirc_frontend/src/tests/enums.rs new file mode 100644 index 00000000000..9d6fb708351 --- /dev/null +++ b/compiler/noirc_frontend/src/tests/enums.rs @@ -0,0 +1,228 @@ +use crate::{ + hir::{ + def_collector::{dc_crate::CompilationError, errors::DefCollectorErrorKind}, + resolution::errors::ResolverError, + type_check::TypeCheckError, + }, + parser::ParserErrorReason, + tests::{get_program_errors, get_program_using_features}, +}; +use CompilationError::*; +use DefCollectorErrorKind::Duplicate; +use ResolverError::*; +use TypeCheckError::{ArityMisMatch, TypeMismatch}; + +#[test] +fn error_with_duplicate_enum_variant() { + let src = r#" + enum Foo { + Bar(i32), + Bar(u8), + } + + fn main() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 2); + assert!(matches!(&errors[0].0, DefinitionError(Duplicate { .. }))); + assert!(matches!(&errors[1].0, ResolverError(UnusedItem { .. }))); +} + +#[test] +fn errors_on_unspecified_unstable_enum() { + // Enums are experimental - this will need to be updated when they are stabilized + let src = r#" + enum Foo { Bar } + + fn main() { + let _x = Foo::Bar; + } + "#; + + let no_features = &[]; + let errors = get_program_using_features(src, no_features).2; + assert_eq!(errors.len(), 1); + + let CompilationError::ParseError(error) = &errors[0].0 else { + panic!("Expected a ParseError experimental feature error"); + }; + + assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))); +} + +#[test] +fn errors_on_unspecified_unstable_match() { + // Enums are experimental - this will need to be updated when they are stabilized + let src = r#" + fn main() { + match 3 { + _ => (), + } + } + "#; + + let no_features = &[]; + let errors = get_program_using_features(src, no_features).2; + assert_eq!(errors.len(), 1); + + let CompilationError::ParseError(error) = &errors[0].0 else { + panic!("Expected a ParseError experimental feature error"); + }; + + 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, 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, 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, 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, ResolverError(NoSuchField { .. }))); +} + +#[test] +fn match_integer_type_mismatch_in_pattern() { + let src = r#" + fn main() { + match 2 { + Foo::One(_) => (), + } + } + + enum Foo { + One(i32), + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!(&errors[0].0, TypeError(TypeMismatch { .. }))); +} + +#[test] +fn match_shadow_global() { + let src = r#" + fn main() { + match 2 { + foo => assert_eq(foo, 2), + } + } + + fn foo() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 0); +} + +#[test] +fn match_no_shadow_global() { + let src = r#" + fn main() { + match 2 { + crate::foo => (), + } + } + + fn foo() {} + "#; + let errors = dbg!(get_program_errors(src)); + assert_eq!(errors.len(), 1); + + assert!(matches!(&errors[0].0, ResolverError(UnexpectedItemInPattern { .. }))); +} + +#[test] +fn constructor_arg_arity_mismatch_in_pattern() { + let src = r#" + fn main() { + match Foo::One(1) { + Foo::One(_, _) => (), // too many + Foo::Two(_) => (), // too few + } + } + + enum Foo { + One(i32), + Two(i32, i32), + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 2); + + assert!(matches!(&errors[0].0, TypeError(ArityMisMatch { .. }))); + assert!(matches!(&errors[1].0, TypeError(ArityMisMatch { .. }))); +} From 97d338ec682579de3d5411d9ae9d766417ab2d48 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 26 Feb 2025 11:24:14 -0600 Subject: [PATCH 4/5] Clippy --- compiler/noirc_frontend/src/elaborator/enums.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index f6f6eb359ef..80a58fcf2c3 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -595,7 +595,7 @@ impl Elaborator<'_> { } PathResolutionItem::Method(_type_id, _type_turbofish, func_id) => { // TODO(#7430): Take type_turbofish into account when instantiating the function's type - let meta = self.interner.function_meta(&func_id); + let meta = self.interner.function_meta(func_id); let Some(variant_index) = meta.enum_variant_index else { let item = resolution.description(); self.push_err(ResolverError::UnexpectedItemInPattern { location, item }); From 616757516b3d23207f111f2ae913f9bc426486e8 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 26 Feb 2025 11:36:04 -0600 Subject: [PATCH 5/5] Fix merge --- compiler/noirc_frontend/src/tests/enums.rs | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/tests/enums.rs b/compiler/noirc_frontend/src/tests/enums.rs index 9d6fb708351..5a2480d67ae 100644 --- a/compiler/noirc_frontend/src/tests/enums.rs +++ b/compiler/noirc_frontend/src/tests/enums.rs @@ -24,8 +24,8 @@ fn error_with_duplicate_enum_variant() { "#; let errors = get_program_errors(src); assert_eq!(errors.len(), 2); - assert!(matches!(&errors[0].0, DefinitionError(Duplicate { .. }))); - assert!(matches!(&errors[1].0, ResolverError(UnusedItem { .. }))); + assert!(matches!(&errors[0], DefinitionError(Duplicate { .. }))); + assert!(matches!(&errors[1], ResolverError(UnusedItem { .. }))); } #[test] @@ -43,7 +43,7 @@ fn errors_on_unspecified_unstable_enum() { let errors = get_program_using_features(src, no_features).2; assert_eq!(errors.len(), 1); - let CompilationError::ParseError(error) = &errors[0].0 else { + let CompilationError::ParseError(error) = &errors[0] else { panic!("Expected a ParseError experimental feature error"); }; @@ -65,7 +65,7 @@ fn errors_on_unspecified_unstable_match() { let errors = get_program_using_features(src, no_features).2; assert_eq!(errors.len(), 1); - let CompilationError::ParseError(error) = &errors[0].0 else { + let CompilationError::ParseError(error) = &errors[0] else { panic!("Expected a ParseError experimental feature error"); }; @@ -85,7 +85,7 @@ fn errors_on_repeated_match_variables_in_pattern() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - assert!(matches!(&errors[0].0, ResolverError(VariableAlreadyDefinedInPattern { .. }))); + assert!(matches!(&errors[0], ResolverError(VariableAlreadyDefinedInPattern { .. }))); } #[test] @@ -107,7 +107,7 @@ fn duplicate_field_in_match_struct_pattern() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - assert!(matches!(&errors[0].0, ResolverError(DuplicateField { .. }))); + assert!(matches!(&errors[0], ResolverError(DuplicateField { .. }))); } #[test] @@ -129,7 +129,7 @@ fn missing_field_in_match_struct_pattern() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - assert!(matches!(&errors[0].0, ResolverError(MissingFields { .. }))); + assert!(matches!(&errors[0], ResolverError(MissingFields { .. }))); } #[test] @@ -151,7 +151,7 @@ fn no_such_field_in_match_struct_pattern() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - assert!(matches!(&errors[0].0, ResolverError(NoSuchField { .. }))); + assert!(matches!(&errors[0], ResolverError(NoSuchField { .. }))); } #[test] @@ -170,7 +170,7 @@ fn match_integer_type_mismatch_in_pattern() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - assert!(matches!(&errors[0].0, TypeError(TypeMismatch { .. }))); + assert!(matches!(&errors[0], TypeError(TypeMismatch { .. }))); } #[test] @@ -202,7 +202,7 @@ fn match_no_shadow_global() { let errors = dbg!(get_program_errors(src)); assert_eq!(errors.len(), 1); - assert!(matches!(&errors[0].0, ResolverError(UnexpectedItemInPattern { .. }))); + assert!(matches!(&errors[0], ResolverError(UnexpectedItemInPattern { .. }))); } #[test] @@ -223,6 +223,6 @@ fn constructor_arg_arity_mismatch_in_pattern() { let errors = get_program_errors(src); assert_eq!(errors.len(), 2); - assert!(matches!(&errors[0].0, TypeError(ArityMisMatch { .. }))); - assert!(matches!(&errors[1].0, TypeError(ArityMisMatch { .. }))); + assert!(matches!(&errors[0], TypeError(ArityMisMatch { .. }))); + assert!(matches!(&errors[1], TypeError(ArityMisMatch { .. }))); }