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

fix: Filter comptime globals #5538

Merged
merged 13 commits into from
Jul 25, 2024
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
58 changes: 27 additions & 31 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::mem::replace;

use crate::{
hir_def::expr::HirIdent,
node_interner::{DependencyId, FuncId},
Expand All @@ -11,42 +9,40 @@ impl<'context> Elaborator<'context> {
/// Elaborate an expression from the middle of a comptime scope.
/// When this happens we require additional information to know
/// what variables should be in scope.
pub fn elaborate_item_from_comptime<T>(
&mut self,
pub fn elaborate_item_from_comptime<'a, T>(
&'a mut self,
current_function: Option<FuncId>,
f: impl FnOnce(&mut Self) -> T,
f: impl FnOnce(&mut Elaborator<'a>) -> T,
) -> T {
self.function_context.push(FunctionContext::default());
let old_scope = self.scopes.end_function();
self.scopes.start_function();
let function_id = current_function.map(DependencyId::Function);
let old_item = replace(&mut self.current_item, function_id);
// Create a fresh elaborator to ensure no state is changed from
// this elaborator
let mut elaborator = Elaborator::new(
self.interner,
self.def_maps,
self.crate_id,
self.debug_comptime_in_file,
);

// Note: recover_generics isn't good enough here because any existing generics
// should not be in scope of this new function
let old_generics = std::mem::take(&mut self.generics);
elaborator.function_context.push(FunctionContext::default());
elaborator.scopes.start_function();

let old_crate_and_module = current_function.map(|function| {
let meta = self.interner.function_meta(&function);
let old_crate = replace(&mut self.crate_id, meta.source_crate);
let old_module = replace(&mut self.local_module, meta.source_module);
self.introduce_generics_into_scope(meta.all_generics.clone());
(old_crate, old_module)
});
if let Some(function) = current_function {
let meta = elaborator.interner.function_meta(&function);
elaborator.current_item = Some(DependencyId::Function(function));
elaborator.crate_id = meta.source_crate;
elaborator.local_module = meta.source_module;
elaborator.file = meta.source_file;
elaborator.introduce_generics_into_scope(meta.all_generics.clone());
}

self.populate_scope_from_comptime_scopes();
let result = f(self);
elaborator.comptime_scopes = std::mem::take(&mut self.comptime_scopes);
elaborator.populate_scope_from_comptime_scopes();

if let Some((old_crate, old_module)) = old_crate_and_module {
self.crate_id = old_crate;
self.local_module = old_module;
}
let result = f(&mut elaborator);
elaborator.check_and_pop_function_context();

self.generics = old_generics;
self.current_item = old_item;
self.scopes.end_function();
self.scopes.0.push(old_scope);
self.check_and_pop_function_context();
self.comptime_scopes = elaborator.comptime_scopes;
self.errors.append(&mut elaborator.errors);
result
}

Expand Down
70 changes: 45 additions & 25 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
},
dc_mod,
},
def_map::DefMaps,
resolution::{errors::ResolverError, path_resolver::PathResolver},
scope::ScopeForest as GenericScopeForest,
type_check::TypeCheckError,
Expand Down Expand Up @@ -54,7 +55,7 @@ use crate::{
use crate::{
hir::{
def_collector::dc_crate::{UnresolvedFunctions, UnresolvedTraitImpl},
def_map::{CrateDefMap, ModuleData},
def_map::ModuleData,
},
hir_def::traits::TraitImpl,
macros_api::ItemVisibility,
Expand Down Expand Up @@ -102,7 +103,7 @@ pub struct Elaborator<'context> {

pub(crate) interner: &'context mut NodeInterner,

def_maps: &'context mut BTreeMap<CrateId, CrateDefMap>,
def_maps: &'context mut DefMaps,

file: FileId,

Expand Down Expand Up @@ -130,8 +131,6 @@ pub struct Elaborator<'context> {
/// to the corresponding trait impl ID.
current_trait_impl: Option<TraitImplId>,

trait_id: Option<TraitId>,

/// In-resolution names
///
/// This needs to be a set because we can have multiple in-resolution
Expand Down Expand Up @@ -195,22 +194,22 @@ struct FunctionContext {

impl<'context> Elaborator<'context> {
pub fn new(
context: &'context mut Context,
interner: &'context mut NodeInterner,
def_maps: &'context mut DefMaps,
crate_id: CrateId,
debug_comptime_in_file: Option<FileId>,
) -> Self {
Self {
scopes: ScopeForest::default(),
errors: Vec::new(),
interner: &mut context.def_interner,
def_maps: &mut context.def_maps,
interner,
def_maps,
file: FileId::dummy(),
nested_loops: 0,
generics: Vec::new(),
lambda_stack: Vec::new(),
self_type: None,
current_item: None,
trait_id: None,
local_module: LocalModuleId::dummy_id(),
crate_id,
resolving_ids: BTreeSet::new(),
Expand All @@ -223,6 +222,19 @@ impl<'context> Elaborator<'context> {
}
}

pub fn from_context(
context: &'context mut Context,
crate_id: CrateId,
debug_comptime_in_file: Option<FileId>,
) -> Self {
Self::new(
&mut context.def_interner,
&mut context.def_maps,
crate_id,
debug_comptime_in_file,
)
}

pub fn elaborate(
context: &'context mut Context,
crate_id: CrateId,
Expand All @@ -238,7 +250,7 @@ impl<'context> Elaborator<'context> {
items: CollectedItems,
debug_comptime_in_file: Option<FileId>,
) -> Self {
let mut this = Self::new(context, crate_id, debug_comptime_in_file);
let mut this = Self::from_context(context, crate_id, debug_comptime_in_file);

// Filter out comptime items to execute their functions first if needed.
// This step is why comptime items can only refer to other comptime items
Expand Down Expand Up @@ -337,17 +349,12 @@ impl<'context> Elaborator<'context> {
}

fn elaborate_functions(&mut self, functions: UnresolvedFunctions) {
self.file = functions.file_id;
self.trait_id = functions.trait_id; // TODO: Resolve?
self.self_type = functions.self_type;

for (local_module, id, _) in functions.functions {
self.local_module = local_module;
self.recover_generics(|this| this.elaborate_function(id));
for (_, id, _) in functions.functions {
self.elaborate_function(id);
}

self.generics.clear();
self.self_type = None;
self.trait_id = None;
}

fn introduce_generics_into_scope(&mut self, all_generics: Vec<ResolvedGeneric>) {
Expand All @@ -365,7 +372,7 @@ impl<'context> Elaborator<'context> {
self.generics = all_generics;
}

fn elaborate_function(&mut self, id: FuncId) {
pub(crate) fn elaborate_function(&mut self, id: FuncId) {
let func_meta = self.interner.func_meta.get_mut(&id);
let func_meta =
func_meta.expect("FuncMetas should be declared before a function is elaborated");
Expand All @@ -378,11 +385,21 @@ impl<'context> Elaborator<'context> {
FunctionBody::Resolving => return,
};

let func_meta = func_meta.clone();

assert_eq!(
self.crate_id, func_meta.source_crate,
"Functions in other crates should be already elaborated"
);

self.local_module = func_meta.source_module;
self.file = func_meta.source_file;
self.self_type = func_meta.self_type.clone();
self.current_trait_impl = func_meta.trait_impl;

self.scopes.start_function();
let old_item = std::mem::replace(&mut self.current_item, Some(DependencyId::Function(id)));

let func_meta = func_meta.clone();

self.trait_bounds = func_meta.trait_constraints.clone();
self.function_context.push(FunctionContext::default());

Expand Down Expand Up @@ -775,6 +792,8 @@ impl<'context> Elaborator<'context> {
source_crate: self.crate_id,
source_module: self.local_module,
function_body: FunctionBody::Unresolved(func.kind, body, func.def.span),
self_type: self.self_type.clone(),
source_file: self.file,
};

self.interner.push_fn_meta(meta, func_id);
Expand Down Expand Up @@ -1003,10 +1022,7 @@ impl<'context> Elaborator<'context> {
self.self_type = None;
}

fn get_module_mut(
def_maps: &mut BTreeMap<CrateId, CrateDefMap>,
module: ModuleId,
) -> &mut ModuleData {
fn get_module_mut(def_maps: &mut DefMaps, module: ModuleId) -> &mut ModuleData {
let message = "A crate should always be present for a given crate id";
&mut def_maps.get_mut(&module.krate).expect(message).modules[module.local_id.0]
}
Expand Down Expand Up @@ -1528,19 +1544,23 @@ impl<'context> Elaborator<'context> {
let (comptime_structs, structs) =
items.types.into_iter().partition(|typ| typ.1.struct_def.is_comptime);

let (comptime_globals, globals) =
items.globals.into_iter().partition(|global| global.stmt_def.comptime);

let comptime = CollectedItems {
functions: comptime_function_sets,
types: comptime_structs,
type_aliases: BTreeMap::new(),
traits: BTreeMap::new(),
trait_impls: comptime_trait_impls,
globals: Vec::new(),
globals: comptime_globals,
impls: rustc_hash::FxHashMap::default(),
};

items.functions = function_sets;
items.trait_impls = trait_impls;
items.types = structs;
items.globals = globals;
(comptime, items)
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,8 @@ impl<'context> Elaborator<'context> {
&mut self,
path: &Path,
) -> Option<(TraitMethodId, TraitConstraint, bool)> {
let trait_id = self.trait_id?;
let trait_impl = self.current_trait_impl?;
let trait_id = self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id;

if path.kind == PathKind::Plain && path.segments.len() == 2 {
let name = &path.segments[0].0.contents;
Expand Down
32 changes: 25 additions & 7 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::ast::{BinaryOpKind, FunctionKind, IntegerBitSize, Signedness};
use crate::elaborator::Elaborator;
use crate::graph::CrateId;
use crate::hir_def::expr::ImplKind;
use crate::hir_def::function::FunctionBody;
use crate::macros_api::UnaryOp;
use crate::monomorphization::{
perform_impl_bindings, perform_instantiation_bindings, resolve_trait_method,
Expand Down Expand Up @@ -135,18 +136,35 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
self.define_pattern(parameter, typ, argument, arg_location)?;
}

let function_body =
self.elaborator.interner.function(&function).try_as_expr().ok_or_else(|| {
let function = self.elaborator.interner.function_name(&function).to_owned();
InterpreterError::NonComptimeFnCallInSameCrate { function, location }
})?;

let function_body = self.get_function_body(function, location)?;
let result = self.evaluate(function_body)?;

self.exit_function(previous_state);
Ok(result)
}

/// Try to retrieve a function's body.
/// If the function has not yet been resolved this will attempt to lazily resolve it.
/// Afterwards, if the function's body is still not known or the function is still
/// in a Resolving state we issue an error.
fn get_function_body(&mut self, function: FuncId, location: Location) -> IResult<ExprId> {
let meta = self.elaborator.interner.function_meta(&function);
match self.elaborator.interner.function(&function).try_as_expr() {
Some(body) => Ok(body),
None => {
if matches!(&meta.function_body, FunctionBody::Unresolved(..)) {
self.elaborator.elaborate_item_from_comptime(None, |elaborator| {
elaborator.elaborate_function(function);
});

self.get_function_body(function, location)
} else {
let function = self.elaborator.interner.function_name(&function).to_owned();
Err(InterpreterError::NonComptimeFnCallInSameCrate { function, location })
}
}
}
}

fn call_special(
&mut self,
function: FuncId,
Expand Down
4 changes: 3 additions & 1 deletion compiler/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ impl ModuleId {
}

impl ModuleId {
pub fn module(self, def_maps: &BTreeMap<CrateId, CrateDefMap>) -> &ModuleData {
pub fn module(self, def_maps: &DefMaps) -> &ModuleData {
&def_maps[&self.krate].modules()[self.local_id.0]
}
}

pub type DefMaps = BTreeMap<CrateId, CrateDefMap>;

/// Map of all modules and scopes defined within a crate.
///
/// The definitions of the crate are accessible indirectly via the scopes of each module.
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use fm::FileId;
use iter_extended::vecmap;
use noirc_errors::{Location, Span};

Expand Down Expand Up @@ -156,6 +157,13 @@ pub struct FuncMeta {

/// The module this function was defined in
pub source_module: LocalModuleId,

/// THe file this function was defined in
pub source_file: FileId,

/// If this function is from an impl (trait or regular impl), this
/// is the object type of the impl. Otherwise this is None.
pub self_type: Option<Type>,
}

#[derive(Debug, Clone)]
Expand Down
Loading