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: Add storage slots to globals #1019

Merged
merged 4 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 3 additions & 6 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ impl Driver {
) -> Result<CompiledContract, ReportedError> {
let functions = try_btree_map(&contract.functions, |function| {
let function_name = self.function_name(*function).to_owned();

self.compile_no_check(options, *function).map(|program| (function_name, program))
let program = self.compile_no_check(options, *function)?;
Ok((function_name, program))
})?;

Ok(CompiledContract { name: contract.name, functions })
Expand All @@ -225,10 +225,7 @@ impl Driver {
};

// All Binaries should have a main function
match local_crate.main_function() {
Some(func_id) => Ok(func_id),
None => Err(ReportedError),
}
local_crate.main_function().ok_or(ReportedError)
}

/// Compile the current crate. Assumes self.check_crate is called beforehand!
Expand Down
18 changes: 8 additions & 10 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,10 @@ fn resolve_globals(
globals: Vec<UnresolvedGlobal>,
crate_id: CrateId,
) -> Vec<(FileId, StmtId)> {
let mut global_ids = Vec::new();

for global in globals {
let path_resolver =
StandardPathResolver::new(ModuleId { local_id: global.module_id, krate: crate_id });
vecmap(globals, |global| {
let module_id = ModuleId { local_id: global.module_id, krate: crate_id };
let path_resolver = StandardPathResolver::new(module_id);
let storage_slot = context.next_storage_slot(module_id);

let mut resolver = Resolver::new(
&mut context.def_interner,
Expand All @@ -273,15 +272,14 @@ fn resolve_globals(

let name = global.stmt_def.pattern.name_ident().clone();

let hir_stmt = resolver.resolve_global_let(global.stmt_def);
let hir_stmt = resolver.resolve_global_let(global.stmt_def, storage_slot);

context.def_interner.update_global(global.stmt_id, hir_stmt);

context.def_interner.push_global(global.stmt_id, name.clone(), global.module_id);
context.def_interner.push_global(global.stmt_id, name, global.module_id);

global_ids.push((global.file_id, global.stmt_id));
}
global_ids
(global.file_id, global.stmt_id)
})
}

fn type_check_globals(
Expand Down
24 changes: 24 additions & 0 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,27 @@ pub struct Context {
pub crate_graph: CrateGraph,
pub(crate) def_maps: HashMap<CrateId, CrateDefMap>,
pub file_manager: FileManager,

/// Maps a given (contract) module id to the next available storage slot
/// for that contract.
pub storage_slots: HashMap<def_map::ModuleId, StorageSlot>,
}

pub type StorageSlot = u32;

impl Context {
pub fn new(file_manager: FileManager, crate_graph: CrateGraph, language: Language) -> Context {
let mut ctx = Context {
def_interner: NodeInterner::default(),
def_maps: HashMap::new(),
crate_graph,
file_manager,
storage_slots: HashMap::new(),
};
ctx.def_interner.set_language(&language);
ctx
}

/// Returns the CrateDefMap for a given CrateId.
/// It is perfectly valid for the compiler to look
/// up a CrateDefMap and it is not available.
Expand All @@ -46,4 +54,20 @@ impl Context {
pub fn crates(&self) -> impl Iterator<Item = CrateId> + '_ {
self.crate_graph.iter_keys()
}

fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData {
&self.def_maps[&module_id.krate].modules()[module_id.local_id.0]
}

/// Returns the next available storage slot in the given module.
/// Returns None if the given module is not a contract module.
fn next_storage_slot(&mut self, module_id: def_map::ModuleId) -> Option<StorageSlot> {
let module = self.module(module_id);

module.is_contract.then(|| {
let next_slot = self.storage_slots.entry(module_id).or_insert(0);
*next_slot += 1;
*next_slot
})
}
}
12 changes: 9 additions & 3 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::hir::StorageSlot;
// Fix usage of intern and resolve
// In some places, we do intern, however in others we are resolving and interning
// Ideally, I want to separate the interning and resolving abstractly
Expand Down Expand Up @@ -567,7 +568,7 @@ impl<'a> Resolver<'a> {
for (stmt_id, global_info) in self.interner.get_all_globals() {
if global_info.local_id == self.path_resolver.local_module_id() {
let global_stmt = self.interner.let_statement(&stmt_id);
let definition = DefinitionKind::Global(global_stmt.expression);
let definition = DefinitionKind::Global(global_stmt.expression, None); // TODO
jfecher marked this conversation as resolved.
Show resolved Hide resolved
self.add_global_variable_decl(global_info.ident, definition);
}
}
Expand Down Expand Up @@ -722,9 +723,14 @@ impl<'a> Resolver<'a> {
}
}

pub fn resolve_global_let(&mut self, let_stmt: crate::LetStatement) -> HirStatement {
pub fn resolve_global_let(
&mut self,
let_stmt: crate::LetStatement,
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
storage_slot: Option<StorageSlot>,
) -> HirStatement {
let expression = self.resolve_expression(let_stmt.expression);
let definition = DefinitionKind::Global(expression);
let definition = DefinitionKind::Global(expression, storage_slot);

HirStatement::Let(HirLetStatement {
pattern: self.resolve_pattern(let_stmt.pattern, definition),
r#type: self.resolve_type(let_stmt.r#type),
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ impl<'interner> Monomorphizer<'interner> {
let ident = ast::Ident { location, mutable, definition, name, typ };
ast::Expression::Ident(ident)
}
DefinitionKind::Global(expr_id) => self.expr_infer(*expr_id),
DefinitionKind::Global(expr_id, _slot) => self.expr_infer(*expr_id),
DefinitionKind::Local(_) => {
let ident = self.local_ident(&ident).unwrap();
ast::Expression::Ident(ident)
Expand Down
7 changes: 4 additions & 3 deletions crates/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::graph::CrateId;
use crate::hir::def_collector::dc_crate::UnresolvedStruct;
use crate::hir::def_map::{LocalModuleId, ModuleId};
use crate::hir::type_check::TypeCheckError;
use crate::hir::StorageSlot;
use crate::hir_def::stmt::HirLetStatement;
use crate::hir_def::types::{StructType, Type};
use crate::hir_def::{
Expand Down Expand Up @@ -202,7 +203,7 @@ impl DefinitionInfo {
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum DefinitionKind {
Function(FuncId),
Global(ExprId),
Global(ExprId, Option<StorageSlot>),
jfecher marked this conversation as resolved.
Show resolved Hide resolved

/// Locals may be defined in let statements or parameters,
/// in which case they will not have an associated ExprId
Expand All @@ -217,13 +218,13 @@ impl DefinitionKind {
/// True if this definition is for a global variable.
/// Note that this returns false for top-level functions.
pub fn is_global(&self) -> bool {
matches!(self, DefinitionKind::Global(_))
matches!(self, DefinitionKind::Global(..))
}

pub fn get_rhs(self) -> Option<ExprId> {
match self {
DefinitionKind::Function(_) => None,
DefinitionKind::Global(id) => Some(id),
DefinitionKind::Global(id, ..) => Some(id),
DefinitionKind::Local(id) => id,
DefinitionKind::GenericType(_) => None,
}
Expand Down