From 864a91dc3b53cf1c324256bdb95728ef8caf50a2 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 28 Mar 2023 13:10:43 +0100 Subject: [PATCH 1/3] Parse literals or constructors for globals --- .../src/hir/resolution/resolver.rs | 16 ++++++++-------- crates/noirc_frontend/src/parser/parser.rs | 11 ++++++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 3ed70034f03..2cf00d42e68 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -826,6 +826,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); @@ -897,14 +905,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(); diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 4a57c6145ba..b40e6811bcb 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -119,7 +119,7 @@ fn global_declaration() -> impl NoirParser { ); 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_constructor(expression()).map_with_span(Expression::new)); p.map(LetStatement::new_let).map(TopLevelStatement::Global) } @@ -942,10 +942,7 @@ fn field_name() -> impl NoirParser { })) } -fn constructor

(expr_parser: P) -> impl NoirParser -where - P: ExprParser, -{ +fn constructor(expr_parser: impl ExprParser) -> impl NoirParser { let args = constructor_field(expr_parser) .separated_by(just(Token::Comma)) .at_least(1) @@ -977,6 +974,10 @@ fn literal() -> impl NoirParser { }) } +fn literal_or_constructor(expr_parser: impl ExprParser) -> impl NoirParser { + literal().or(constructor(expr_parser)) +} + #[cfg(test)] mod test { use noirc_errors::CustomDiagnostic; From 0e2de89b454dff7b38be3cbeaa034fd0ee0ddc77 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 28 Mar 2023 17:28:53 +0100 Subject: [PATCH 2/3] Allow structs and arrays as globals --- .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 11 ++++++++--- crates/noirc_frontend/src/monomorphization/mod.rs | 5 ++++- crates/noirc_frontend/src/parser/parser.rs | 6 +++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index 89dbc6b1e7f..ef4960e9135 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -149,11 +149,13 @@ impl DefCollector { } } + // Must resolve structs before we resolve globals. + resolve_structs(context, def_collector.collected_types, crate_id, errors); + // 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); - - resolve_structs(context, def_collector.collected_types, crate_id, errors); + let file_global_ids = + resolve_globals(context, def_collector.collected_globals, crate_id, errors); // 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 @@ -180,6 +182,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); @@ -257,6 +260,7 @@ fn resolve_globals( context: &mut Context, globals: Vec, crate_id: CrateId, + errors: &mut Vec, ) -> Vec<(FileId, StmtId)> { vecmap(globals, |global| { let module_id = ModuleId { local_id: global.module_id, krate: crate_id }; @@ -273,6 +277,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); diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index d852db9ac24..92506ad294d 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/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"), } } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index b40e6811bcb..28f788d1cab 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -119,7 +119,7 @@ fn global_declaration() -> impl NoirParser { ); let p = then_commit(p, global_type_annotation()); let p = then_commit_ignore(p, just(Token::Assign)); - let p = then_commit(p, literal_or_constructor(expression()).map_with_span(Expression::new)); + let p = then_commit(p, literal_or_collection(expression()).map_with_span(Expression::new)); p.map(LetStatement::new_let).map(TopLevelStatement::Global) } @@ -974,8 +974,8 @@ fn literal() -> impl NoirParser { }) } -fn literal_or_constructor(expr_parser: impl ExprParser) -> impl NoirParser { - literal().or(constructor(expr_parser)) +fn literal_or_collection(expr_parser: impl ExprParser) -> impl NoirParser { + choice((literal(), constructor(expr_parser.clone()), array_expr(expr_parser))) } #[cfg(test)] From 464f16a8b7ba47fc859ea53de9fc21a72ed1418f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 28 Mar 2023 18:19:17 +0100 Subject: [PATCH 3/3] Fix global resolution order --- .../src/hir/def_collector/dc_crate.rs | 33 +++++++++++++++---- .../src/hir/resolution/resolver.rs | 3 +- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index ef4960e9135..6a6986160c6 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -13,8 +13,8 @@ use crate::hir::type_check::type_check_func; 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; @@ -149,13 +149,24 @@ 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 + // + // 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 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, 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 @@ -256,6 +267,16 @@ 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, +) -> (Vec, Vec) { + globals + .into_iter() + .partition(|global| matches!(&global.stmt_def.expression.kind, ExpressionKind::Literal(_))) +} + fn resolve_globals( context: &mut Context, globals: Vec, diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index ee147302df8..41f157f5683 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -1189,7 +1189,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; } };