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: Remove 'comptime or separate crate' restriction on comptime code #5609

Merged
merged 9 commits into from
Jul 26, 2024
1 change: 0 additions & 1 deletion aztec_macros/src/transforms/note_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ pub fn generate_note_interface_impl(module: &mut SortedModule) -> Result<(), Azt
generics: vec![],
methods: vec![],
where_clause: vec![],
is_comptime: false,
};
module.impls.push(default_impl.clone());
module.impls.last_mut().unwrap()
Expand Down
1 change: 0 additions & 1 deletion aztec_macros/src/transforms/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@
///
/// To:
///
/// impl<Context> Storage<Contex> {

Check warning on line 174 in aztec_macros/src/transforms/storage.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Contex)
/// fn init(context: Context) -> Self {
/// Storage {
/// a_map: Map::new(context, 0, |context, slot| {
Expand Down Expand Up @@ -246,7 +246,6 @@
methods: vec![(init, Span::default())],

where_clause: vec![],
is_comptime: false,
};
module.impls.push(storage_impl);

Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_frontend/src/ast/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ pub struct NoirStruct {
pub generics: UnresolvedGenerics,
pub fields: Vec<(Ident, UnresolvedType)>,
pub span: Span,
pub is_comptime: bool,
}

impl Display for NoirStruct {
Expand Down
3 changes: 0 additions & 3 deletions compiler/noirc_frontend/src/ast/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ pub struct TypeImpl {
pub generics: UnresolvedGenerics,
pub where_clause: Vec<UnresolvedTraitConstraint>,
pub methods: Vec<(NoirFunction, Span)>,
pub is_comptime: bool,
}

/// Ast node for an implementation of a trait for a particular type
Expand All @@ -70,8 +69,6 @@ pub struct NoirTraitImpl {
pub where_clause: Vec<UnresolvedTraitConstraint>,

pub items: Vec<TraitImplItem>,

pub is_comptime: bool,
}

/// Represents a simple trait constraint such as `where Foo: TraitY<U, V>`
Expand Down
144 changes: 47 additions & 97 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
def_collector::{
dc_crate::{
filter_literal_globals, CompilationError, ImplMap, UnresolvedGlobal,
UnresolvedStruct, UnresolvedTypeAlias,
UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias,
},
dc_mod,
},
Expand Down Expand Up @@ -251,15 +251,7 @@ impl<'context> Elaborator<'context> {
debug_comptime_in_file: Option<FileId>,
) -> Self {
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
// in the same crate, but can refer to any item in dependencies. Trying to
// run these at the same time as other items would lead to them seeing empty
// function bodies from functions that have yet to be elaborated.
let (comptime_items, runtime_items) = Self::filter_comptime_items(items);
this.elaborate_items(comptime_items);
this.elaborate_items(runtime_items);
this.elaborate_items(items);
this.check_and_pop_function_context();
this
}
Expand All @@ -284,11 +276,11 @@ impl<'context> Elaborator<'context> {
}

// Must resolve structs before we resolve globals.
let mut generated_items = self.collect_struct_definitions(items.types);
self.collect_struct_definitions(&items.types);

self.define_function_metas(&mut items.functions, &mut items.impls, &mut items.trait_impls);

self.collect_traits(items.traits, &mut generated_items);
self.collect_traits(&items.traits);

// 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
Expand All @@ -312,7 +304,7 @@ impl<'context> Elaborator<'context> {

// We have to run any comptime attributes on functions before the function is elaborated
// since the generated items are checked beforehand as well.
self.run_attributes_on_functions(&items.functions, &mut generated_items);
let generated_items = self.run_attributes(&items.traits, &items.types, &items.functions);

// After everything is collected, we can elaborate our generated items.
// It may be better to inline these within `items` entirely since elaborating them
Expand Down Expand Up @@ -1119,30 +1111,20 @@ impl<'context> Elaborator<'context> {
self.generics.clear();
}

fn collect_struct_definitions(
&mut self,
structs: BTreeMap<StructId, UnresolvedStruct>,
) -> CollectedItems {
fn collect_struct_definitions(&mut self, structs: &BTreeMap<StructId, UnresolvedStruct>) {
// This is necessary to avoid cloning the entire struct map
// when adding checks after each struct field is resolved.
let struct_ids = structs.keys().copied().collect::<Vec<_>>();

// This will contain any additional top-level items that are generated at compile-time
// via macros. This often includes derived trait impls.
let mut generated_items = CollectedItems::default();

// Resolve each field in each struct.
// Each struct should already be present in the NodeInterner after def collection.
for (type_id, mut typ) in structs {
for (type_id, typ) in structs {
self.file = typ.file_id;
self.local_module = typ.module_id;

let attributes = std::mem::take(&mut typ.struct_def.attributes);
let span = typ.struct_def.span;

let fields = self.resolve_struct_fields(typ.struct_def, type_id);
let fields = self.resolve_struct_fields(&typ.struct_def, *type_id);
let fields_len = fields.len();
self.interner.update_struct(type_id, |struct_def| {
self.interner.update_struct(*type_id, |struct_def| {
struct_def.set_fields(fields);

// TODO(https://github.com/noir-lang/noir/issues/5156): Remove this with implicit numeric generics
Expand All @@ -1169,12 +1151,11 @@ impl<'context> Elaborator<'context> {
});

for field_index in 0..fields_len {
self.interner
.add_definition_location(ReferenceId::StructMember(type_id, field_index), None);
self.interner.add_definition_location(
ReferenceId::StructMember(*type_id, field_index),
None,
);
}

let item = Value::StructDefinition(type_id);
self.run_comptime_attributes_on_item(&attributes, item, span, &mut generated_items);
}

// Check whether the struct fields have nested slices
Expand All @@ -1196,8 +1177,6 @@ impl<'context> Elaborator<'context> {
}
}
}

generated_items
}

fn run_comptime_attributes_on_item(
Expand Down Expand Up @@ -1314,7 +1293,7 @@ impl<'context> Elaborator<'context> {

pub fn resolve_struct_fields(
&mut self,
unresolved: NoirStruct,
unresolved: &NoirStruct,
struct_id: StructId,
) -> Vec<(Ident, Type)> {
self.recover_generics(|this| {
Expand All @@ -1325,7 +1304,9 @@ impl<'context> Elaborator<'context> {
let struct_def = this.interner.get_struct(struct_id);
this.add_existing_generics(&unresolved.generics, &struct_def.borrow().generics);

let fields = vecmap(unresolved.fields, |(ident, typ)| (ident, this.resolve_type(typ)));
let fields = vecmap(&unresolved.fields, |(ident, typ)| {
(ident.clone(), this.resolve_type(typ.clone()))
});

this.resolving_ids.remove(&struct_id);

Expand Down Expand Up @@ -1504,66 +1485,6 @@ impl<'context> Elaborator<'context> {
})
}

/// Filters out comptime items from non-comptime items.
/// Returns a pair of (comptime items, non-comptime items)
fn filter_comptime_items(mut items: CollectedItems) -> (CollectedItems, CollectedItems) {
let mut function_sets = Vec::with_capacity(items.functions.len());
let mut comptime_function_sets = Vec::new();

for function_set in items.functions {
let mut functions = Vec::with_capacity(function_set.functions.len());
let mut comptime_functions = Vec::new();

for function in function_set.functions {
if function.2.def.is_comptime {
comptime_functions.push(function);
} else {
functions.push(function);
}
}

let file_id = function_set.file_id;
let self_type = function_set.self_type;
let trait_id = function_set.trait_id;

if !comptime_functions.is_empty() {
comptime_function_sets.push(UnresolvedFunctions {
functions: comptime_functions,
file_id,
trait_id,
self_type: self_type.clone(),
});
}

function_sets.push(UnresolvedFunctions { functions, file_id, trait_id, self_type });
}

let (comptime_trait_impls, trait_impls) =
items.trait_impls.into_iter().partition(|trait_impl| trait_impl.is_comptime);

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: comptime_globals,
impls: rustc_hash::FxHashMap::default(),
};

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

fn add_items(
&mut self,
items: Vec<TopLevelStatement>,
Expand Down Expand Up @@ -1612,7 +1533,6 @@ impl<'context> Elaborator<'context> {
methods,
generics: trait_impl.impl_generics,
where_clause: trait_impl.where_clause,
is_comptime: trait_impl.is_comptime,

// These last fields are filled in later
trait_id: None,
Expand Down Expand Up @@ -1676,6 +1596,36 @@ impl<'context> Elaborator<'context> {
}
}

/// Run all the attributes on each item. The ordering is unspecified to users but currently
/// we run trait attributes first to (e.g.) register derive handlers before derive is
/// called on structs.
/// Returns any new items generated by attributes.
fn run_attributes(
&mut self,
traits: &BTreeMap<TraitId, UnresolvedTrait>,
types: &BTreeMap<StructId, UnresolvedStruct>,
functions: &[UnresolvedFunctions],
) -> CollectedItems {
let mut generated_items = CollectedItems::default();

for (trait_id, trait_) in traits {
let attributes = &trait_.trait_def.attributes;
let item = Value::TraitDefinition(*trait_id);
let span = trait_.trait_def.span;
self.run_comptime_attributes_on_item(attributes, item, span, &mut generated_items);
}

for (struct_id, struct_def) in types {
let attributes = &struct_def.struct_def.attributes;
let item = Value::StructDefinition(*struct_id);
let span = struct_def.struct_def.span;
self.run_comptime_attributes_on_item(attributes, item, span, &mut generated_items);
}

self.run_attributes_on_functions(functions, &mut generated_items);
generated_items
}

fn run_attributes_on_functions(
&mut self,
function_sets: &[UnresolvedFunctions],
Expand Down
29 changes: 9 additions & 20 deletions compiler/noirc_frontend/src/elaborator/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ use crate::{
FunctionKind, TraitItem, UnresolvedGeneric, UnresolvedGenerics, UnresolvedTraitConstraint,
},
hir::{
def_collector::dc_crate::{
CollectedItems, CompilationError, UnresolvedTrait, UnresolvedTraitImpl,
},
def_collector::dc_crate::{CompilationError, UnresolvedTrait, UnresolvedTraitImpl},
type_check::TypeCheckError,
},
hir_def::{
Expand All @@ -29,43 +27,34 @@ use crate::{
use super::Elaborator;

impl<'context> Elaborator<'context> {
pub fn collect_traits(
&mut self,
traits: BTreeMap<TraitId, UnresolvedTrait>,
generated_items: &mut CollectedItems,
) {
pub fn collect_traits(&mut self, traits: &BTreeMap<TraitId, UnresolvedTrait>) {
for (trait_id, unresolved_trait) in traits {
self.recover_generics(|this| {
let resolved_generics = this.interner.get_trait(trait_id).generics.clone();
let resolved_generics = this.interner.get_trait(*trait_id).generics.clone();
this.add_existing_generics(
&unresolved_trait.trait_def.generics,
&resolved_generics,
);

// Resolve order
// 1. Trait Types ( Trait constants can have a trait type, therefore types before constants)
let _ = this.resolve_trait_types(&unresolved_trait);
let _ = this.resolve_trait_types(unresolved_trait);
// 2. Trait Constants ( Trait's methods can use trait types & constants, therefore they should be after)
let _ = this.resolve_trait_constants(&unresolved_trait);
let _ = this.resolve_trait_constants(unresolved_trait);
// 3. Trait Methods
let methods = this.resolve_trait_methods(trait_id, &unresolved_trait);
let methods = this.resolve_trait_methods(*trait_id, unresolved_trait);

this.interner.update_trait(trait_id, |trait_def| {
this.interner.update_trait(*trait_id, |trait_def| {
trait_def.set_methods(methods);
});

let attributes = &unresolved_trait.trait_def.attributes;
let item = crate::hir::comptime::Value::TraitDefinition(trait_id);
let span = unresolved_trait.trait_def.span;
this.run_comptime_attributes_on_item(attributes, item, span, generated_items);
});

// This check needs to be after the trait's methods are set since
// the interner may set `interner.ordering_type` based on the result type
// of the Cmp trait, if this is it.
if self.crate_id.is_stdlib() {
self.interner.try_add_infix_operator_trait(trait_id);
self.interner.try_add_prefix_operator_trait(trait_id);
self.interner.try_add_infix_operator_trait(*trait_id);
self.interner.try_add_prefix_operator_trait(*trait_id);
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/noirc_frontend/src/hir/comptime/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
DebugEvaluateComptime { diagnostic: CustomDiagnostic, location: Location },
FailedToParseMacro { error: ParserError, tokens: Rc<Tokens>, rule: &'static str, file: FileId },
UnsupportedTopLevelItemUnquote { item: String, location: Location },
NonComptimeFnCallInSameCrate { function: String, location: Location },
ComptimeDependencyCycle { function: String, location: Location },
NoImpl { location: Location },
NoMatchingImplFound { error: NoMatchingImplFoundError, file: FileId },
ImplMethodTypeMismatch { expected: Type, actual: Type, location: Location },
Expand Down Expand Up @@ -112,7 +112,7 @@
| InterpreterError::CannotInlineMacro { location, .. }
| InterpreterError::UnquoteFoundDuringEvaluation { location, .. }
| InterpreterError::UnsupportedTopLevelItemUnquote { location, .. }
| InterpreterError::NonComptimeFnCallInSameCrate { location, .. }
| InterpreterError::ComptimeDependencyCycle { location, .. }
| InterpreterError::Unimplemented { location, .. }
| InterpreterError::NoImpl { location, .. }
| InterpreterError::ImplMethodTypeMismatch { location, .. }
Expand Down Expand Up @@ -313,7 +313,7 @@
let message = format!("Failed to parse macro's token stream into {rule}");
let tokens = vecmap(&tokens.0, ToString::to_string).join(" ");

// 10 is an aribtrary number of tokens here chosen to fit roughly onto one line

Check warning on line 316 in compiler/noirc_frontend/src/hir/comptime/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (aribtrary)
let token_stream = if tokens.len() > 10 {
format!("The resulting token stream was: {tokens}")
} else {
Expand Down Expand Up @@ -342,10 +342,10 @@
error.add_note(format!("Unquoted item was:\n{item}"));
error
}
InterpreterError::NonComptimeFnCallInSameCrate { function, location } => {
let msg = format!("`{function}` cannot be called in a `comptime` context here");
InterpreterError::ComptimeDependencyCycle { function, location } => {
let msg = format!("Comptime dependency cycle while resolving `{function}`");
let secondary =
"This function must be `comptime` or in a separate crate to be called".into();
"This function uses comptime code internally which calls into itself".into();
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::Unimplemented { item, location } => {
Expand Down
Loading
Loading