Skip to content

Commit

Permalink
feat: Remove 'comptime or separate crate' restriction on comptime code (
Browse files Browse the repository at this point in the history 
#5609)

# Description

## Problem\*

Resolves <none, this is a bonus>

## Summary\*

Removes the restriction that comptime code can only call into other
comptime code or code in a separate crate. This is possible now that
functions can be lazily resolved in the interpreter.

This means we no longer need hacky solutions like "comptime impl" on
trait impls. `comptime` itself also gets simplified from its previous
two meanings of "run this at comptime" and "make this visible to other
comptime code" to just "run this at comptime."

## Additional Context

I've also snuck in a sneaky ordering change where we run annotations on
traits before annotations on structs. Function annotations are still run
last.

## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [x] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
@jfecher
jfecher authored Jul 26, 2024
1 parent 29295a0 commit 1cddf42
Show file tree
Hide file tree
Showing 18 changed files with 77 additions and 184 deletions.
1 change: 0 additions & 1 deletion aztec_macros/src/transforms/note_interface.rs
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ pub fn generate_storage_implementation(
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
View file
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
View file
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
View file
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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub enum InterpreterError {
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 @@ impl InterpreterError {
| 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 @@ -342,10 +342,10 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
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

0 comments on commit 1cddf42

Please sign in to comment.