Merge branch 'master' into ab/no-duplicate-error

* master:
  feat(experimental): Support struct constructors in match patterns (#7489)
  feat: use resolved type instead of needing Constructor.struct_type (#7500)
  feat: better error message when keyword is found instead of type in p… (#7501)
  chore: bump external pinned commits (#7497)
  feat(experimental): Add invalid pattern syntax error (#7487)
  fix(performance): Accurately mark safe constant indices for arrays of complex types (#7491)

.github/benchmark_projects.yml

@@ -1,4 +1,4 @@
-define: &AZ_COMMIT 481d3beaba9fcc85743919bc640a849dad7b7b88
+define: &AZ_COMMIT f7a8f6be5c4b9c9aa9f59b4b99104b30b7bbff4c
 projects:
   private-kernel-inner:
     repo: AztecProtocol/aztec-packages

EXTERNAL_NOIR_LIBRARIES.yml

@@ -1,4 +1,4 @@
-define: &AZ_COMMIT 481d3beaba9fcc85743919bc640a849dad7b7b88
+define: &AZ_COMMIT f7a8f6be5c4b9c9aa9f59b4b99104b30b7bbff4c
 libraries:
   noir_check_shuffle:
     repo: noir-lang/noir_check_shuffle

compiler/noirc_evaluator/src/ssa/ir/dfg.rs

@@ -658,7 +658,11 @@ impl DataFlowGraph {
     pub(crate) fn is_safe_index(&self, index: ValueId, array: ValueId) -> bool {
         #[allow(clippy::match_like_matches_macro)]
         match (self.type_of_value(array), self.get_numeric_constant(index)) {
-            (Type::Array(_, len), Some(index)) if index.to_u128() < (len as u128) => true,
+            (Type::Array(elements, len), Some(index))
+                if index.to_u128() < (len as u128 * elements.len() as u128) =>
+            {
+                true
+            }
             _ => false,
         }
     }

compiler/noirc_frontend/src/ast/expression.rs

@@ -7,9 +7,7 @@ use crate::ast::{
     Ident, ItemVisibility, Path, Pattern, Statement, StatementKind, UnresolvedTraitConstraint,
     UnresolvedType, UnresolvedTypeData, Visibility,
 };
-use crate::node_interner::{
-    ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId, TypeId,
-};
+use crate::node_interner::{ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId};
 use crate::token::{Attributes, FmtStrFragment, FunctionAttribute, Token, Tokens};
 use crate::{Kind, Type};
 use acvm::{acir::AcirField, FieldElement};
@@ -223,11 +221,7 @@ impl ExpressionKind {
     pub fn constructor(
         (typ, fields): (UnresolvedType, Vec<(Ident, Expression)>),
     ) -> ExpressionKind {
-        ExpressionKind::Constructor(Box::new(ConstructorExpression {
-            typ,
-            fields,
-            struct_type: None,
-        }))
+        ExpressionKind::Constructor(Box::new(ConstructorExpression { typ, fields }))
     }
 }
 
@@ -545,11 +539,6 @@ pub struct MethodCallExpression {
 pub struct ConstructorExpression {
     pub typ: UnresolvedType,
     pub fields: Vec<(Ident, Expression)>,
-
-    /// This may be filled out during macro expansion
-    /// so that we can skip re-resolving the type name since it
-    /// would be lost at that point.
-    pub struct_type: Option<TypeId>,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]

compiler/noirc_frontend/src/ast/statement.rs

@@ -623,7 +623,6 @@ impl Pattern {
                     kind: ExpressionKind::Constructor(Box::new(ConstructorExpression {
                         typ: UnresolvedType::from_path(path.clone()),
                         fields,
-                        struct_type: None,
                     })),
                     location: *location,
                 })

compiler/noirc_frontend/src/elaborator/enums.rs

@@ -1,12 +1,14 @@
+use std::collections::BTreeMap;
+
 use fm::FileId;
 use fxhash::FxHashMap as HashMap;
 use iter_extended::{try_vecmap, vecmap};
 use noirc_errors::Location;
 
 use crate::{
     ast::{
-        EnumVariant, Expression, ExpressionKind, FunctionKind, Ident, 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},
@@ -311,6 +313,14 @@ 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);
+            Pattern::Error
+        };
+
         match expression.kind {
             ExpressionKind::Literal(Literal::Integer(value, negative)) => {
                 let actual = self.interner.next_type_variable_with_kind(Kind::IntegerOrField);
@@ -348,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());
                         }
@@ -363,10 +376,7 @@ impl Elaborator<'_> {
                     }
                     Err(error) => {
                         self.push_err(error, location.file);
-                        // Default to defining a variable of the same name although this could
-                        // cause further match warnings/errors (e.g. redundant cases).
-                        let id = self.fresh_match_variable(expected_type.clone(), location);
-                        Pattern::Binding(id)
+                        Pattern::Error
                     }
                 }
             }
@@ -376,7 +386,9 @@ impl Elaborator<'_> {
                 expected_type,
                 variables_defined,
             ),
-            ExpressionKind::Constructor(_) => todo!("handle constructors"),
+            ExpressionKind::Constructor(constructor) => {
+                self.constructor_to_pattern(*constructor, variables_defined)
+            }
             ExpressionKind::Tuple(fields) => {
                 let field_types = vecmap(0..fields.len(), |_| self.interner.next_type_variable());
                 let actual = Type::Tuple(field_types.clone());
@@ -402,7 +414,7 @@ impl Elaborator<'_> {
                 if let StatementKind::Expression(expr) = self.interner.get_statement_kind(id) {
                     self.expression_to_pattern(expr.clone(), expected_type, variables_defined)
                 } else {
-                    panic!("Invalid expr kind {expression}")
+                    syntax_error(self)
                 }
             }
 
@@ -425,12 +437,57 @@ impl Elaborator<'_> {
             | ExpressionKind::AsTraitPath(_)
             | ExpressionKind::TypePath(_)
             | ExpressionKind::Resolved(_)
-            | ExpressionKind::Error => {
-                panic!("Invalid expr kind {expression}")
-            }
+            | ExpressionKind::Error => syntax_error(self),
         }
     }
 
+    fn constructor_to_pattern(
+        &mut self,
+        constructor: ConstructorExpression,
+        variables_defined: &mut Vec<Ident>,
+    ) -> Pattern {
+        let location = constructor.typ.location;
+        let typ = 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);
+            let pattern =
+                self.expression_to_pattern(field, &expected_field_type, variables_defined);
+            fields.insert(field_name, pattern);
+        }
+
+        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,
@@ -552,6 +609,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
@@ -703,6 +777,7 @@ 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;
@@ -931,6 +1006,11 @@ enum Pattern {
     /// 1 <= n < 20.
     #[allow(unused)]
     Range(SignedField, SignedField),
+
+    /// An error occurred while translating this pattern. This Pattern kind always translates
+    /// to a Fail branch in the decision tree, although the compiler is expected to halt
+    /// with errors before execution.
+    Error,
 }
 
 #[derive(Clone)]

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -761,15 +761,8 @@ impl<'context> Elaborator<'context> {
 
         let last_segment = path.last_segment();
 
-        let typ = if let Some(struct_id) = constructor.struct_type {
-            let typ = self.interner.get_type(struct_id);
-            let generics = typ.borrow().instantiate(self.interner);
-            Type::DataType(typ, generics)
-        } else {
-            match self.lookup_type_or_error(path) {
-                Some(typ) => typ,
-                None => return (HirExpression::Error, Type::Error),
-            }
+        let Some(typ) = self.lookup_type_or_error(path) else {
+            return (HirExpression::Error, Type::Error);
         };
 
         self.elaborate_constructor_with_type(typ, constructor.fields, location, Some(last_segment))

compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs

@@ -129,12 +129,10 @@ impl HirExpression {
                 let fields = vecmap(constructor.fields.clone(), |(name, expr): (Ident, ExprId)| {
                     (name, expr.to_display_ast(interner))
                 });
-                let struct_type = None;
 
                 ExpressionKind::Constructor(Box::new(ConstructorExpression {
                     typ: UnresolvedType::from_path(type_name),
                     fields,
-                    struct_type,
                 }))
             }
             HirExpression::MemberAccess(access) => {

compiler/noirc_frontend/src/hir/comptime/value.rs

@@ -9,7 +9,7 @@ use strum_macros::Display;
 use crate::{
     ast::{
         ArrayLiteral, BlockExpression, ConstructorExpression, Expression, ExpressionKind, Ident,
-        IntegerBitSize, LValue, Literal, Path, Pattern, Signedness, Statement, StatementKind,
+        IntegerBitSize, LValue, Literal, Pattern, Signedness, Statement, StatementKind,
         UnresolvedType, UnresolvedTypeData,
     },
     elaborator::Elaborator,
@@ -249,18 +249,18 @@ impl Value {
                     Ok((Ident::new(unwrap_rc(name), location), field))
                 })?;
 
-                let struct_type = match typ.follow_bindings_shallow().as_ref() {
-                    Type::DataType(def, _) => Some(def.borrow().id),
+                let typ = match typ.follow_bindings_shallow().as_ref() {
+                    Type::DataType(data_type, generics) => {
+                        Type::DataType(data_type.clone(), generics.clone())
+                    }
                     _ => return Err(InterpreterError::NonStructInConstructor { typ, location }),
                 };
 
-                // Since we've provided the struct_type, the path should be ignored.
-                let type_name = Path::from_single(String::new(), location);
-                ExpressionKind::Constructor(Box::new(ConstructorExpression {
-                    typ: UnresolvedType::from_path(type_name),
-                    fields,
-                    struct_type,
-                }))
+                let quoted_type_id = elaborator.interner.push_quoted_type(typ);
+
+                let typ = UnresolvedTypeData::Resolved(quoted_type_id);
+                let typ = UnresolvedType { typ, location };
+                ExpressionKind::Constructor(Box::new(ConstructorExpression { typ, fields }))
             }
             value @ Value::Enum(..) => {
                 let hir = value.into_hir_expression(elaborator.interner, location)?;

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("Invalid syntax in match pattern")]
+    InvalidSyntaxInPattern { span: Span },
     #[error("Variable '{existing}' was already defined in the same match pattern")]
     VariableAlreadyDefinedInPattern { existing: Ident, new_span: Span },
 }
@@ -694,6 +696,12 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                     String::new(),
                     *span)
             }
+            ResolverError::InvalidSyntaxInPattern { span } => {
+                Diagnostic::simple_error(
+                    "Invalid syntax in match pattern".into(), 
+                    "Only literal, constructor, and variable patterns are allowed".into(),
+                    *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");

compiler/noirc_frontend/src/parser/parser/expression.rs

@@ -440,11 +440,7 @@ impl<'a> Parser<'a> {
             Self::parse_constructor_field,
         );
 
-        ExpressionKind::Constructor(Box::new(ConstructorExpression {
-            typ,
-            fields,
-            struct_type: None,
-        }))
+        ExpressionKind::Constructor(Box::new(ConstructorExpression { typ, fields }))
     }
 
     fn parse_constructor_field(&mut self) -> Option<(Ident, Expression)> {