Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow structs and arrays as globals #1054

Merged
merged 4 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -151,10 +151,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
Expand All @@ -180,6 +193,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);
Expand Down Expand Up @@ -253,10 +267,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 };
Expand All @@ -273,6 +298,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);

Expand Down
19 changes: 10 additions & 9 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,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);
Expand Down Expand Up @@ -900,14 +908,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();
Expand Down Expand Up @@ -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;
}
};

Expand Down
5 changes: 4 additions & 1 deletion crates/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}

Expand Down
11 changes: 6 additions & 5 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down