From ce2d1f0a9c37f3bd251a7372ea63825bca662ef7 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 11 Oct 2024 10:18:11 +0100 Subject: [PATCH 01/15] Start work on relative attributes --- .../noirc_frontend/src/elaborator/comptime.rs | 91 +++++++++++++++---- compiler/noirc_frontend/src/elaborator/mod.rs | 2 + .../noirc_frontend/src/elaborator/patterns.rs | 2 +- .../noirc_frontend/src/elaborator/types.rs | 2 +- compiler/noirc_frontend/src/lexer/token.rs | 14 +++ compiler/noirc_frontend/src/node_interner.rs | 8 +- noir_stdlib/src/meta/mod.nr | 6 ++ 7 files changed, 105 insertions(+), 20 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 65cb6072c62..8e38385ee7b 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, fmt::Display}; +use std::{collections::BTreeMap, fmt::Display, cmp::Ordering}; use fm::FileId; use iter_extended::vecmap; @@ -137,7 +137,7 @@ impl<'context> Elaborator<'context> { item: Value, span: Span, attribute_context: AttributeContext, - generated_items: &mut CollectedItems, + attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, ) { for attribute in attributes { self.run_comptime_attribute_on_item( @@ -145,7 +145,7 @@ impl<'context> Elaborator<'context> { &item, span, attribute_context, - generated_items, + attributes_to_run, ); } } @@ -156,7 +156,7 @@ impl<'context> Elaborator<'context> { item: &Value, span: Span, attribute_context: AttributeContext, - generated_items: &mut CollectedItems, + attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, ) { if let SecondaryAttribute::Meta(attribute) = attribute { self.elaborate_in_comptime_context(|this| { @@ -166,7 +166,7 @@ impl<'context> Elaborator<'context> { span, attribute.contents_span, attribute_context, - generated_items, + attributes_to_run, ) { this.errors.push(error); } @@ -181,7 +181,7 @@ impl<'context> Elaborator<'context> { span: Span, attribute_span: Span, attribute_context: AttributeContext, - generated_items: &mut CollectedItems, + attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.attribute_file; self.local_module = attribute_context.attribute_module; @@ -233,6 +233,11 @@ impl<'context> Elaborator<'context> { return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file)); }; + attributes_to_run.push((function, item, attribute_context, arguments)); + Ok(()) + } + + fn foo(&mut self, attribute_context: AttributeContext, function: FuncId, arguments: Vec, item: Value, location: Location, generated_items: &mut CollectedItems) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.file; self.local_module = attribute_context.module; @@ -537,7 +542,7 @@ impl<'context> Elaborator<'context> { functions: &[UnresolvedFunctions], module_attributes: &[ModuleAttribute], ) -> CollectedItems { - let mut generated_items = CollectedItems::default(); + let mut attributes_to_run = Vec::new(); for (trait_id, trait_) in traits { let attributes = &trait_.trait_def.attributes; @@ -549,7 +554,7 @@ impl<'context> Elaborator<'context> { item, span, context, - &mut generated_items, + &mut attributes_to_run, ); } @@ -563,21 +568,37 @@ impl<'context> Elaborator<'context> { item, span, context, - &mut generated_items, + &mut attributes_to_run, ); } - self.run_attributes_on_functions(functions, &mut generated_items); - - self.run_attributes_on_modules(module_attributes, &mut generated_items); - + self.run_attributes_on_functions(functions, &mut attributes_to_run); + self.run_attributes_on_modules(module_attributes, &mut attributes_to_run); + + // sort + attributes_to_run.sort_by(|(l_fn, ..), (r_fn, ..)| { + let l_fn = DependencyId::Attribute(*l_fn); + let r_fn = DependencyId::Attribute(*r_fn); + self.interner.dependency_graph.depe + }); + + // -1: O O + // ^ ^ + // 0: O + // ^ ^ + // 1: O O + // ^ + // 2: O + + // run + let generated_items = CollectedItems::default(); generated_items } fn run_attributes_on_modules( &mut self, module_attributes: &[ModuleAttribute], - generated_items: &mut CollectedItems, + attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, ) { for module_attribute in module_attributes { let local_id = module_attribute.module_id; @@ -593,14 +614,14 @@ impl<'context> Elaborator<'context> { attribute_module: module_attribute.attribute_module_id, }; - self.run_comptime_attribute_on_item(attribute, &item, span, context, generated_items); + self.run_comptime_attribute_on_item(attribute, &item, span, context, attributes_to_run); } } fn run_attributes_on_functions( &mut self, function_sets: &[UnresolvedFunctions], - generated_items: &mut CollectedItems, + attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, ) { for function_set in function_sets { self.self_type = function_set.self_type.clone(); @@ -615,7 +636,7 @@ impl<'context> Elaborator<'context> { item, span, context, - generated_items, + attributes_to_run, ); } } @@ -650,4 +671,40 @@ impl<'context> Elaborator<'context> { _ => false, } } + + pub(super) fn register_attribute_order(&mut self, id: FuncId, attributes: &[SecondaryAttribute]) { + for attribute in attributes { + let (name, run_before) = match attribute { + SecondaryAttribute::RunBefore(name) => (name, true), + SecondaryAttribute::RunAfter(name) => (name, false), + _ => continue, + }; + + let Some(path) = Parser::for_str(name).parse_path_no_turbofish() else { + todo!("function should be a path") + }; + + let definition_id = self.resolve_variable(path).id; + + match self.interner.definition(definition_id).kind { + DefinitionKind::Function(function_id) => { + if function_id == id { + todo!("Attribute cannot be run before or after itself"); + } + + let from = DependencyId::Attribute(id); + let to = DependencyId::Attribute(function_id); + + if run_before { + self.interner.add_dependency(from, to); + } else { + self.interner.add_dependency(to, from); + } + }, + _ => { + todo!("path doesn't refer to a function") + } + } + } + } } diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index a9173621fc7..20324a42cce 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -796,6 +796,8 @@ impl<'context> Elaborator<'context> { None }; + self.register_attribute_order(func_id, func.secondary_attributes()); + let attributes = func.secondary_attributes().iter(); let attributes = attributes.filter_map(|secondary_attribute| secondary_attribute.as_custom()); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 0f6cb78fae7..3bd109b8426 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -534,7 +534,7 @@ impl<'context> Elaborator<'context> { (id, typ) } - fn resolve_variable(&mut self, path: Path) -> HirIdent { + pub fn resolve_variable(&mut self, path: Path) -> HirIdent { if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) { if let Some(error) = trait_path_resolution.error { self.push_err(error); diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 82d14743428..5b7877c1ade 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -566,7 +566,7 @@ impl<'context> Elaborator<'context> { let path_resolution = self.resolve_path(path.clone()).ok()?; let ModuleDefId::FunctionId(func_id) = path_resolution.module_def_id else { return None }; - let meta = self.interner.function_meta(&func_id); + let meta = self.interner.try_function_meta(&func_id)?; let the_trait = self.interner.get_trait(meta.trait_id?); let method = the_trait.find_method(path.last_name())?; let constraint = the_trait.as_constraint(path.span); diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 4037135a889..4fe57580dd2 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -831,6 +831,8 @@ impl Attribute { ["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs), ["use_callers_scope"] => Attribute::Secondary(SecondaryAttribute::UseCallersScope), ["allow", tag] => Attribute::Secondary(SecondaryAttribute::Allow(tag.to_string())), + ["run_before", f] => Attribute::Secondary(SecondaryAttribute::RunBefore(f.to_string())), + ["run_after", f] => Attribute::Secondary(SecondaryAttribute::RunAfter(f.to_string())), tokens => { tokens.iter().try_for_each(|token| validate(token))?; Attribute::Secondary(SecondaryAttribute::Meta(CustomAttribute { @@ -961,6 +963,12 @@ pub enum SecondaryAttribute { /// Allow chosen warnings to happen so they are silenced. Allow(String), + + /// Specifies this attribute should run before the attribute specified as an argument + RunBefore(String), + + /// Specifies this attribute should run after the attribute specified as an argument + RunAfter(String), } impl SecondaryAttribute { @@ -986,6 +994,8 @@ impl SecondaryAttribute { SecondaryAttribute::Varargs => Some("varargs".to_string()), SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()), SecondaryAttribute::Allow(_) => Some("allow".to_string()), + SecondaryAttribute::RunBefore(_) => Some("run_before".to_string()), + SecondaryAttribute::RunAfter(_) => Some("run_after".to_string()), } } @@ -1017,6 +1027,8 @@ impl fmt::Display for SecondaryAttribute { SecondaryAttribute::Varargs => write!(f, "#[varargs]"), SecondaryAttribute::UseCallersScope => write!(f, "#[use_callers_scope]"), SecondaryAttribute::Allow(ref k) => write!(f, "#[allow(#{k})]"), + SecondaryAttribute::RunBefore(attr) => write!(f, "#[run_before({attr})]"), + SecondaryAttribute::RunAfter(attr) => write!(f, "#[run_after({attr})]"), } } } @@ -1065,6 +1077,8 @@ impl AsRef for SecondaryAttribute { SecondaryAttribute::Meta(attribute) => &attribute.contents, SecondaryAttribute::Field(string) | SecondaryAttribute::Abi(string) + | SecondaryAttribute::RunBefore(string) + | SecondaryAttribute::RunAfter(string) | SecondaryAttribute::Allow(string) => string, SecondaryAttribute::ContractLibraryMethod => "", SecondaryAttribute::Export => "", diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index b80c37c2ce4..86e433ec7bf 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -292,6 +292,7 @@ pub enum DependencyId { Function(FuncId), Alias(TypeAliasId), Variable(Location), + Attribute(FuncId), } /// A reference to a module, struct, trait, etc., mainly used by the LSP code @@ -2071,6 +2072,11 @@ impl NodeInterner { push_error(alias.name.to_string(), &scc, i, alias.location); break; } + DependencyId::Attribute(id) => { + let name = self.function_name(&id).to_string(); + let location = self.function_meta(&id).location; + push_error(name, &scc, i, location) + } // Mutually recursive functions are allowed DependencyId::Function(_) => (), // Local variables should never be in a dependency cycle, scoping rules @@ -2092,7 +2098,7 @@ impl NodeInterner { fn get_cycle_error_string(&self, scc: &[PetGraphIndex], start_index: usize) -> String { let index_to_string = |index: PetGraphIndex| match self.dependency_graph[index] { DependencyId::Struct(id) => Cow::Owned(self.get_struct(id).borrow().name.to_string()), - DependencyId::Function(id) => Cow::Borrowed(self.function_name(&id)), + DependencyId::Function(id) | DependencyId::Attribute(id) => Cow::Borrowed(self.function_name(&id)), DependencyId::Alias(id) => { Cow::Owned(self.get_type_alias(id).borrow().name.to_string()) } diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index f756be364b1..507e8e0c75a 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -28,6 +28,10 @@ pub comptime fn unquote(code: Quoted) -> Quoted { pub comptime fn type_of(x: T) -> Type {} // docs:end:type_of +/// Stub function used so that other attributes can specify if they +/// want to run before or after any attributes without a stage annotation. +pub comptime fn default_stage() {} + // docs:start:derive_example // These are needed for the unconstrained hashmap we're using to store derive functions use crate::collections::umap::UHashMap; @@ -46,6 +50,7 @@ comptime mut global HANDLERS: UHashMap Quoted { // docs:end:derive let mut result = quote {}; @@ -67,6 +72,7 @@ pub comptime fn derive(s: StructDefinition, traits: [TraitDefinition]) -> Quoted // docs:start:derive_via // To register a handler for a trait, just add it to our handlers map // docs:start:derive_via_signature +#[run_before(derive)] pub comptime fn derive_via(t: TraitDefinition, f: DeriveFunction) { // docs:end:derive_via_signature HANDLERS.insert(t, f); From 687f950ed5e216334d8d1cfd5f69df186b758a5b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 11 Oct 2024 16:50:47 +0100 Subject: [PATCH 02/15] Select bellman-ford --- .../noirc_frontend/src/elaborator/comptime.rs | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 8e38385ee7b..ab8d22967ef 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, fmt::Display, cmp::Ordering}; +use std::{collections::BTreeMap, fmt::Display}; use fm::FileId; use iter_extended::vecmap; @@ -576,20 +576,32 @@ impl<'context> Elaborator<'context> { self.run_attributes_on_modules(module_attributes, &mut attributes_to_run); // sort - attributes_to_run.sort_by(|(l_fn, ..), (r_fn, ..)| { - let l_fn = DependencyId::Attribute(*l_fn); - let r_fn = DependencyId::Attribute(*r_fn); - self.interner.dependency_graph.depe - }); + // Use bellman_ford to determine path costs to a start node + // - anything running before the start node has its edge and edge cost reversed + let stages = petgraph::algo::bellman_ford(graph, start); // -1: O O // ^ ^ - // 0: O + // 0: O let this be the start node // ^ ^ // 1: O O // ^ // 2: O + // -1: O O + // v v edge cost both -1 + // 0: O + // ^ ^ edge cost both 1 + // 1: O O + // ^ edge cost is 1 + // 2: O + + attributes_to_run.sort_by(|(l_fn, ..), (r_fn, ..)| { + let l_fn = DependencyId::Attribute(*l_fn); + let r_fn = DependencyId::Attribute(*r_fn); + self.interner.dependency_graph.depe + }); + // run let generated_items = CollectedItems::default(); generated_items From 752c93e325255a4a2901c1138928098f535a97a5 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 22 Oct 2024 16:09:48 -0500 Subject: [PATCH 03/15] Get it working --- .../noirc_frontend/src/attribute_order.rs | 93 +++++++++++ .../noirc_frontend/src/elaborator/comptime.rs | 149 ++++++++---------- compiler/noirc_frontend/src/lib.rs | 1 + compiler/noirc_frontend/src/node_interner.rs | 12 +- 4 files changed, 168 insertions(+), 87 deletions(-) create mode 100644 compiler/noirc_frontend/src/attribute_order.rs diff --git a/compiler/noirc_frontend/src/attribute_order.rs b/compiler/noirc_frontend/src/attribute_order.rs new file mode 100644 index 00000000000..f4a45f7ff33 --- /dev/null +++ b/compiler/noirc_frontend/src/attribute_order.rs @@ -0,0 +1,93 @@ +use fm::FileId; +use noirc_errors::Span; +use petgraph::prelude::{DiGraph, NodeIndex}; +use rustc_hash::FxHashMap as HashMap; + +use crate::{ + ast::Expression, + hir::{comptime::Value, def_map::LocalModuleId}, + node_interner::FuncId, +}; + +#[derive(Debug)] +pub struct AttributeGraph { + default_stage: NodeIndex, + + order: DiGraph, + + indices: HashMap, + + modified_functions: std::collections::HashSet, +} + +#[derive(Debug, Copy, Clone)] +pub(crate) struct AttributeContext { + // The file where generated items should be added + pub(crate) file: FileId, + // The module where generated items should be added + pub(crate) module: LocalModuleId, + // The file where the attribute is located + pub(crate) attribute_file: FileId, + // The module where the attribute is located + pub(crate) attribute_module: LocalModuleId, +} + +pub(crate) type CollectedAttributes = Vec<(FuncId, Value, Vec, AttributeContext, Span)>; + +impl AttributeContext { + pub(crate) fn new(file: FileId, module: LocalModuleId) -> Self { + Self { file, module, attribute_file: file, attribute_module: module } + } +} + +impl Default for AttributeGraph { + fn default() -> Self { + let mut order = DiGraph::default(); + let mut indices = HashMap::default(); + + let default_stage = order.add_node(FuncId::dummy_id()); + indices.insert(FuncId::dummy_id(), default_stage); + + Self { default_stage, order, indices, modified_functions: Default::default() } + } +} + +impl AttributeGraph { + pub fn get_or_insert(&mut self, attr: FuncId) -> NodeIndex { + if let Some(index) = self.indices.get(&attr) { + return *index; + } + + let index = self.order.add_node(attr); + self.indices.insert(attr, index); + index + } + + pub fn add_ordering_constraint(&mut self, run_first: FuncId, run_second: FuncId) { + let first_index = self.get_or_insert(run_first); + let second_index = self.get_or_insert(run_second); + + // Just for debugging + if run_first != FuncId::dummy_id() { + self.modified_functions.insert(run_first); + self.modified_functions.insert(run_second); + } + + self.order.update_edge(second_index, first_index, 1.0); + } + + /// The default ordering of an attribute: run in the default stage + pub fn run_in_default_stage(&mut self, attr: FuncId) { + let index = self.get_or_insert(attr); + self.order.update_edge(self.default_stage, index, 1.0); + } + + pub(crate) fn sort_attributes_by_run_order(&self, attributes: &mut CollectedAttributes) { + let topological_sort = petgraph::algo::toposort(&self.order, None).unwrap(); + + let ordering: HashMap = + topological_sort.into_iter().map(|index| (self.order[index], index.index())).collect(); + + attributes.sort_by_key(|(f, ..)| ordering[f]); + } +} diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index ab8d22967ef..920191e4173 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -6,6 +6,7 @@ use noirc_errors::{Location, Span}; use crate::{ ast::{Documented, Expression, ExpressionKind}, + attribute_order::{AttributeContext, CollectedAttributes}, hir::{ comptime::{Interpreter, InterpreterError, Value}, def_collector::{ @@ -15,7 +16,7 @@ use crate::{ }, dc_mod, }, - def_map::{LocalModuleId, ModuleId}, + def_map::ModuleId, resolution::errors::ResolverError, }, hir_def::expr::{HirExpression, HirIdent}, @@ -28,24 +29,6 @@ use crate::{ use super::{Elaborator, FunctionContext, ResolverMeta}; -#[derive(Debug, Copy, Clone)] -struct AttributeContext { - // The file where generated items should be added - file: FileId, - // The module where generated items should be added - module: LocalModuleId, - // The file where the attribute is located - attribute_file: FileId, - // The module where the attribute is located - attribute_module: LocalModuleId, -} - -impl AttributeContext { - fn new(file: FileId, module: LocalModuleId) -> Self { - Self { file, module, attribute_file: file, attribute_module: module } - } -} - impl<'context> Elaborator<'context> { /// Elaborate an expression from the middle of a comptime scope. /// When this happens we require additional information to know @@ -131,16 +114,16 @@ impl<'context> Elaborator<'context> { } } - fn run_comptime_attributes_on_item( + fn collect_comptime_attributes_on_item( &mut self, attributes: &[SecondaryAttribute], item: Value, span: Span, attribute_context: AttributeContext, - attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, + attributes_to_run: &mut CollectedAttributes, ) { for attribute in attributes { - self.run_comptime_attribute_on_item( + self.collect_comptime_attribute_on_item( attribute, &item, span, @@ -150,17 +133,17 @@ impl<'context> Elaborator<'context> { } } - fn run_comptime_attribute_on_item( + fn collect_comptime_attribute_on_item( &mut self, attribute: &SecondaryAttribute, item: &Value, span: Span, attribute_context: AttributeContext, - attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, + attributes_to_run: &mut CollectedAttributes, ) { if let SecondaryAttribute::Meta(attribute) = attribute { self.elaborate_in_comptime_context(|this| { - if let Err(error) = this.run_comptime_attribute_name_on_item( + if let Err(error) = this.collect_comptime_attribute_name_on_item( &attribute.contents, item.clone(), span, @@ -174,14 +157,15 @@ impl<'context> Elaborator<'context> { } } - fn run_comptime_attribute_name_on_item( + /// Resolve an attribute to the function it refers to and add it to `attributes_to_run` + fn collect_comptime_attribute_name_on_item( &mut self, attribute: &str, item: Value, span: Span, attribute_span: Span, attribute_context: AttributeContext, - attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, + attributes_to_run: &mut CollectedAttributes, ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.attribute_file; self.local_module = attribute_context.attribute_module; @@ -233,11 +217,19 @@ impl<'context> Elaborator<'context> { return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file)); }; - attributes_to_run.push((function, item, attribute_context, arguments)); + attributes_to_run.push((function, item, arguments, attribute_context, span)); Ok(()) } - fn foo(&mut self, attribute_context: AttributeContext, function: FuncId, arguments: Vec, item: Value, location: Location, generated_items: &mut CollectedItems) -> Result<(), (CompilationError, FileId)> { + fn run_attribute( + &mut self, + attribute_context: AttributeContext, + function: FuncId, + arguments: Vec, + item: Value, + location: Location, + generated_items: &mut CollectedItems, + ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.file; self.local_module = attribute_context.module; @@ -249,10 +241,7 @@ impl<'context> Elaborator<'context> { arguments, location, ) - .map_err(|error| { - let file = error.get_location().file; - (error.into(), file) - })?; + .map_err(|error| error.into_compilation_error_pair())?; arguments.insert(0, (item, location)); @@ -549,7 +538,7 @@ impl<'context> Elaborator<'context> { let item = Value::TraitDefinition(*trait_id); let span = trait_.trait_def.span; let context = AttributeContext::new(trait_.file_id, trait_.module_id); - self.run_comptime_attributes_on_item( + self.collect_comptime_attributes_on_item( attributes, item, span, @@ -563,7 +552,7 @@ impl<'context> Elaborator<'context> { let item = Value::StructDefinition(*struct_id); let span = struct_def.struct_def.span; let context = AttributeContext::new(struct_def.file_id, struct_def.module_id); - self.run_comptime_attributes_on_item( + self.collect_comptime_attributes_on_item( attributes, item, span, @@ -572,45 +561,29 @@ impl<'context> Elaborator<'context> { ); } - self.run_attributes_on_functions(functions, &mut attributes_to_run); - self.run_attributes_on_modules(module_attributes, &mut attributes_to_run); - - // sort - // Use bellman_ford to determine path costs to a start node - // - anything running before the start node has its edge and edge cost reversed - let stages = petgraph::algo::bellman_ford(graph, start); - - // -1: O O - // ^ ^ - // 0: O let this be the start node - // ^ ^ - // 1: O O - // ^ - // 2: O - - // -1: O O - // v v edge cost both -1 - // 0: O - // ^ ^ edge cost both 1 - // 1: O O - // ^ edge cost is 1 - // 2: O - - attributes_to_run.sort_by(|(l_fn, ..), (r_fn, ..)| { - let l_fn = DependencyId::Attribute(*l_fn); - let r_fn = DependencyId::Attribute(*r_fn); - self.interner.dependency_graph.depe - }); + self.collect_attributes_on_functions(functions, &mut attributes_to_run); + self.collect_attributes_on_modules(module_attributes, &mut attributes_to_run); + + self.interner.attribute_order.sort_attributes_by_run_order(&mut attributes_to_run); // run - let generated_items = CollectedItems::default(); + let mut generated_items = CollectedItems::default(); + for (attribute, item, args, context, span) in attributes_to_run { + let location = Location::new(span, context.attribute_file); + if let Err(error) = + self.run_attribute(context, attribute, args, item, location, &mut generated_items) + { + self.errors.push(error); + } + } + generated_items } - fn run_attributes_on_modules( + fn collect_attributes_on_modules( &mut self, module_attributes: &[ModuleAttribute], - attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, + attributes_to_run: &mut CollectedAttributes, ) { for module_attribute in module_attributes { let local_id = module_attribute.module_id; @@ -626,14 +599,20 @@ impl<'context> Elaborator<'context> { attribute_module: module_attribute.attribute_module_id, }; - self.run_comptime_attribute_on_item(attribute, &item, span, context, attributes_to_run); + self.collect_comptime_attribute_on_item( + attribute, + &item, + span, + context, + attributes_to_run, + ); } } - fn run_attributes_on_functions( + fn collect_attributes_on_functions( &mut self, function_sets: &[UnresolvedFunctions], - attributes_to_run: &mut Vec<(FuncId, Value, AttributeContext, Vec)>, + attributes_to_run: &mut CollectedAttributes, ) { for function_set in function_sets { self.self_type = function_set.self_type.clone(); @@ -643,7 +622,7 @@ impl<'context> Elaborator<'context> { let attributes = function.secondary_attributes(); let item = Value::FunctionDefinition(*function_id); let span = function.span(); - self.run_comptime_attributes_on_item( + self.collect_comptime_attributes_on_item( attributes, item, span, @@ -684,7 +663,13 @@ impl<'context> Elaborator<'context> { } } - pub(super) fn register_attribute_order(&mut self, id: FuncId, attributes: &[SecondaryAttribute]) { + pub(super) fn register_attribute_order( + &mut self, + id: FuncId, + attributes: &[SecondaryAttribute], + ) { + let mut has_order = false; + for attribute in attributes { let (name, run_before) = match attribute { SecondaryAttribute::RunBefore(name) => (name, true), @@ -692,6 +677,7 @@ impl<'context> Elaborator<'context> { _ => continue, }; + // Parse a path from #[run_before(path)] let Some(path) = Parser::for_str(name).parse_path_no_turbofish() else { todo!("function should be a path") }; @@ -699,24 +685,27 @@ impl<'context> Elaborator<'context> { let definition_id = self.resolve_variable(path).id; match self.interner.definition(definition_id).kind { - DefinitionKind::Function(function_id) => { - if function_id == id { + DefinitionKind::Function(attribute_arg) => { + if attribute_arg == id { todo!("Attribute cannot be run before or after itself"); } - let from = DependencyId::Attribute(id); - let to = DependencyId::Attribute(function_id); - + has_order = true; if run_before { - self.interner.add_dependency(from, to); + self.interner.attribute_order.add_ordering_constraint(attribute_arg, id); } else { - self.interner.add_dependency(to, from); + self.interner.attribute_order.add_ordering_constraint(id, attribute_arg); } - }, + } _ => { todo!("path doesn't refer to a function") } } } + + // If no ordering was specified, set it to the default stage. + if !has_order { + self.interner.attribute_order.run_in_default_stage(id); + } } } diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 9d98b125e32..1cbb2057fd0 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -11,6 +11,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] pub mod ast; +pub mod attribute_order; pub mod debug; pub mod elaborator; pub mod graph; diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 86e433ec7bf..6c4bc8b2d1d 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -16,6 +16,7 @@ use rustc_hash::FxHashMap as HashMap; use crate::ast::{ ExpressionKind, Ident, LValue, Pattern, StatementKind, UnaryOp, UnresolvedTypeData, }; +use crate::attribute_order::AttributeGraph; use crate::graph::CrateId; use crate::hir::comptime; use crate::hir::def_collector::dc_crate::CompilationError; @@ -273,6 +274,8 @@ pub struct NodeInterner { /// Captures the documentation comments for each module, struct, trait, function, etc. pub(crate) doc_comments: HashMap>, + + pub(crate) attribute_order: AttributeGraph, } /// A dependency in the dependency graph may be a type or a definition. @@ -292,7 +295,6 @@ pub enum DependencyId { Function(FuncId), Alias(TypeAliasId), Variable(Location), - Attribute(FuncId), } /// A reference to a module, struct, trait, etc., mainly used by the LSP code @@ -680,6 +682,7 @@ impl Default for NodeInterner { trait_impl_associated_types: HashMap::default(), usage_tracker: UsageTracker::default(), doc_comments: HashMap::default(), + attribute_order: AttributeGraph::default(), } } } @@ -2072,11 +2075,6 @@ impl NodeInterner { push_error(alias.name.to_string(), &scc, i, alias.location); break; } - DependencyId::Attribute(id) => { - let name = self.function_name(&id).to_string(); - let location = self.function_meta(&id).location; - push_error(name, &scc, i, location) - } // Mutually recursive functions are allowed DependencyId::Function(_) => (), // Local variables should never be in a dependency cycle, scoping rules @@ -2098,7 +2096,7 @@ impl NodeInterner { fn get_cycle_error_string(&self, scc: &[PetGraphIndex], start_index: usize) -> String { let index_to_string = |index: PetGraphIndex| match self.dependency_graph[index] { DependencyId::Struct(id) => Cow::Owned(self.get_struct(id).borrow().name.to_string()), - DependencyId::Function(id) | DependencyId::Attribute(id) => Cow::Borrowed(self.function_name(&id)), + DependencyId::Function(id) => Cow::Borrowed(self.function_name(&id)), DependencyId::Alias(id) => { Cow::Owned(self.get_type_alias(id).borrow().name.to_string()) } From a3d7d213aaf6cb747dfc5c4bd030a367446f55b9 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 23 Oct 2024 13:21:09 -0500 Subject: [PATCH 04/15] Order attributes by source ordering --- .../noirc_frontend/src/attribute_order.rs | 93 ------------------ .../noirc_frontend/src/elaborator/comptime.rs | 98 ++++++++----------- compiler/noirc_frontend/src/elaborator/mod.rs | 2 - .../noirc_frontend/src/hir/def_map/mod.rs | 25 +++++ .../src/hir/def_map/module_data.rs | 10 ++ compiler/noirc_frontend/src/lexer/token.rs | 14 --- compiler/noirc_frontend/src/lib.rs | 1 - compiler/noirc_frontend/src/node_interner.rs | 4 - noir_stdlib/src/meta/mod.nr | 6 -- 9 files changed, 78 insertions(+), 175 deletions(-) delete mode 100644 compiler/noirc_frontend/src/attribute_order.rs diff --git a/compiler/noirc_frontend/src/attribute_order.rs b/compiler/noirc_frontend/src/attribute_order.rs deleted file mode 100644 index f4a45f7ff33..00000000000 --- a/compiler/noirc_frontend/src/attribute_order.rs +++ /dev/null @@ -1,93 +0,0 @@ -use fm::FileId; -use noirc_errors::Span; -use petgraph::prelude::{DiGraph, NodeIndex}; -use rustc_hash::FxHashMap as HashMap; - -use crate::{ - ast::Expression, - hir::{comptime::Value, def_map::LocalModuleId}, - node_interner::FuncId, -}; - -#[derive(Debug)] -pub struct AttributeGraph { - default_stage: NodeIndex, - - order: DiGraph, - - indices: HashMap, - - modified_functions: std::collections::HashSet, -} - -#[derive(Debug, Copy, Clone)] -pub(crate) struct AttributeContext { - // The file where generated items should be added - pub(crate) file: FileId, - // The module where generated items should be added - pub(crate) module: LocalModuleId, - // The file where the attribute is located - pub(crate) attribute_file: FileId, - // The module where the attribute is located - pub(crate) attribute_module: LocalModuleId, -} - -pub(crate) type CollectedAttributes = Vec<(FuncId, Value, Vec, AttributeContext, Span)>; - -impl AttributeContext { - pub(crate) fn new(file: FileId, module: LocalModuleId) -> Self { - Self { file, module, attribute_file: file, attribute_module: module } - } -} - -impl Default for AttributeGraph { - fn default() -> Self { - let mut order = DiGraph::default(); - let mut indices = HashMap::default(); - - let default_stage = order.add_node(FuncId::dummy_id()); - indices.insert(FuncId::dummy_id(), default_stage); - - Self { default_stage, order, indices, modified_functions: Default::default() } - } -} - -impl AttributeGraph { - pub fn get_or_insert(&mut self, attr: FuncId) -> NodeIndex { - if let Some(index) = self.indices.get(&attr) { - return *index; - } - - let index = self.order.add_node(attr); - self.indices.insert(attr, index); - index - } - - pub fn add_ordering_constraint(&mut self, run_first: FuncId, run_second: FuncId) { - let first_index = self.get_or_insert(run_first); - let second_index = self.get_or_insert(run_second); - - // Just for debugging - if run_first != FuncId::dummy_id() { - self.modified_functions.insert(run_first); - self.modified_functions.insert(run_second); - } - - self.order.update_edge(second_index, first_index, 1.0); - } - - /// The default ordering of an attribute: run in the default stage - pub fn run_in_default_stage(&mut self, attr: FuncId) { - let index = self.get_or_insert(attr); - self.order.update_edge(self.default_stage, index, 1.0); - } - - pub(crate) fn sort_attributes_by_run_order(&self, attributes: &mut CollectedAttributes) { - let topological_sort = petgraph::algo::toposort(&self.order, None).unwrap(); - - let ordering: HashMap = - topological_sort.into_iter().map(|index| (self.order[index], index.index())).collect(); - - attributes.sort_by_key(|(f, ..)| ordering[f]); - } -} diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 920191e4173..57a19c2de7b 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -6,7 +6,6 @@ use noirc_errors::{Location, Span}; use crate::{ ast::{Documented, Expression, ExpressionKind}, - attribute_order::{AttributeContext, CollectedAttributes}, hir::{ comptime::{Interpreter, InterpreterError, Value}, def_collector::{ @@ -16,7 +15,7 @@ use crate::{ }, dc_mod, }, - def_map::ModuleId, + def_map::{LocalModuleId, ModuleId}, resolution::errors::ResolverError, }, hir_def::expr::{HirExpression, HirIdent}, @@ -29,6 +28,27 @@ use crate::{ use super::{Elaborator, FunctionContext, ResolverMeta}; +#[derive(Debug, Copy, Clone)] +pub(crate) struct AttributeContext { + // The file where generated items should be added + pub(crate) file: FileId, + // The module where generated items should be added + pub(crate) module: LocalModuleId, + // The file where the attribute is located + pub(crate) attribute_file: FileId, + // The module where the attribute is located + pub(crate) attribute_module: LocalModuleId, +} + +pub(crate) type CollectedAttributes = + Vec<(FuncId, Value, Vec, AttributeContext, Span, LocalModuleId)>; + +impl AttributeContext { + pub(crate) fn new(file: FileId, module: LocalModuleId) -> Self { + Self { file, module, attribute_file: file, attribute_module: module } + } +} + impl<'context> Elaborator<'context> { /// Elaborate an expression from the middle of a comptime scope. /// When this happens we require additional information to know @@ -217,7 +237,14 @@ impl<'context> Elaborator<'context> { return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file)); }; - attributes_to_run.push((function, item, arguments, attribute_context, span)); + attributes_to_run.push(( + function, + item, + arguments, + attribute_context, + span, + self.local_module, + )); Ok(()) } @@ -520,10 +547,9 @@ 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. + /// Run all the attributes on each item in the crate in source-order. + /// Source-order is defined as running all child modules before their parent modules are run. + /// Child modules of a parent are run in order of their `mod foo;` declarations in the parent. pub(super) fn run_attributes( &mut self, traits: &BTreeMap, @@ -564,11 +590,11 @@ impl<'context> Elaborator<'context> { self.collect_attributes_on_functions(functions, &mut attributes_to_run); self.collect_attributes_on_modules(module_attributes, &mut attributes_to_run); - self.interner.attribute_order.sort_attributes_by_run_order(&mut attributes_to_run); + self.sort_attributes_by_run_order(&mut attributes_to_run); // run let mut generated_items = CollectedItems::default(); - for (attribute, item, args, context, span) in attributes_to_run { + for (attribute, item, args, context, span, _) in attributes_to_run { let location = Location::new(span, context.attribute_file); if let Err(error) = self.run_attribute(context, attribute, args, item, location, &mut generated_items) @@ -580,6 +606,14 @@ impl<'context> Elaborator<'context> { generated_items } + fn sort_attributes_by_run_order(&self, attributes: &mut CollectedAttributes) { + let module_order = self.def_maps[&self.crate_id].get_module_topological_order(); + + // Sort each attribute by (module, location in file) so that we can execute in + // the order they were defined in, running attributes in child modules first. + attributes.sort_by_key(|(_, _, _, _, span, module)| (module_order[module], span.start())); + } + fn collect_attributes_on_modules( &mut self, module_attributes: &[ModuleAttribute], @@ -662,50 +696,4 @@ impl<'context> Elaborator<'context> { _ => false, } } - - pub(super) fn register_attribute_order( - &mut self, - id: FuncId, - attributes: &[SecondaryAttribute], - ) { - let mut has_order = false; - - for attribute in attributes { - let (name, run_before) = match attribute { - SecondaryAttribute::RunBefore(name) => (name, true), - SecondaryAttribute::RunAfter(name) => (name, false), - _ => continue, - }; - - // Parse a path from #[run_before(path)] - let Some(path) = Parser::for_str(name).parse_path_no_turbofish() else { - todo!("function should be a path") - }; - - let definition_id = self.resolve_variable(path).id; - - match self.interner.definition(definition_id).kind { - DefinitionKind::Function(attribute_arg) => { - if attribute_arg == id { - todo!("Attribute cannot be run before or after itself"); - } - - has_order = true; - if run_before { - self.interner.attribute_order.add_ordering_constraint(attribute_arg, id); - } else { - self.interner.attribute_order.add_ordering_constraint(id, attribute_arg); - } - } - _ => { - todo!("path doesn't refer to a function") - } - } - } - - // If no ordering was specified, set it to the default stage. - if !has_order { - self.interner.attribute_order.run_in_default_stage(id); - } - } } diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index e87accb14db..aef0771c486 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -820,8 +820,6 @@ impl<'context> Elaborator<'context> { None }; - self.register_attribute_order(func_id, func.secondary_attributes()); - let attributes = func.secondary_attributes().iter(); let attributes = attributes.filter_map(|secondary_attribute| secondary_attribute.as_custom()); diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 60ee2c52842..af766d4d2cd 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -293,6 +293,31 @@ impl CrateDefMap { String::new() } } + + /// Return a topological ordering of each module such that any child modules + /// are before their parent modules. Sibling modules will respect the ordering + /// declared from their parent module (the `mod foo; mod bar;` declarations). + pub fn get_module_topological_order(&self) -> HashMap { + let mut ordering = HashMap::default(); + self.topologically_sort_modules(self.root, &mut 0, &mut ordering); + ordering + } + + fn topologically_sort_modules( + &self, + current: LocalModuleId, + index: &mut usize, + ordering: &mut HashMap, + ) { + let data = &self.modules[current.0]; + + for child in &data.child_declaration_order { + self.topologically_sort_modules(*child, index, ordering); + } + + ordering.insert(current, *index); + *index += 1; + } } /// Specifies a contract function and extra metadata that diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index fe6fe8285d3..06188f3920b 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -14,6 +14,11 @@ pub struct ModuleData { pub parent: Option, pub children: HashMap, + /// Each child in the order they were declared in the parent module. + /// E.g. for a module containing `mod foo; mod bar; mod baz` this would + /// be `vec![foo, bar, baz]`. + pub child_declaration_order: Vec, + /// Contains all definitions visible to the current module. This includes /// all definitions in self.definitions as well as all imported definitions. scope: ItemScope, @@ -47,6 +52,7 @@ impl ModuleData { ModuleData { parent, children: HashMap::new(), + child_declaration_order: Vec::new(), scope: ItemScope::default(), definitions: ItemScope::default(), location, @@ -73,6 +79,10 @@ impl ModuleData { ) -> Result<(), (Ident, Ident)> { self.scope.add_definition(name.clone(), visibility, item_id, trait_id)?; + if let ModuleDefId::ModuleId(child) = item_id { + self.child_declaration_order.push(child.local_id); + } + // definitions is a subset of self.scope so it is expected if self.scope.define_func_def // returns without error, so will self.definitions.define_func_def. self.definitions.add_definition(name, visibility, item_id, trait_id) diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index f6d81a860ec..4fb6037244d 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -832,8 +832,6 @@ impl Attribute { ["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs), ["use_callers_scope"] => Attribute::Secondary(SecondaryAttribute::UseCallersScope), ["allow", tag] => Attribute::Secondary(SecondaryAttribute::Allow(tag.to_string())), - ["run_before", f] => Attribute::Secondary(SecondaryAttribute::RunBefore(f.to_string())), - ["run_after", f] => Attribute::Secondary(SecondaryAttribute::RunAfter(f.to_string())), tokens => { tokens.iter().try_for_each(|token| validate(token))?; Attribute::Secondary(SecondaryAttribute::Meta(CustomAttribute { @@ -974,12 +972,6 @@ pub enum SecondaryAttribute { /// Allow chosen warnings to happen so they are silenced. Allow(String), - - /// Specifies this attribute should run before the attribute specified as an argument - RunBefore(String), - - /// Specifies this attribute should run after the attribute specified as an argument - RunAfter(String), } impl SecondaryAttribute { @@ -1005,8 +997,6 @@ impl SecondaryAttribute { SecondaryAttribute::Varargs => Some("varargs".to_string()), SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()), SecondaryAttribute::Allow(_) => Some("allow".to_string()), - SecondaryAttribute::RunBefore(_) => Some("run_before".to_string()), - SecondaryAttribute::RunAfter(_) => Some("run_after".to_string()), } } @@ -1042,8 +1032,6 @@ impl fmt::Display for SecondaryAttribute { SecondaryAttribute::Varargs => write!(f, "varargs"), SecondaryAttribute::UseCallersScope => write!(f, "use_callers_scope"), SecondaryAttribute::Allow(ref k) => write!(f, "allow({k})"), - SecondaryAttribute::RunBefore(attr) => write!(f, "#[run_before({attr})]"), - SecondaryAttribute::RunAfter(attr) => write!(f, "#[run_after({attr})]"), } } } @@ -1093,8 +1081,6 @@ impl AsRef for SecondaryAttribute { SecondaryAttribute::Meta(attribute) => &attribute.contents, SecondaryAttribute::Field(string) | SecondaryAttribute::Abi(string) - | SecondaryAttribute::RunBefore(string) - | SecondaryAttribute::RunAfter(string) | SecondaryAttribute::Allow(string) => string, SecondaryAttribute::ContractLibraryMethod => "", SecondaryAttribute::Export => "", diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 1cbb2057fd0..9d98b125e32 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -11,7 +11,6 @@ #![warn(clippy::semicolon_if_nothing_returned)] pub mod ast; -pub mod attribute_order; pub mod debug; pub mod elaborator; pub mod graph; diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index eba685d8f4f..ca7e0c6aa59 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -16,7 +16,6 @@ use rustc_hash::FxHashMap as HashMap; use crate::ast::{ ExpressionKind, Ident, LValue, Pattern, StatementKind, UnaryOp, UnresolvedTypeData, }; -use crate::attribute_order::AttributeGraph; use crate::graph::CrateId; use crate::hir::comptime; use crate::hir::def_collector::dc_crate::CompilationError; @@ -275,8 +274,6 @@ pub struct NodeInterner { /// Captures the documentation comments for each module, struct, trait, function, etc. pub(crate) doc_comments: HashMap>, - - pub(crate) attribute_order: AttributeGraph, } /// A dependency in the dependency graph may be a type or a definition. @@ -684,7 +681,6 @@ impl Default for NodeInterner { trait_impl_associated_types: HashMap::default(), usage_tracker: UsageTracker::default(), doc_comments: HashMap::default(), - attribute_order: AttributeGraph::default(), } } } diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index 383ca9d73ed..ff662b878ec 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -28,10 +28,6 @@ pub comptime fn unquote(code: Quoted) -> Quoted { pub comptime fn type_of(x: T) -> Type {} // docs:end:type_of -/// Stub function used so that other attributes can specify if they -/// want to run before or after any attributes without a stage annotation. -pub comptime fn default_stage() {} - // docs:start:derive_example // These are needed for the unconstrained hashmap we're using to store derive functions use crate::collections::umap::UHashMap; @@ -51,7 +47,6 @@ comptime mut global HANDLERS: UHashMap Quoted { // docs:end:derive let mut result = quote {}; @@ -71,7 +66,6 @@ pub comptime fn derive(s: StructDefinition, traits: [TraitDefinition]) -> Quoted // docs:start:derive_via // To register a handler for a trait, just add it to our handlers map // docs:start:derive_via_signature -#[run_before(derive)] pub comptime fn derive_via(t: TraitDefinition, f: DeriveFunction) { // docs:end:derive_via_signature HANDLERS.insert(t, f); From a16d2408c1f48046bb28eda16196e91b467cec3d Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 23 Oct 2024 16:01:51 -0500 Subject: [PATCH 05/15] Fix order of module attributes and add test --- .../noirc_frontend/src/elaborator/comptime.rs | 78 +++++++------------ .../src/hir/def_collector/dc_crate.rs | 4 +- .../noirc_frontend/src/hir/def_map/mod.rs | 4 +- compiler/noirc_frontend/src/lexer/token.rs | 2 +- .../attribute_order/Nargo.toml | 7 ++ .../attribute_order/src/a.nr | 13 ++++ .../attribute_order/src/a/a_child1.nr | 5 ++ .../attribute_order/src/a/a_child2.nr | 5 ++ .../attribute_order/src/b/b_child1.nr | 4 + .../attribute_order/src/b/b_child2.nr | 2 + .../attribute_order/src/b/mod.nr | 16 ++++ .../attribute_order/src/main.nr | 37 +++++++++ 12 files changed, 120 insertions(+), 57 deletions(-) create mode 100644 test_programs/compile_success_empty/attribute_order/Nargo.toml create mode 100644 test_programs/compile_success_empty/attribute_order/src/a.nr create mode 100644 test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr create mode 100644 test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr create mode 100644 test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr create mode 100644 test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr create mode 100644 test_programs/compile_success_empty/attribute_order/src/b/mod.nr create mode 100644 test_programs/compile_success_empty/attribute_order/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 57a19c2de7b..8664db316cb 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -40,8 +40,7 @@ pub(crate) struct AttributeContext { pub(crate) attribute_module: LocalModuleId, } -pub(crate) type CollectedAttributes = - Vec<(FuncId, Value, Vec, AttributeContext, Span, LocalModuleId)>; +pub(crate) type CollectedAttributes = Vec<(FuncId, Value, Vec, AttributeContext, Span)>; impl AttributeContext { pub(crate) fn new(file: FileId, module: LocalModuleId) -> Self { @@ -138,7 +137,6 @@ impl<'context> Elaborator<'context> { &mut self, attributes: &[SecondaryAttribute], item: Value, - span: Span, attribute_context: AttributeContext, attributes_to_run: &mut CollectedAttributes, ) { @@ -146,7 +144,6 @@ impl<'context> Elaborator<'context> { self.collect_comptime_attribute_on_item( attribute, &item, - span, attribute_context, attributes_to_run, ); @@ -157,7 +154,6 @@ impl<'context> Elaborator<'context> { &mut self, attribute: &SecondaryAttribute, item: &Value, - span: Span, attribute_context: AttributeContext, attributes_to_run: &mut CollectedAttributes, ) { @@ -166,7 +162,6 @@ impl<'context> Elaborator<'context> { if let Err(error) = this.collect_comptime_attribute_name_on_item( &attribute.contents, item.clone(), - span, attribute.contents_span, attribute_context, attributes_to_run, @@ -183,21 +178,17 @@ impl<'context> Elaborator<'context> { attribute: &str, item: Value, span: Span, - attribute_span: Span, attribute_context: AttributeContext, attributes_to_run: &mut CollectedAttributes, ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.attribute_file; self.local_module = attribute_context.attribute_module; - let location = Location::new(attribute_span, self.file); + let location = Location::new(span, self.file); let Some((function, arguments)) = Self::parse_attribute(attribute, location)? else { return Err(( - ResolverError::UnableToParseAttribute { - attribute: attribute.to_string(), - span: attribute_span, - } - .into(), + ResolverError::UnableToParseAttribute { attribute: attribute.to_string(), span } + .into(), self.file, )); }; @@ -212,11 +203,8 @@ impl<'context> Elaborator<'context> { HirExpression::Ident(ident, _) => ident.id, _ => { return Err(( - ResolverError::AttributeFunctionIsNotAPath { - function: function_string, - span: attribute_span, - } - .into(), + ResolverError::AttributeFunctionIsNotAPath { function: function_string, span } + .into(), self.file, )) } @@ -224,11 +212,7 @@ impl<'context> Elaborator<'context> { let Some(definition) = self.interner.try_definition(definition_id) else { return Err(( - ResolverError::AttributeFunctionNotInScope { - name: function_string, - span: attribute_span, - } - .into(), + ResolverError::AttributeFunctionNotInScope { name: function_string, span }.into(), self.file, )); }; @@ -237,14 +221,7 @@ impl<'context> Elaborator<'context> { return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file)); }; - attributes_to_run.push(( - function, - item, - arguments, - attribute_context, - span, - self.local_module, - )); + attributes_to_run.push((function, item, arguments, attribute_context, span)); Ok(()) } @@ -562,12 +539,10 @@ impl<'context> Elaborator<'context> { for (trait_id, trait_) in traits { let attributes = &trait_.trait_def.attributes; let item = Value::TraitDefinition(*trait_id); - let span = trait_.trait_def.span; let context = AttributeContext::new(trait_.file_id, trait_.module_id); self.collect_comptime_attributes_on_item( attributes, item, - span, context, &mut attributes_to_run, ); @@ -576,12 +551,10 @@ impl<'context> Elaborator<'context> { 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; let context = AttributeContext::new(struct_def.file_id, struct_def.module_id); self.collect_comptime_attributes_on_item( attributes, item, - span, context, &mut attributes_to_run, ); @@ -594,13 +567,21 @@ impl<'context> Elaborator<'context> { // run let mut generated_items = CollectedItems::default(); - for (attribute, item, args, context, span, _) in attributes_to_run { + for (attribute, item, args, context, span) in attributes_to_run { let location = Location::new(span, context.attribute_file); - if let Err(error) = - self.run_attribute(context, attribute, args, item, location, &mut generated_items) - { - self.errors.push(error); - } + + self.elaborate_in_comptime_context(|this| { + if let Err(error) = this.run_attribute( + context, + attribute, + args, + item, + location, + &mut generated_items, + ) { + this.errors.push(error); + } + }); } generated_items @@ -611,7 +592,9 @@ impl<'context> Elaborator<'context> { // Sort each attribute by (module, location in file) so that we can execute in // the order they were defined in, running attributes in child modules first. - attributes.sort_by_key(|(_, _, _, _, span, module)| (module_order[module], span.start())); + attributes.sort_by_key(|(_, _, _, ctx, span)| { + (module_order[&ctx.attribute_module], span.start()) + }); } fn collect_attributes_on_modules( @@ -624,7 +607,6 @@ impl<'context> Elaborator<'context> { let module_id = ModuleId { krate: self.crate_id, local_id }; let item = Value::ModuleDefinition(module_id); let attribute = &module_attribute.attribute; - let span = Span::default(); let context = AttributeContext { file: module_attribute.file_id, @@ -633,13 +615,7 @@ impl<'context> Elaborator<'context> { attribute_module: module_attribute.attribute_module_id, }; - self.collect_comptime_attribute_on_item( - attribute, - &item, - span, - context, - attributes_to_run, - ); + self.collect_comptime_attribute_on_item(attribute, &item, context, attributes_to_run); } } @@ -655,11 +631,9 @@ impl<'context> Elaborator<'context> { let context = AttributeContext::new(function_set.file_id, *local_module); let attributes = function.secondary_attributes(); let item = Value::FunctionDefinition(*function_id); - let span = function.span(); self.collect_comptime_attributes_on_item( attributes, item, - span, context, attributes_to_run, ); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 16fd43ba2a2..96cc0ce021e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -119,9 +119,11 @@ pub struct ModuleAttribute { pub file_id: FileId, // The module this attribute is attached to pub module_id: LocalModuleId, + // The file where the attribute exists (it could be the same as `file_id` - // or a different one if it's an inner attribute in a different file) + // or a different one if it is an outer attribute in the parent of the module it applies to) pub attribute_file_id: FileId, + // The module where the attribute is defined (similar to `attribute_file_id`, // it could be different than `module_id` for inner attributes) pub attribute_module_id: LocalModuleId, diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index af766d4d2cd..300baa00669 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -309,9 +309,7 @@ impl CrateDefMap { index: &mut usize, ordering: &mut HashMap, ) { - let data = &self.modules[current.0]; - - for child in &data.child_declaration_order { + for child in &self.modules[current.0].child_declaration_order { self.topologically_sort_modules(*child, index, ordering); } diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 4fb6037244d..70e75d6e3ff 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -676,7 +676,7 @@ impl Attributes { pub fn has_contract_library_method(&self) -> bool { self.secondary .iter() - .any(|attribute| attribute == &SecondaryAttribute::ContractLibraryMethod) + .any(|attribute| *attribute == SecondaryAttribute::ContractLibraryMethod) } pub fn is_test_function(&self) -> bool { diff --git a/test_programs/compile_success_empty/attribute_order/Nargo.toml b/test_programs/compile_success_empty/attribute_order/Nargo.toml new file mode 100644 index 00000000000..4471bed86dc --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "attribute_order" +type = "bin" +authors = [""] +compiler_version = ">=0.36.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/attribute_order/src/a.nr b/test_programs/compile_success_empty/attribute_order/src/a.nr new file mode 100644 index 00000000000..c2fbb5c5839 --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/a.nr @@ -0,0 +1,13 @@ +// This is run fifth, after each child module finishes even though +// the function comes before the module declarations in the source. +#[crate::assert_run_order_function(4)] +fn foo() {} + +mod a_child1; +mod a_child2; + +#[crate::assert_run_order_struct(5)] +struct Bar {} + +#[crate::assert_run_order_trait(6)] +trait Foo {} diff --git a/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr b/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr new file mode 100644 index 00000000000..9e19cb0256e --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr @@ -0,0 +1,5 @@ +#[crate::assert_run_order_function(0)] +fn foo(){} + +#[crate::assert_run_order_struct(1)] +struct Foo {} diff --git a/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr b/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr new file mode 100644 index 00000000000..0374ffb18c8 --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr @@ -0,0 +1,5 @@ +#[crate::assert_run_order_trait(2)] +trait Foo {} + +#[crate::assert_run_order_function(3)] +fn foo() {} diff --git a/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr b/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr new file mode 100644 index 00000000000..05dbdfab1c9 --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr @@ -0,0 +1,4 @@ +#[crate::assert_run_order_function(8)] +fn foo() {} + +#![crate::assert_run_order_module(9)] diff --git a/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr b/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr new file mode 100644 index 00000000000..7bfa294503a --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr @@ -0,0 +1,2 @@ +#[crate::assert_run_order_function(7)] +fn foo() {} diff --git a/test_programs/compile_success_empty/attribute_order/src/b/mod.nr b/test_programs/compile_success_empty/attribute_order/src/b/mod.nr new file mode 100644 index 00000000000..b4ad01fb6fa --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/b/mod.nr @@ -0,0 +1,16 @@ +// Declare these in reverse order to ensure the declaration +// order here is respected +mod b_child2; + +#[crate::assert_run_order_module(10)] +mod b_child1; + +#[crate::assert_run_order_function(11)] +fn foo() {} + +#![crate::assert_run_order_module(12)] + +#[crate::assert_run_order_function(13)] +fn bar() {} + +#![crate::assert_run_order_module(14)] diff --git a/test_programs/compile_success_empty/attribute_order/src/main.nr b/test_programs/compile_success_empty/attribute_order/src/main.nr new file mode 100644 index 00000000000..04270db7f46 --- /dev/null +++ b/test_programs/compile_success_empty/attribute_order/src/main.nr @@ -0,0 +1,37 @@ +mod a; +mod b; + +#[assert_run_order_function(15)] +fn main() { + assert_eq(attributes_run, 16); +} + +comptime mut global attributes_run: Field = 0; + +pub comptime fn assert_run_order_function(_f: FunctionDefinition, order: Field) { + let a = attributes_run; + println(f"{order} == {a}"); + assert_eq(order, attributes_run); + attributes_run += 1; +} + +pub comptime fn assert_run_order_struct(_s: StructDefinition, order: Field) { + let a = attributes_run; + println(f"{order} == {a}"); + assert_eq(order, attributes_run); + attributes_run += 1; +} + +pub comptime fn assert_run_order_trait(_t: TraitDefinition, order: Field) { + let a = attributes_run; + println(f"{order} == {a}"); + assert_eq(order, attributes_run); + attributes_run += 1; +} + +pub comptime fn assert_run_order_module(_m: Module, order: Field) { + let a = attributes_run; + println(f"{order} == {a}"); + assert_eq(order, attributes_run); + attributes_run += 1; +} From 6c2e724a686914a0d774a74475ef605c59aa1d83 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 23 Oct 2024 16:03:45 -0500 Subject: [PATCH 06/15] Remove printlns --- .../compile_success_empty/attribute_order/src/main.nr | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test_programs/compile_success_empty/attribute_order/src/main.nr b/test_programs/compile_success_empty/attribute_order/src/main.nr index 04270db7f46..0e9113302a5 100644 --- a/test_programs/compile_success_empty/attribute_order/src/main.nr +++ b/test_programs/compile_success_empty/attribute_order/src/main.nr @@ -9,29 +9,21 @@ fn main() { comptime mut global attributes_run: Field = 0; pub comptime fn assert_run_order_function(_f: FunctionDefinition, order: Field) { - let a = attributes_run; - println(f"{order} == {a}"); assert_eq(order, attributes_run); attributes_run += 1; } pub comptime fn assert_run_order_struct(_s: StructDefinition, order: Field) { - let a = attributes_run; - println(f"{order} == {a}"); assert_eq(order, attributes_run); attributes_run += 1; } pub comptime fn assert_run_order_trait(_t: TraitDefinition, order: Field) { - let a = attributes_run; - println(f"{order} == {a}"); assert_eq(order, attributes_run); attributes_run += 1; } pub comptime fn assert_run_order_module(_m: Module, order: Field) { - let a = attributes_run; - println(f"{order} == {a}"); assert_eq(order, attributes_run); attributes_run += 1; } From 1483dd99556b661472b371e8aaed58ebcf3a6e1e Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 11:06:04 -0500 Subject: [PATCH 07/15] Insert each item into the program as it is generated --- compiler/noirc_frontend/src/elaborator/comptime.rs | 10 ++++++---- compiler/noirc_frontend/src/elaborator/mod.rs | 12 +----------- .../compile_success_empty/attribute_order/src/a.nr | 6 +++--- .../attribute_order/src/a/a_child1.nr | 4 ++-- .../attribute_order/src/a/a_child2.nr | 4 ++-- .../attribute_order/src/b/b_child1.nr | 2 +- .../attribute_order/src/b/b_child2.nr | 2 +- .../attribute_order/src/b/mod.nr | 4 ++-- 8 files changed, 18 insertions(+), 26 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 8664db316cb..56223c2c216 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -533,7 +533,7 @@ impl<'context> Elaborator<'context> { types: &BTreeMap, functions: &[UnresolvedFunctions], module_attributes: &[ModuleAttribute], - ) -> CollectedItems { + ) { let mut attributes_to_run = Vec::new(); for (trait_id, trait_) in traits { @@ -566,10 +566,10 @@ impl<'context> Elaborator<'context> { self.sort_attributes_by_run_order(&mut attributes_to_run); // run - let mut generated_items = CollectedItems::default(); for (attribute, item, args, context, span) in attributes_to_run { let location = Location::new(span, context.attribute_file); + let mut generated_items = CollectedItems::default(); self.elaborate_in_comptime_context(|this| { if let Err(error) = this.run_attribute( context, @@ -582,9 +582,11 @@ impl<'context> Elaborator<'context> { this.errors.push(error); } }); - } - generated_items + if !generated_items.is_empty() { + self.elaborate_items(generated_items); + } + } } fn sort_attributes_by_run_order(&self, attributes: &mut CollectedAttributes) { diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index aef0771c486..5c80ffcb1d9 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -295,23 +295,13 @@ 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. - let generated_items = self.run_attributes( + self.run_attributes( &items.traits, &items.types, &items.functions, &items.module_attributes, ); - // After everything is collected, we can elaborate our generated items. - // It may be better to inline these within `items` entirely since elaborating them - // all here means any globals will not see these. Inlining them completely within `items` - // means we must be more careful about missing any additional items that need to be already - // elaborated. E.g. if a new struct is created, we've already passed the code path to - // elaborate them. - if !generated_items.is_empty() { - self.elaborate_items(generated_items); - } - for functions in items.functions { self.elaborate_functions(functions); } diff --git a/test_programs/compile_success_empty/attribute_order/src/a.nr b/test_programs/compile_success_empty/attribute_order/src/a.nr index c2fbb5c5839..663643f1288 100644 --- a/test_programs/compile_success_empty/attribute_order/src/a.nr +++ b/test_programs/compile_success_empty/attribute_order/src/a.nr @@ -1,13 +1,13 @@ // This is run fifth, after each child module finishes even though // the function comes before the module declarations in the source. #[crate::assert_run_order_function(4)] -fn foo() {} +pub fn foo() {} mod a_child1; mod a_child2; #[crate::assert_run_order_struct(5)] -struct Bar {} +pub struct Bar {} #[crate::assert_run_order_trait(6)] -trait Foo {} +pub trait Foo {} diff --git a/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr b/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr index 9e19cb0256e..0806308c077 100644 --- a/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr +++ b/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr @@ -1,5 +1,5 @@ #[crate::assert_run_order_function(0)] -fn foo(){} +pub fn foo(){} #[crate::assert_run_order_struct(1)] -struct Foo {} +pub struct Foo {} diff --git a/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr b/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr index 0374ffb18c8..3548f4450a5 100644 --- a/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr +++ b/test_programs/compile_success_empty/attribute_order/src/a/a_child2.nr @@ -1,5 +1,5 @@ #[crate::assert_run_order_trait(2)] -trait Foo {} +pub trait Foo {} #[crate::assert_run_order_function(3)] -fn foo() {} +pub fn foo() {} diff --git a/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr b/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr index 05dbdfab1c9..a8e7e898ec1 100644 --- a/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr +++ b/test_programs/compile_success_empty/attribute_order/src/b/b_child1.nr @@ -1,4 +1,4 @@ #[crate::assert_run_order_function(8)] -fn foo() {} +pub fn foo() {} #![crate::assert_run_order_module(9)] diff --git a/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr b/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr index 7bfa294503a..8e6d967707a 100644 --- a/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr +++ b/test_programs/compile_success_empty/attribute_order/src/b/b_child2.nr @@ -1,2 +1,2 @@ #[crate::assert_run_order_function(7)] -fn foo() {} +pub fn foo() {} diff --git a/test_programs/compile_success_empty/attribute_order/src/b/mod.nr b/test_programs/compile_success_empty/attribute_order/src/b/mod.nr index b4ad01fb6fa..cefb066d1ca 100644 --- a/test_programs/compile_success_empty/attribute_order/src/b/mod.nr +++ b/test_programs/compile_success_empty/attribute_order/src/b/mod.nr @@ -6,11 +6,11 @@ mod b_child2; mod b_child1; #[crate::assert_run_order_function(11)] -fn foo() {} +pub fn foo() {} #![crate::assert_run_order_module(12)] #[crate::assert_run_order_function(13)] -fn bar() {} +pub fn bar() {} #![crate::assert_run_order_module(14)] From c81e5082e635076f3c9f2cc34bec4a8636432e57 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 11:12:33 -0500 Subject: [PATCH 08/15] Fix tests --- .../compile_success_empty/derive_impl/src/main.nr | 8 +++++--- test_programs/execution_success/derive/src/main.nr | 13 ++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/test_programs/compile_success_empty/derive_impl/src/main.nr b/test_programs/compile_success_empty/derive_impl/src/main.nr index db3f78a35c6..4396169235d 100644 --- a/test_programs/compile_success_empty/derive_impl/src/main.nr +++ b/test_programs/compile_success_empty/derive_impl/src/main.nr @@ -20,15 +20,17 @@ comptime fn derive_default(typ: StructDefinition) -> Quoted { } } +// Bar needs to be defined before Foo so that its impls are defined before +// Foo's are. +#[derive_default] +struct Bar {} + #[derive_default] struct Foo { x: Field, y: Bar, } -#[derive_default] -struct Bar {} - comptime fn make_field_exprs(fields: [(Quoted, Type)]) -> [Quoted] { let mut result = &[]; for my_field in fields { diff --git a/test_programs/execution_success/derive/src/main.nr b/test_programs/execution_success/derive/src/main.nr index 6900aa6aead..f7d4f6b607a 100644 --- a/test_programs/execution_success/derive/src/main.nr +++ b/test_programs/execution_success/derive/src/main.nr @@ -25,6 +25,14 @@ comptime fn derive_do_nothing(s: StructDefinition) -> Quoted { // Test stdlib derive fns & multiple traits // - We can derive Ord and Hash even though std::cmp::Ordering and std::hash::Hasher aren't imported +// - We need to define MyOtherOtherStruct first since MyOtherStruct references it as a field and +// attributes are run in reading order. If it were defined afterward, the derived Eq impl for MyOtherStruct +// would error that MyOtherOtherStruct doesn't (yet) implement Eq. +#[derive(Eq, Default, Hash, Ord)] +struct MyOtherOtherStruct { + x: T, +} + #[derive(Eq, Default, Hash, Ord)] struct MyOtherStruct { field1: A, @@ -32,11 +40,6 @@ struct MyOtherStruct { field3: MyOtherOtherStruct, } -#[derive(Eq, Default, Hash, Ord)] -struct MyOtherOtherStruct { - x: T, -} - #[derive(Eq, Default, Hash, Ord)] struct EmptyStruct {} From c921fc462bf69fab4fbc9a40a3e7102ef12755b2 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 11:37:03 -0500 Subject: [PATCH 09/15] Add another test --- .../Nargo.toml | 7 +++++++ .../src/main.nr | 11 +++++++++++ 2 files changed, 18 insertions(+) create mode 100644 test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/Nargo.toml create mode 100644 test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr diff --git a/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/Nargo.toml b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/Nargo.toml new file mode 100644 index 00000000000..ad71067a58f --- /dev/null +++ b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "attribute_sees_results_of_previous_attribute" +type = "bin" +authors = [""] +compiler_version = ">=0.34.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr new file mode 100644 index 00000000000..c92dfa7379b --- /dev/null +++ b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr @@ -0,0 +1,11 @@ +#[derive(Eq)] +pub struct Foo {} + +#[check_eq_derived_for_foo] +unconstrained fn main() {} + +comptime fn check_eq_derived_for_foo(_f: FunctionDefinition) { + let foo = quote[Foo].as_type(); + let eq = quote[Eq].as_trait_constraint(); + assert(foo.implements(eq)); +} From e11f404891125ade9eb8834f0b333288aec5da9f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 12:13:26 -0500 Subject: [PATCH 10/15] Revert token contents change --- .../noirc_frontend/src/elaborator/comptime.rs | 14 ++++---- .../noirc_frontend/src/elaborator/patterns.rs | 2 +- compiler/noirc_frontend/src/lexer/token.rs | 34 +++++++++---------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 56223c2c216..3d46ec7e0a7 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -29,21 +29,21 @@ use crate::{ use super::{Elaborator, FunctionContext, ResolverMeta}; #[derive(Debug, Copy, Clone)] -pub(crate) struct AttributeContext { +struct AttributeContext { // The file where generated items should be added - pub(crate) file: FileId, + file: FileId, // The module where generated items should be added - pub(crate) module: LocalModuleId, + module: LocalModuleId, // The file where the attribute is located - pub(crate) attribute_file: FileId, + attribute_file: FileId, // The module where the attribute is located - pub(crate) attribute_module: LocalModuleId, + attribute_module: LocalModuleId, } -pub(crate) type CollectedAttributes = Vec<(FuncId, Value, Vec, AttributeContext, Span)>; +type CollectedAttributes = Vec<(FuncId, Value, Vec, AttributeContext, Span)>; impl AttributeContext { - pub(crate) fn new(file: FileId, module: LocalModuleId) -> Self { + fn new(file: FileId, module: LocalModuleId) -> Self { Self { file, module, attribute_file: file, attribute_module: module } } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 5226a7402c9..d55011f98a1 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -498,7 +498,7 @@ impl<'context> Elaborator<'context> { (id, typ) } - pub fn resolve_variable(&mut self, path: Path) -> HirIdent { + fn resolve_variable(&mut self, path: Path) -> HirIdent { if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) { if let Some(error) = trait_path_resolution.error { self.push_err(error); diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 70e75d6e3ff..8f05832d26d 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -676,7 +676,7 @@ impl Attributes { pub fn has_contract_library_method(&self) -> bool { self.secondary .iter() - .any(|attribute| *attribute == SecondaryAttribute::ContractLibraryMethod) + .any(|attribute| attribute == &SecondaryAttribute::ContractLibraryMethod) } pub fn is_test_function(&self) -> bool { @@ -1012,27 +1012,27 @@ impl SecondaryAttribute { } pub(crate) fn contents(&self) -> String { - self.to_string() + match self { + SecondaryAttribute::Deprecated(None) => "deprecated".to_string(), + SecondaryAttribute::Deprecated(Some(ref note)) => { + format!("deprecated({note:?})") + } + SecondaryAttribute::Tag(ref attribute) => format!("'{}", attribute.contents), + SecondaryAttribute::Meta(ref attribute) => attribute.contents.to_string(), + SecondaryAttribute::ContractLibraryMethod => "contract_library_method".to_string(), + SecondaryAttribute::Export => "export".to_string(), + SecondaryAttribute::Field(ref k) => format!("field({k})"), + SecondaryAttribute::Abi(ref k) => format!("abi({k})"), + SecondaryAttribute::Varargs => "varargs".to_string(), + SecondaryAttribute::UseCallersScope => "use_callers_scope".to_string(), + SecondaryAttribute::Allow(ref k) => format!("allow({k})"), + } } } impl fmt::Display for SecondaryAttribute { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - SecondaryAttribute::Deprecated(None) => write!(f, "deprecated"), - SecondaryAttribute::Deprecated(Some(ref note)) => { - write!(f, "deprecated({note:?})") - } - SecondaryAttribute::Tag(ref attribute) => write!(f, "'{}", attribute.contents), - SecondaryAttribute::Meta(ref attribute) => write!(f, "{}", attribute.contents), - SecondaryAttribute::ContractLibraryMethod => write!(f, "contract_library_method"), - SecondaryAttribute::Export => write!(f, "export"), - SecondaryAttribute::Field(ref k) => write!(f, "field({k})"), - SecondaryAttribute::Abi(ref k) => write!(f, "abi({k})"), - SecondaryAttribute::Varargs => write!(f, "varargs"), - SecondaryAttribute::UseCallersScope => write!(f, "use_callers_scope"), - SecondaryAttribute::Allow(ref k) => write!(f, "allow({k})"), - } + write!(f, "#[{}]", self.contents()) } } From 894b9622b1185567b94183834c6023e1d274d677 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 12:17:33 -0500 Subject: [PATCH 11/15] Format --- .../attribute_sees_result_of_previous_attribute/src/main.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr index c92dfa7379b..3e396dd92d4 100644 --- a/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr +++ b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr @@ -5,7 +5,7 @@ pub struct Foo {} unconstrained fn main() {} comptime fn check_eq_derived_for_foo(_f: FunctionDefinition) { - let foo = quote[Foo].as_type(); - let eq = quote[Eq].as_trait_constraint(); + let foo = quote {Foo}.as_type(); + let eq = quote {Eq}.as_trait_constraint(); assert(foo.implements(eq)); } From a95925d07660979988c6954e63ebd53ad6dee00f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 12:21:17 -0500 Subject: [PATCH 12/15] Fmt again --- .../compile_success_empty/attribute_order/src/a/a_child1.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr b/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr index 0806308c077..834bbd43fb5 100644 --- a/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr +++ b/test_programs/compile_success_empty/attribute_order/src/a/a_child1.nr @@ -1,5 +1,5 @@ #[crate::assert_run_order_function(0)] -pub fn foo(){} +pub fn foo() {} #[crate::assert_run_order_struct(1)] pub struct Foo {} From ecd59740592ac4afc690dd85d6fe6176b26354cb Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 12:41:43 -0500 Subject: [PATCH 13/15] Add inline submodule to test --- .../attribute_order/src/b/mod.nr | 19 ++++++++++++++----- .../attribute_order/src/main.nr | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/test_programs/compile_success_empty/attribute_order/src/b/mod.nr b/test_programs/compile_success_empty/attribute_order/src/b/mod.nr index cefb066d1ca..77df04e15a9 100644 --- a/test_programs/compile_success_empty/attribute_order/src/b/mod.nr +++ b/test_programs/compile_success_empty/attribute_order/src/b/mod.nr @@ -2,15 +2,24 @@ // order here is respected mod b_child2; -#[crate::assert_run_order_module(10)] +#[crate::assert_run_order_module(12)] mod b_child1; -#[crate::assert_run_order_function(11)] +#[crate::assert_run_order_function(13)] pub fn foo() {} -#![crate::assert_run_order_module(12)] +#![crate::assert_run_order_module(14)] -#[crate::assert_run_order_function(13)] +// Test inline submodules as well +#[crate::assert_run_order_module(15)] +mod b_child3 { + #![crate::assert_run_order_module(10)] + + #[crate::assert_run_order_function(11)] + pub fn foo() {} +} + +#[crate::assert_run_order_function(16)] pub fn bar() {} -#![crate::assert_run_order_module(14)] +#![crate::assert_run_order_module(17)] diff --git a/test_programs/compile_success_empty/attribute_order/src/main.nr b/test_programs/compile_success_empty/attribute_order/src/main.nr index 0e9113302a5..9d5ba32b58e 100644 --- a/test_programs/compile_success_empty/attribute_order/src/main.nr +++ b/test_programs/compile_success_empty/attribute_order/src/main.nr @@ -1,9 +1,9 @@ mod a; mod b; -#[assert_run_order_function(15)] +#[assert_run_order_function(18)] fn main() { - assert_eq(attributes_run, 16); + assert_eq(attributes_run, 19); } comptime mut global attributes_run: Field = 0; From b5ecb1878d71baeaf910fa7b3ca1533b8eeffaa2 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 24 Oct 2024 12:55:24 -0500 Subject: [PATCH 14/15] Fix test --- .../attribute_sees_result_of_previous_attribute/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr index 3e396dd92d4..561c3e12e0c 100644 --- a/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr +++ b/test_programs/compile_success_empty/attribute_sees_result_of_previous_attribute/src/main.nr @@ -2,7 +2,7 @@ pub struct Foo {} #[check_eq_derived_for_foo] -unconstrained fn main() {} +fn main() {} comptime fn check_eq_derived_for_foo(_f: FunctionDefinition) { let foo = quote {Foo}.as_type(); From 24543782a0a3d5bad5af601da4eb1c90a7ce0300 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 28 Oct 2024 16:12:32 -0500 Subject: [PATCH 15/15] Docs & fmt stdlib --- docs/docs/noir/concepts/comptime.md | 121 +++++++++++++++--- noir_stdlib/src/array/check_shuffle.nr | 2 +- noir_stdlib/src/bigint.nr | 2 +- noir_stdlib/src/cmp.nr | 2 +- noir_stdlib/src/collections/map.nr | 6 +- noir_stdlib/src/collections/umap.nr | 4 +- noir_stdlib/src/ec/consts/te.nr | 2 +- noir_stdlib/src/ec/montcurve.nr | 10 +- noir_stdlib/src/ec/swcurve.nr | 8 +- noir_stdlib/src/ec/tecurve.nr | 8 +- noir_stdlib/src/eddsa.nr | 2 +- noir_stdlib/src/embedded_curve_ops.nr | 2 +- noir_stdlib/src/field/bn254.nr | 2 +- noir_stdlib/src/field/mod.nr | 2 +- noir_stdlib/src/hash/mimc.nr | 2 +- noir_stdlib/src/hash/mod.nr | 2 +- noir_stdlib/src/hash/poseidon/bn254/consts.nr | 2 +- noir_stdlib/src/hash/poseidon/mod.nr | 2 +- noir_stdlib/src/hash/poseidon2.nr | 2 +- noir_stdlib/src/meta/expr.nr | 4 +- noir_stdlib/src/meta/mod.nr | 39 +++++- noir_stdlib/src/meta/trait_constraint.nr | 2 +- noir_stdlib/src/meta/trait_def.nr | 2 +- noir_stdlib/src/ops/mod.nr | 4 +- noir_stdlib/src/option.nr | 4 +- noir_stdlib/src/prelude.nr | 12 +- noir_stdlib/src/uint128.nr | 4 +- 27 files changed, 187 insertions(+), 67 deletions(-) diff --git a/docs/docs/noir/concepts/comptime.md b/docs/docs/noir/concepts/comptime.md index cea3a896c17..7c3f3e7f3d9 100644 --- a/docs/docs/noir/concepts/comptime.md +++ b/docs/docs/noir/concepts/comptime.md @@ -41,7 +41,7 @@ Note that while in a `comptime` context, any runtime variables _local to the cur Evaluation rules of `comptime` follows the normal unconstrained evaluation rules for other Noir code. There are a few things to note though: - Certain built-in functions may not be available, although more may be added over time. -- Evaluation order of global items is currently unspecified. For example, given the following two functions we can't guarantee +- Evaluation order of `comptime {}` blocks within global items is currently unspecified. For example, given the following two functions we can't guarantee which `println` will execute first. The ordering of the two printouts will be arbitrary, but should be stable across multiple compilations with the same `nargo` version as long as the program is also unchanged. ```rust @@ -56,11 +56,14 @@ fn two() { - Since evaluation order is unspecified, care should be taken when using mutable globals so that they do not rely on a particular ordering. For example, using globals to generate unique ids should be fine but relying on certain ids always being produced (especially after edits to the program) should be avoided. -- Although most ordering of globals is unspecified, two are: +- Although the ordering of comptime code is usually unspecified, there are cases where it is: - Dependencies of a crate will always be evaluated before the dependent crate. - - Any annotations on a function will be run before the function itself is resolved. This is to allow the annotation to modify the function if necessary. Note that if the + - Any attributes on a function will be run before the function body is resolved. This is to allow the attribute to modify the function if necessary. Note that if the function itself was called at compile-time previously, it will already be resolved and cannot be modified. To prevent accidentally calling functions you wish to modify - at compile-time, it may be helpful to sort your `comptime` annotation functions into a different crate along with any dependencies they require. + at compile-time, it may be helpful to sort your `comptime` annotation functions into a different submodule crate along with any dependencies they require. + - Unlike raw `comptime {}` blocks, attributes on top-level items in the program do have a set evaluation order. Attributes within a module are evaluated top-down, and attributes + in different modules are evaluated submodule-first. Sibling modules to the same parent module are evaluated in order of the module declarations (`mod foo; mod bar;`) in their + parent module. ## Lowering @@ -89,7 +92,7 @@ fn main() { } ``` -Not all types of values can be lowered. For example, `Type`s and `TypeDefinition`s (among other types) cannot be lowered at all. +Not all types of values can be lowered. For example, references, `Type`s, and `TypeDefinition`s (among other types) cannot be lowered at all. ```rust fn main() { @@ -100,6 +103,19 @@ fn main() { comptime fn get_type() -> Type { ... } ``` +Values of certain types may also change type when they are lowered. For example, a comptime format string will already be +formatted, and thus lowers into a runtime string instead: + +```rust +fn main() { + let foo = comptime { + let i = 2; + f"i = {i}" + }; + assert_eq(foo, "i = 2"); +} +``` + --- # (Quasi) Quote @@ -121,6 +137,19 @@ Calling such a function at compile-time without `!` will just return the `Quoted For those familiar with quoting from other languages (primarily lisps), Noir's `quote` is actually a _quasiquote_. This means we can escape the quoting by using the unquote operator to splice values in the middle of quoted code. +In addition to curly braces, you can also use square braces for the quote operator: + +```rust +comptime { + let q1 = quote { 1 }; + let q2 = quote [ 2 ]; + assert_eq(q1, q2); + + // Square braces can be used to quote mismatched curly braces if needed + let _ = quote[}]; +} +``` + # Unquote The unquote operator `$` is usable within a `quote` expression. @@ -149,7 +178,7 @@ If it is an expression (even a parenthesized one), it will do nothing. Most like Unquoting can also be avoided by escaping the `$` with a backslash: -``` +```rust comptime { let x = quote { 1 + 2 }; @@ -158,26 +187,48 @@ comptime { } ``` +## Combining Tokens + +Note that `Quoted` is internally a series of separate tokens, and that all unquoting does is combine these token vectors, code +that appears to append like a string actually appends like a vector internally: + +```rust +comptime { + let x = 3; + let q = quote { foo$x }; // This is [foo, 3], not [foo3] + + // Spaces are ignored in general, they're never part of a token + assert_eq(q, quote { foo 3 }); +} +``` + +If you do want string semantics, you can use format strings then convert back to a `Quoted` value with `.quoted_contents()`. +Note that formatting a quoted value with multiple tokens will always insert a space between each token. If this is +undesired, you'll need to only operate on quoted values containing a single token. To do this, you can iterate +over each token of a larger quoted value with `.tokens()`: + +#include_code concatenate-example noir_stdlib/src/meta/mod.nr rust + --- -# Annotations +# Attributes -Annotations provide a way to run a `comptime` function on an item in the program. -When you use an annotation, the function with the same name will be called with that item as an argument: +Attributes provide a way to run a `comptime` function on an item in the program. +When you use an attribute, the function with the same name will be called with that item as an argument: ```rust -#[my_struct_annotation] +#[my_struct_attribute] struct Foo {} -comptime fn my_struct_annotation(s: StructDefinition) { - println("Called my_struct_annotation!"); +comptime fn my_struct_attribute(s: StructDefinition) { + println("Called my_struct_attribute!"); } -#[my_function_annotation] +#[my_function_attribute] fn foo() {} -comptime fn my_function_annotation(f: FunctionDefinition) { - println("Called my_function_annotation!"); +comptime fn my_function_attribute(f: FunctionDefinition) { + println("Called my_function_attribute!"); } ``` @@ -188,17 +239,49 @@ For example, this is the mechanism used to insert additional trait implementatio #include_code derive-field-count-example noir_stdlib/src/meta/mod.nr rust -## Calling annotations with additional arguments +## Calling attributes with additional arguments -Arguments may optionally be given to annotations. -When this is done, these additional arguments are passed to the annotation function after the item argument. +Arguments may optionally be given to attributes. +When this is done, these additional arguments are passed to the attribute function after the item argument. #include_code annotation-arguments-example noir_stdlib/src/meta/mod.nr rust -We can also take any number of arguments by adding the `varargs` annotation: +We can also take any number of arguments by adding the `varargs` attribute: #include_code annotation-varargs-example noir_stdlib/src/meta/mod.nr rust +## Attribute Evaluation Order + +Unlike the evaluation order of stray `comptime {}` blocks within functions, attributes have a well-defined evaluation +order. Within a module, attributes are evaluated top to bottom. Between modules, attributes in child modules are evaluated +first. Attributes in sibling modules are resolved following the `mod foo; mod bar;` declaration order within their parent +modules. + +```rust +mod foo; // attributes in foo are run first +mod bar; // followed by attributes in bar + +// followed by any attributes in the parent module +#[derive(Eq)] +struct Baz {} +``` + +Note that because of this evaluation order, you may get an error trying to derive a trait for a struct whose fields +have not yet had the trait derived already: + +```rust +// Error! `Bar` field of `Foo` does not (yet) implement Eq! +#[derive(Eq)] +struct Foo { + bar: Bar +} + +#[derive(Eq)] +struct Bar {} +``` + +In this case, the issue can be resolved by rearranging the structs. + --- # Comptime API diff --git a/noir_stdlib/src/array/check_shuffle.nr b/noir_stdlib/src/array/check_shuffle.nr index 82028d487c7..2e8c227feee 100644 --- a/noir_stdlib/src/array/check_shuffle.nr +++ b/noir_stdlib/src/array/check_shuffle.nr @@ -59,8 +59,8 @@ where } mod test { - use super::check_shuffle; use crate::cmp::Eq; + use super::check_shuffle; struct CompoundStruct { a: bool, diff --git a/noir_stdlib/src/bigint.nr b/noir_stdlib/src/bigint.nr index 203ff90d444..be072257be3 100644 --- a/noir_stdlib/src/bigint.nr +++ b/noir_stdlib/src/bigint.nr @@ -1,5 +1,5 @@ -use crate::ops::{Add, Sub, Mul, Div}; use crate::cmp::Eq; +use crate::ops::{Add, Div, Mul, Sub}; global bn254_fq = &[ 0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97, diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr index 10be6e7b867..ae515150a4d 100644 --- a/noir_stdlib/src/cmp.nr +++ b/noir_stdlib/src/cmp.nr @@ -537,7 +537,7 @@ where } mod cmp_tests { - use crate::cmp::{min, max}; + use crate::cmp::{max, min}; #[test] fn sanity_check_min() { diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index cd203c43ad3..b46bfa837fb 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -1,8 +1,8 @@ use crate::cmp::Eq; -use crate::option::Option; -use crate::default::Default; -use crate::hash::{Hash, Hasher, BuildHasher}; use crate::collections::bounded_vec::BoundedVec; +use crate::default::Default; +use crate::hash::{BuildHasher, Hash, Hasher}; +use crate::option::Option; // We use load factor alpha_max = 0.75. // Upon exceeding it, assert will fail in order to inform the user diff --git a/noir_stdlib/src/collections/umap.nr b/noir_stdlib/src/collections/umap.nr index 9b72b6173ca..3e074551e9d 100644 --- a/noir_stdlib/src/collections/umap.nr +++ b/noir_stdlib/src/collections/umap.nr @@ -1,7 +1,7 @@ use crate::cmp::Eq; -use crate::option::Option; use crate::default::Default; -use crate::hash::{Hash, Hasher, BuildHasher}; +use crate::hash::{BuildHasher, Hash, Hasher}; +use crate::option::Option; // An unconstrained hash table with open addressing and quadratic probing. // Note that "unconstrained" here means that almost all operations on this diff --git a/noir_stdlib/src/ec/consts/te.nr b/noir_stdlib/src/ec/consts/te.nr index 561c16e846a..150eb849947 100644 --- a/noir_stdlib/src/ec/consts/te.nr +++ b/noir_stdlib/src/ec/consts/te.nr @@ -1,5 +1,5 @@ -use crate::ec::tecurve::affine::Point as TEPoint; use crate::ec::tecurve::affine::Curve as TECurve; +use crate::ec::tecurve::affine::Point as TEPoint; pub struct BabyJubjub { pub curve: TECurve, diff --git a/noir_stdlib/src/ec/montcurve.nr b/noir_stdlib/src/ec/montcurve.nr index 6c83feb1607..239585ba13f 100644 --- a/noir_stdlib/src/ec/montcurve.nr +++ b/noir_stdlib/src/ec/montcurve.nr @@ -3,16 +3,16 @@ pub mod affine { // Points are represented by two-dimensional Cartesian coordinates. // All group operations are induced by those of the corresponding Twisted Edwards curve. // See e.g. for details on the correspondences. + use crate::cmp::Eq; + use crate::ec::is_square; use crate::ec::montcurve::curvegroup; + use crate::ec::safe_inverse; + use crate::ec::sqrt; use crate::ec::swcurve::affine::Curve as SWCurve; use crate::ec::swcurve::affine::Point as SWPoint; use crate::ec::tecurve::affine::Curve as TECurve; use crate::ec::tecurve::affine::Point as TEPoint; - use crate::ec::is_square; - use crate::ec::safe_inverse; - use crate::ec::sqrt; use crate::ec::ZETA; - use crate::cmp::Eq; // Curve specification pub struct Curve { // Montgomery Curve configuration (ky^2 = x^3 + j*x^2 + x) @@ -222,12 +222,12 @@ pub mod curvegroup { // Points are represented by three-dimensional projective (homogeneous) coordinates. // All group operations are induced by those of the corresponding Twisted Edwards curve. // See e.g. for details on the correspondences. + use crate::cmp::Eq; use crate::ec::montcurve::affine; use crate::ec::swcurve::curvegroup::Curve as SWCurve; use crate::ec::swcurve::curvegroup::Point as SWPoint; use crate::ec::tecurve::curvegroup::Curve as TECurve; use crate::ec::tecurve::curvegroup::Point as TEPoint; - use crate::cmp::Eq; pub struct Curve { // Montgomery Curve configuration (ky^2 z = x*(x^2 + j*x*z + z*z)) pub j: Field, diff --git a/noir_stdlib/src/ec/swcurve.nr b/noir_stdlib/src/ec/swcurve.nr index 145b2506f73..d9c1cf8c8c7 100644 --- a/noir_stdlib/src/ec/swcurve.nr +++ b/noir_stdlib/src/ec/swcurve.nr @@ -3,11 +3,11 @@ pub mod affine { // Points are represented by two-dimensional Cartesian coordinates. // Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates // for reasons of efficiency, cf. . - use crate::ec::swcurve::curvegroup; - use crate::ec::safe_inverse; + use crate::cmp::Eq; use crate::ec::is_square; + use crate::ec::safe_inverse; use crate::ec::sqrt; - use crate::cmp::Eq; + use crate::ec::swcurve::curvegroup; // Curve specification pub struct Curve { // Short Weierstrass curve @@ -190,8 +190,8 @@ pub mod curvegroup { // CurveGroup representation of Weierstrass curves // Points are represented by three-dimensional Jacobian coordinates. // See for details. - use crate::ec::swcurve::affine; use crate::cmp::Eq; + use crate::ec::swcurve::affine; // Curve specification pub struct Curve { // Short Weierstrass curve diff --git a/noir_stdlib/src/ec/tecurve.nr b/noir_stdlib/src/ec/tecurve.nr index 0088896015d..8512413c831 100644 --- a/noir_stdlib/src/ec/tecurve.nr +++ b/noir_stdlib/src/ec/tecurve.nr @@ -4,12 +4,12 @@ pub mod affine { // Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates // for reasons of efficiency. // See for details. - use crate::ec::tecurve::curvegroup; + use crate::cmp::Eq; use crate::ec::montcurve::affine::Curve as MCurve; use crate::ec::montcurve::affine::Point as MPoint; use crate::ec::swcurve::affine::Curve as SWCurve; use crate::ec::swcurve::affine::Point as SWPoint; - use crate::cmp::Eq; + use crate::ec::tecurve::curvegroup; // Curve specification pub struct Curve { // Twisted Edwards curve @@ -197,12 +197,12 @@ pub mod curvegroup { // CurveGroup coordinate representation of Twisted Edwards curves // Points are represented by four-dimensional projective coordinates, viz. extended Twisted Edwards coordinates. // See section 3 of for details. - use crate::ec::tecurve::affine; + use crate::cmp::Eq; use crate::ec::montcurve::curvegroup::Curve as MCurve; use crate::ec::montcurve::curvegroup::Point as MPoint; use crate::ec::swcurve::curvegroup::Curve as SWCurve; use crate::ec::swcurve::curvegroup::Point as SWPoint; - use crate::cmp::Eq; + use crate::ec::tecurve::affine; // Curve specification pub struct Curve { // Twisted Edwards curve diff --git a/noir_stdlib/src/eddsa.nr b/noir_stdlib/src/eddsa.nr index 89a0b05b072..c049b7abbb5 100644 --- a/noir_stdlib/src/eddsa.nr +++ b/noir_stdlib/src/eddsa.nr @@ -1,8 +1,8 @@ +use crate::default::Default; use crate::ec::consts::te::baby_jubjub; use crate::ec::tecurve::affine::Point as TEPoint; use crate::hash::Hasher; use crate::hash::poseidon::PoseidonHasher; -use crate::default::Default; // Returns true if signature is valid pub fn eddsa_poseidon_verify( diff --git a/noir_stdlib/src/embedded_curve_ops.nr b/noir_stdlib/src/embedded_curve_ops.nr index dd5e4285c00..6b225ee18b2 100644 --- a/noir_stdlib/src/embedded_curve_ops.nr +++ b/noir_stdlib/src/embedded_curve_ops.nr @@ -1,5 +1,5 @@ -use crate::ops::arith::{Add, Sub, Neg}; use crate::cmp::Eq; +use crate::ops::arith::{Add, Neg, Sub}; /// A point on the embedded elliptic curve /// By definition, the base field of the embedded curve is the scalar field of the proof system curve, i.e the Noir Field. diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index 356b47e63cf..9642c2aa1b8 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -141,7 +141,7 @@ pub fn lt(a: Field, b: Field) -> bool { mod tests { // TODO: Allow imports from "super" use crate::field::bn254::{ - decompose, compute_lt, assert_gt, gt, TWO_POW_128, compute_lte, PLO, PHI, + assert_gt, compute_lt, compute_lte, decompose, gt, PHI, PLO, TWO_POW_128, }; #[test] diff --git a/noir_stdlib/src/field/mod.nr b/noir_stdlib/src/field/mod.nr index 915ea8f939e..b632cf1f7a2 100644 --- a/noir_stdlib/src/field/mod.nr +++ b/noir_stdlib/src/field/mod.nr @@ -1,6 +1,6 @@ pub mod bn254; -use bn254::lt as bn254_lt; use crate::runtime::is_unconstrained; +use bn254::lt as bn254_lt; impl Field { /// Asserts that `self` can be represented in `bit_size` bits. diff --git a/noir_stdlib/src/hash/mimc.nr b/noir_stdlib/src/hash/mimc.nr index 6045ae3dbdb..55c06964122 100644 --- a/noir_stdlib/src/hash/mimc.nr +++ b/noir_stdlib/src/hash/mimc.nr @@ -1,5 +1,5 @@ -use crate::hash::Hasher; use crate::default::Default; +use crate::hash::Hasher; // mimc-p/p implementation // constants are (publicly generated) random numbers, for instance using keccak as a ROM. diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index 609017d70aa..15112757312 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -6,11 +6,11 @@ pub mod sha256; pub mod sha512; use crate::default::Default; -use crate::uint128::U128; use crate::embedded_curve_ops::{ EmbeddedCurvePoint, EmbeddedCurveScalar, multi_scalar_mul, multi_scalar_mul_array_return, }; use crate::meta::derive_via; +use crate::uint128::U128; // Kept for backwards compatibility pub use sha256::{digest, sha256, sha256_compression, sha256_var}; diff --git a/noir_stdlib/src/hash/poseidon/bn254/consts.nr b/noir_stdlib/src/hash/poseidon/bn254/consts.nr index 835ed3ea476..42df0ef662f 100644 --- a/noir_stdlib/src/hash/poseidon/bn254/consts.nr +++ b/noir_stdlib/src/hash/poseidon/bn254/consts.nr @@ -2,8 +2,8 @@ // Used like so: sage generate_parameters_grain.sage 1 0 254 2 8 56 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001 // Constants for various Poseidon instances in the case of the prime field of the same order as BN254. // Consistent with https://github.com/iden3/circomlib/blob/master/circuits/poseidon.circom and https://github.com/iden3/circomlib/blob/master/circuits/poseidon_constants.circom -use crate::hash::poseidon::PoseidonConfig; use crate::hash::poseidon::config; +use crate::hash::poseidon::PoseidonConfig; // S-box power fn alpha() -> Field { 5 diff --git a/noir_stdlib/src/hash/poseidon/mod.nr b/noir_stdlib/src/hash/poseidon/mod.nr index 0af7951b8dc..6f0e461a610 100644 --- a/noir_stdlib/src/hash/poseidon/mod.nr +++ b/noir_stdlib/src/hash/poseidon/mod.nr @@ -1,6 +1,6 @@ pub mod bn254; // Instantiations of Poseidon for prime field of the same order as BN254 -use crate::hash::Hasher; use crate::default::Default; +use crate::hash::Hasher; // A config struct defining the parameters of the Poseidon instance to use. // diff --git a/noir_stdlib/src/hash/poseidon2.nr b/noir_stdlib/src/hash/poseidon2.nr index 517c2cd8f5f..f2167c43c2c 100644 --- a/noir_stdlib/src/hash/poseidon2.nr +++ b/noir_stdlib/src/hash/poseidon2.nr @@ -1,5 +1,5 @@ -use crate::hash::Hasher; use crate::default::Default; +use crate::hash::Hasher; comptime global RATE: u32 = 3; diff --git a/noir_stdlib/src/meta/expr.nr b/noir_stdlib/src/meta/expr.nr index 1b04a97ab15..bb795a520d3 100644 --- a/noir_stdlib/src/meta/expr.nr +++ b/noir_stdlib/src/meta/expr.nr @@ -1,8 +1,8 @@ //! Contains methods on the built-in `Expr` type for quoted, syntactically valid expressions. -use crate::option::Option; -use crate::meta::op::UnaryOp; use crate::meta::op::BinaryOp; +use crate::meta::op::UnaryOp; +use crate::option::Option; impl Expr { /// If this expression is an array literal `[elem1, ..., elemN]`, this returns a slice of each element in the array. diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index ff662b878ec..5d2164a510d 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -252,6 +252,41 @@ mod tests { fn do_nothing(_: Self) {} } + // docs:start:concatenate-example + comptime fn concatenate(q1: Quoted, q2: Quoted) -> Quoted { + assert(q1.tokens().len() <= 1); + assert(q2.tokens().len() <= 1); + + f"{q1}{q2}".quoted_contents() + } + + // The CtString type is also useful for a compile-time string of unbounded size + // so that you can append to it in a loop. + comptime fn double_spaced(q: Quoted) -> CtString { + let mut result = "".as_ctstring(); + + for token in q.tokens() { + if result != "".as_ctstring() { + result = result.append_str(" "); + } + result = result.append_fmtstr(f"{token}"); + } + + result + } + + #[test] + fn concatenate_test() { + comptime { + let result = concatenate(quote {foo}, quote {bar}); + assert_eq(result, quote {foobar}); + + let result = double_spaced(quote {foo bar 3}).as_quoted_str!(); + assert_eq(result, "foo bar 3"); + } + } + // docs:end:concatenate-example + // This function is just to remove unused warnings fn remove_unused_warnings() { let _: Bar = Bar { x: 1, y: [2, 3] }; @@ -259,6 +294,8 @@ mod tests { let _: MyOtherStruct = MyOtherStruct { my_other_field: 2 }; let _ = derive_do_nothing(crate::panic::panic(f"")); let _ = derive_do_nothing_alt(crate::panic::panic(f"")); - remove_unused_warnings(); + if false { + remove_unused_warnings(); + } } } diff --git a/noir_stdlib/src/meta/trait_constraint.nr b/noir_stdlib/src/meta/trait_constraint.nr index bf22f454448..2d9c6639dbd 100644 --- a/noir_stdlib/src/meta/trait_constraint.nr +++ b/noir_stdlib/src/meta/trait_constraint.nr @@ -1,5 +1,5 @@ -use crate::hash::{Hash, Hasher}; use crate::cmp::Eq; +use crate::hash::{Hash, Hasher}; impl Eq for TraitConstraint { comptime fn eq(self, other: Self) -> bool { diff --git a/noir_stdlib/src/meta/trait_def.nr b/noir_stdlib/src/meta/trait_def.nr index cc448b2eae5..75f746337d4 100644 --- a/noir_stdlib/src/meta/trait_def.nr +++ b/noir_stdlib/src/meta/trait_def.nr @@ -1,5 +1,5 @@ -use crate::hash::{Hash, Hasher}; use crate::cmp::Eq; +use crate::hash::{Hash, Hasher}; impl TraitDefinition { #[builtin(trait_def_as_trait_constraint)] diff --git a/noir_stdlib/src/ops/mod.nr b/noir_stdlib/src/ops/mod.nr index bf908ea4b27..0e3a2467c60 100644 --- a/noir_stdlib/src/ops/mod.nr +++ b/noir_stdlib/src/ops/mod.nr @@ -1,5 +1,5 @@ pub(crate) mod arith; pub(crate) mod bit; -pub use arith::{Add, Sub, Mul, Div, Rem, Neg}; -pub use bit::{Not, BitOr, BitAnd, BitXor, Shl, Shr}; +pub use arith::{Add, Div, Mul, Neg, Rem, Sub}; +pub use bit::{BitAnd, BitOr, BitXor, Not, Shl, Shr}; diff --git a/noir_stdlib/src/option.nr b/noir_stdlib/src/option.nr index 5db2ce84efd..0a62e412849 100644 --- a/noir_stdlib/src/option.nr +++ b/noir_stdlib/src/option.nr @@ -1,6 +1,6 @@ -use crate::hash::{Hash, Hasher}; -use crate::cmp::{Ordering, Ord, Eq}; +use crate::cmp::{Eq, Ord, Ordering}; use crate::default::Default; +use crate::hash::{Hash, Hasher}; pub struct Option { _is_some: bool, diff --git a/noir_stdlib/src/prelude.nr b/noir_stdlib/src/prelude.nr index b6e54eaae60..a4a6c35b615 100644 --- a/noir_stdlib/src/prelude.nr +++ b/noir_stdlib/src/prelude.nr @@ -1,10 +1,10 @@ -pub use crate::collections::vec::Vec; -pub use crate::collections::bounded_vec::BoundedVec; -pub use crate::option::Option; -pub use crate::{print, println, assert_constant}; -pub use crate::uint128::U128; +pub use crate::{assert_constant, print, println}; pub use crate::cmp::{Eq, Ord}; -pub use crate::default::Default; +pub use crate::collections::bounded_vec::BoundedVec; +pub use crate::collections::vec::Vec; pub use crate::convert::{From, Into}; +pub use crate::default::Default; pub use crate::meta::{derive, derive_via}; +pub use crate::option::Option; pub use crate::panic::panic; +pub use crate::uint128::U128; diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index a4e20859604..06febb3ee00 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -1,5 +1,5 @@ -use crate::ops::{Add, Sub, Mul, Div, Rem, Not, BitOr, BitAnd, BitXor, Shl, Shr}; use crate::cmp::{Eq, Ord, Ordering}; +use crate::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Not, Rem, Shl, Shr, Sub}; global pow64: Field = 18446744073709551616; //2^64; global pow63: Field = 9223372036854775808; // 2^63; @@ -313,7 +313,7 @@ impl Shr for U128 { } mod tests { - use crate::uint128::{U128, pow64, pow63}; + use crate::uint128::{pow63, pow64, U128}; #[test] fn test_not(lo: u64, hi: u64) {