feat: Allow structs and arrays as globals (#1054)

* Parse literals or constructors for globals

* Allow structs and arrays as globals

* Fix global resolution order

crates/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -12,8 +12,8 @@ use crate::hir::type_check::{type_check_func, TypeChecker};
 use crate::hir::Context;
 use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId};
 use crate::{
-    Generics, Ident, LetStatement, NoirFunction, NoirStruct, ParsedModule, Shared, Type,
-    TypeBinding, UnresolvedGenerics, UnresolvedType,
+    ExpressionKind, Generics, Ident, LetStatement, NoirFunction, NoirStruct, ParsedModule, Shared,
+    Type, TypeBinding, UnresolvedGenerics, UnresolvedType,
 };
 use fm::FileId;
 use iter_extended::vecmap;
@@ -150,10 +150,23 @@ impl DefCollector {
 
         // We must first resolve and intern the globals before we can resolve any stmts inside each function.
         // Each function uses its own resolver with a newly created ScopeForest, and must be resolved again to be within a function's scope
-        let file_global_ids = resolve_globals(context, def_collector.collected_globals, crate_id);
+        //
+        // Additionally, we must resolve integer globals before structs since structs may refer to
+        // the values of integer globals as numeric generics.
+        let (integer_globals, other_globals) =
+            filter_integer_globals(def_collector.collected_globals);
+
+        let mut file_global_ids = resolve_globals(context, integer_globals, crate_id, errors);
 
+        // Must resolve structs before we resolve globals.
         resolve_structs(context, def_collector.collected_types, crate_id, errors);
 
+        // We must wait to resolve non-integer globals until after we resolve structs since structs
+        // globals will need to reference the struct type they're initialized to to ensure they are valid.
+        let mut more_global_ids = resolve_globals(context, other_globals, crate_id, errors);
+
+        file_global_ids.append(&mut more_global_ids);
+
         // Before we resolve any function symbols we must go through our impls and
         // re-collect the methods within into their proper module. This cannot be
         // done before resolution since we need to be able to resolve the type of the
@@ -179,6 +192,7 @@ impl DefCollector {
         );
 
         type_check_globals(&mut context.def_interner, file_global_ids, errors);
+
         // Type check all of the functions in the crate
         type_check_functions(&mut context.def_interner, file_func_ids, errors);
         type_check_functions(&mut context.def_interner, file_method_ids, errors);
@@ -252,10 +266,21 @@ where
     errors.extend(new_errors.into_iter().map(|err| err.into().in_file(file)))
 }
 
+/// Separate the globals Vec into two. The first element in the tuple will be the
+/// integer literal globals, and the second will be all other globals.
+fn filter_integer_globals(
+    globals: Vec<UnresolvedGlobal>,
+) -> (Vec<UnresolvedGlobal>, Vec<UnresolvedGlobal>) {
+    globals
+        .into_iter()
+        .partition(|global| matches!(&global.stmt_def.expression.kind, ExpressionKind::Literal(_)))
+}
+
 fn resolve_globals(
     context: &mut Context,
     globals: Vec<UnresolvedGlobal>,
     crate_id: CrateId,
+    errors: &mut Vec<FileDiagnostic>,
 ) -> Vec<(FileId, StmtId)> {
     vecmap(globals, |global| {
         let module_id = ModuleId { local_id: global.module_id, krate: crate_id };
@@ -272,6 +297,7 @@ fn resolve_globals(
         let name = global.stmt_def.pattern.name_ident().clone();
 
         let hir_stmt = resolver.resolve_global_let(global.stmt_def);
+        extend_errors(errors, global.file_id, resolver.take_errors());
 
         context.def_interner.update_global(global.stmt_id, hir_stmt);
 

crates/noirc_frontend/src/hir/resolution/resolver.rs

@@ -839,6 +839,14 @@ impl<'a> Resolver<'a> {
                 Literal::Integer(integer) => HirLiteral::Integer(integer),
                 Literal::Str(str) => HirLiteral::Str(str),
             }),
+            ExpressionKind::Variable(path) => {
+                // If the Path is being used as an Expression, then it is referring to a global from a separate module
+                // Otherwise, then it is referring to an Identifier
+                // This lookup allows support of such statements: let x = foo::bar::SOME_GLOBAL + 10;
+                // If the expression is a singular indent, we search the resolver's current scope as normal.
+                let hir_ident = self.get_ident_from_path(path);
+                HirExpression::Ident(hir_ident)
+            }
             ExpressionKind::Prefix(prefix) => {
                 let operator = prefix.operator;
                 let rhs = self.resolve_expression(prefix.rhs);
@@ -910,14 +918,6 @@ impl<'a> Resolver<'a> {
                 collection: self.resolve_expression(indexed_expr.collection),
                 index: self.resolve_expression(indexed_expr.index),
             }),
-            ExpressionKind::Variable(path) => {
-                // If the Path is being used as an Expression, then it is referring to a global from a separate module
-                // Otherwise, then it is referring to an Identifier
-                // This lookup allows support of such statements: let x = foo::bar::SOME_GLOBAL + 10;
-                // If the expression is a singular indent, we search the resolver's current scope as normal.
-                let hir_ident = self.get_ident_from_path(path);
-                HirExpression::Ident(hir_ident)
-            }
             ExpressionKind::Block(block_expr) => self.resolve_block(block_expr),
             ExpressionKind::Constructor(constructor) => {
                 let span = constructor.type_name.span();
@@ -1199,7 +1199,8 @@ impl<'a> Resolver<'a> {
         let stmt = match self.interner.statement(&global) {
             HirStatement::Let(let_expr) => let_expr,
             other => {
-                unreachable!("Expected global while evaluating array length, found {:?}", other)
+                dbg!(other);
+                return 0;
             }
         };
 

crates/noirc_frontend/src/monomorphization/mod.rs

@@ -341,7 +341,10 @@ impl<'interner> Monomorphizer<'interner> {
 
             HirExpression::Lambda(lambda) => self.lambda(lambda),
 
-            HirExpression::MethodCall(_) | HirExpression::Error => unreachable!(),
+            HirExpression::MethodCall(_) => {
+                unreachable!("Encountered HirExpression::MethodCall during monomorphization")
+            }
+            HirExpression::Error => unreachable!("Encountered Error node during monomorphization"),
         }
     }
 

crates/noirc_frontend/src/parser/parser.rs

@@ -118,7 +118,7 @@ fn global_declaration() -> impl NoirParser<TopLevelStatement> {
     );
     let p = then_commit(p, global_type_annotation());
     let p = then_commit_ignore(p, just(Token::Assign));
-    let p = then_commit(p, literal().map_with_span(Expression::new)); // XXX: this should be a literal
+    let p = then_commit(p, literal_or_collection(expression()).map_with_span(Expression::new));
     p.map(LetStatement::new_let).map(TopLevelStatement::Global)
 }
 
@@ -942,10 +942,7 @@ fn field_name() -> impl NoirParser<Ident> {
     }))
 }
 
-fn constructor<P>(expr_parser: P) -> impl NoirParser<ExpressionKind>
-where
-    P: ExprParser,
-{
+fn constructor(expr_parser: impl ExprParser) -> impl NoirParser<ExpressionKind> {
     let args = constructor_field(expr_parser)
         .separated_by(just(Token::Comma))
         .at_least(1)
@@ -977,6 +974,10 @@ fn literal() -> impl NoirParser<ExpressionKind> {
     })
 }
 
+fn literal_or_collection(expr_parser: impl ExprParser) -> impl NoirParser<ExpressionKind> {
+    choice((literal(), constructor(expr_parser.clone()), array_expr(expr_parser)))
+}
+
 #[cfg(test)]
 mod test {
     use noirc_errors::CustomDiagnostic;