diff --git a/.noir-sync-commit b/.noir-sync-commit index f58ed005d42..cfaf9d81cf2 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -44cf9a2140bc06b550d4b46966f1637598ac11a7 +b84009ca428a5790acf53a6c027146b706170574 diff --git a/noir/noir-repo/aztec_macros/src/utils/ast_utils.rs b/noir/noir-repo/aztec_macros/src/utils/ast_utils.rs index 19372fa5cb5..316aa60da62 100644 --- a/noir/noir-repo/aztec_macros/src/utils/ast_utils.rs +++ b/noir/noir-repo/aztec_macros/src/utils/ast_utils.rs @@ -187,8 +187,8 @@ pub fn check_trait_method_implemented(trait_impl: &NoirTraitImpl, method_name: & /// Checks if an attribute is a custom attribute with a specific name pub fn is_custom_attribute(attr: &SecondaryAttribute, attribute_name: &str) -> bool { - if let SecondaryAttribute::Custom(custom_attr) = attr { - custom_attr.as_str() == attribute_name + if let SecondaryAttribute::Custom(custom_attribute) = attr { + custom_attribute.contents.as_str() == attribute_name } else { false } diff --git a/noir/noir-repo/aztec_macros/src/utils/parse_utils.rs b/noir/noir-repo/aztec_macros/src/utils/parse_utils.rs index 6a2a876e682..e7b3e347a96 100644 --- a/noir/noir-repo/aztec_macros/src/utils/parse_utils.rs +++ b/noir/noir-repo/aztec_macros/src/utils/parse_utils.rs @@ -53,6 +53,7 @@ fn empty_item(item: &mut Item) { ItemKind::Import(use_tree, _) => empty_use_tree(use_tree), ItemKind::Struct(noir_struct) => empty_noir_struct(noir_struct), ItemKind::TypeAlias(noir_type_alias) => empty_noir_type_alias(noir_type_alias), + ItemKind::InnerAttribute(_) => (), } } diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index 88918151366..a315e7ed397 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -452,9 +452,13 @@ fn compile_contract_inner( .attributes .secondary .iter() - .filter_map( - |attr| if let SecondaryAttribute::Custom(tag) = attr { Some(tag) } else { None }, - ) + .filter_map(|attr| { + if let SecondaryAttribute::Custom(attribute) = attr { + Some(&attribute.contents) + } else { + None + } + }) .cloned() .collect(); diff --git a/noir/noir-repo/compiler/noirc_errors/src/position.rs b/noir/noir-repo/compiler/noirc_errors/src/position.rs index 1792197eab7..9b031f56ae2 100644 --- a/noir/noir-repo/compiler/noirc_errors/src/position.rs +++ b/noir/noir-repo/compiler/noirc_errors/src/position.rs @@ -103,6 +103,10 @@ impl Span { let other_distance = other.end() - other.start(); self_distance < other_distance } + + pub fn shift_by(&self, offset: u32) -> Span { + Self::from(self.start() + offset..self.end() + offset) + } } impl From for Range { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index a38d8ef582d..ba23704bba9 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -648,7 +648,11 @@ impl<'block> BrilligBlock<'block> { }; let index_variable = self.convert_ssa_single_addr_value(*index, dfg); - self.validate_array_index(array_variable, index_variable); + + if !dfg.is_safe_index(*index, *array) { + self.validate_array_index(array_variable, index_variable); + } + self.retrieve_variable_from_array( array_pointer, index_variable, @@ -667,7 +671,11 @@ impl<'block> BrilligBlock<'block> { result_ids[0], dfg, ); - self.validate_array_index(source_variable, index_register); + + if !dfg.is_safe_index(*index, *array) { + self.validate_array_index(source_variable, index_register); + } + self.convert_ssa_array_set( source_variable, destination_variable, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs index c4f56d032f9..bcd6865b721 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs @@ -88,6 +88,7 @@ impl From for FileDiagnostic { InternalBug::IndependentSubgraph { call_stack } => { ("There is no path from the output of this brillig call to either return values or inputs of the circuit, which creates an independent subgraph. This is quite likely a soundness vulnerability".to_string(),call_stack) } + InternalBug::AssertFailed { call_stack } => ("As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution".to_string(), call_stack) }; let call_stack = vecmap(call_stack, |location| location); let file_id = call_stack.last().map(|location| location.file).unwrap_or_default(); @@ -111,6 +112,8 @@ pub enum InternalWarning { pub enum InternalBug { #[error("Input to brillig function is in a separate subgraph to output")] IndependentSubgraph { call_stack: CallStack }, + #[error("Assertion is always false")] + AssertFailed { call_stack: CallStack }, } #[derive(Debug, PartialEq, Eq, Clone, Error)] diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index a0be1ee19cf..d12d49784ec 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -2,7 +2,7 @@ use super::big_int::BigIntContext; use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX}; use crate::brillig::brillig_gen::brillig_directive; use crate::brillig::brillig_ir::artifact::GeneratedBrillig; -use crate::errors::{InternalError, RuntimeError, SsaReport}; +use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport}; use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue}; use crate::ssa::ir::dfg::CallStack; use crate::ssa::ir::types::Type as SsaType; @@ -126,6 +126,8 @@ pub(crate) struct AcirContext { big_int_ctx: BigIntContext, expression_width: ExpressionWidth, + + pub(crate) warnings: Vec, } impl AcirContext { @@ -518,6 +520,12 @@ impl AcirContext { self.mark_variables_equivalent(lhs, rhs)?; return Ok(()); } + if diff_expr.is_const() { + // Constraint is always false + self.warnings.push(SsaReport::Bug(InternalBug::AssertFailed { + call_stack: self.get_call_stack(), + })); + } self.acir_ir.assert_is_zero(diff_expr); if let Some(payload) = assert_message { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 0360b15d950..15b44fde65d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -453,6 +453,7 @@ impl<'a> Context<'a> { } warnings.extend(return_warnings); + warnings.extend(self.acir_context.warnings.clone()); // Add the warnings from the alter Ssa passes Ok(self.acir_context.finish(input_witness, return_witnesses, warnings)) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs index b9804062118..095413d7b9a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -377,13 +377,12 @@ fn handle_array_get_group( next_out_of_bounds_index: &mut Option, possible_index_out_of_bounds_indexes: &mut Vec, ) { - let Some(array_length) = function.dfg.try_get_array_length(*array) else { + if function.dfg.try_get_array_length(*array).is_none() { // Nothing to do for slices return; }; - let flattened_size = function.dfg.type_of_value(*array).flattened_size(); - let element_size = flattened_size / array_length; + let element_size = function.dfg.type_of_value(*array).element_size(); if element_size <= 1 { // Not a composite type return; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 3d98f4126cf..89445c4d195 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -117,6 +117,10 @@ struct PerFunctionContext<'f> { /// Track a value's last load across all blocks. /// If a value is not used in anymore loads we can remove the last store to that value. last_loads: HashMap, + + /// Track whether the last instruction is an inc_rc/dec_rc instruction. + /// If it is we should not remove any known store values. + inside_rc_reload: bool, } impl<'f> PerFunctionContext<'f> { @@ -131,6 +135,7 @@ impl<'f> PerFunctionContext<'f> { blocks: BTreeMap::new(), instructions_to_remove: BTreeSet::new(), last_loads: HashMap::default(), + inside_rc_reload: false, } } @@ -281,6 +286,21 @@ impl<'f> PerFunctionContext<'f> { } else { references.mark_value_used(address, self.inserter.function); + let expression = if let Some(expression) = references.expressions.get(&result) { + expression.clone() + } else { + references.expressions.insert(result, Expression::Other(result)); + Expression::Other(result) + }; + if let Some(aliases) = references.aliases.get_mut(&expression) { + aliases.insert(result); + } else { + references + .aliases + .insert(Expression::Other(result), AliasSet::known(result)); + } + references.set_known_value(result, address); + self.last_loads.insert(address, (instruction, block_id)); } } @@ -296,6 +316,14 @@ impl<'f> PerFunctionContext<'f> { self.instructions_to_remove.insert(*last_store); } + let known_value = references.get_known_value(value); + if let Some(known_value) = known_value { + let known_value_is_address = known_value == address; + if known_value_is_address && !self.inside_rc_reload { + self.instructions_to_remove.insert(instruction); + } + } + references.set_known_value(address, value); references.last_stores.insert(address, instruction); } @@ -350,6 +378,18 @@ impl<'f> PerFunctionContext<'f> { Instruction::Call { arguments, .. } => self.mark_all_unknown(arguments, references), _ => (), } + + self.track_rc_reload_state(instruction); + } + + fn track_rc_reload_state(&mut self, instruction: InstructionId) { + match &self.inserter.function.dfg[instruction] { + // We just had an increment or decrement to an array's reference counter + Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. } => { + self.inside_rc_reload = true; + } + _ => self.inside_rc_reload = false, + } } fn check_array_aliasing(&self, references: &mut Block, array: ValueId) { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs index 0a3b2a800ca..6887873dbc3 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs @@ -15,7 +15,9 @@ use acvm::acir::AcirField; use crate::ssa::{ ir::{ - basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function, + basic_block::BasicBlockId, + cfg::ControlFlowGraph, + function::{Function, RuntimeType}, instruction::TerminatorInstruction, }, ssa_gen::Ssa, @@ -30,7 +32,7 @@ impl Ssa { /// 5. Replacing any jmpifs with constant conditions with jmps. If this causes the block to have /// only 1 successor then (2) also will be applied. /// - /// Currently, 1 and 4 are unimplemented. + /// Currently, 1 is unimplemented. #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn simplify_cfg(mut self) -> Self { for function in self.functions.values_mut() { @@ -72,6 +74,10 @@ fn simplify_function(function: &mut Function) { // optimizations performed after this point on the same block should check if // the inlining here was successful before continuing. try_inline_into_predecessor(function, &mut cfg, block, predecessor); + } else { + drop(predecessors); + + check_for_double_jmp(function, block, &mut cfg); } } } @@ -102,6 +108,71 @@ fn check_for_constant_jmpif( } } +/// Optimize a jmp to a block which immediately jmps elsewhere to just jmp to the second block. +fn check_for_double_jmp(function: &mut Function, block: BasicBlockId, cfg: &mut ControlFlowGraph) { + if matches!(function.runtime(), RuntimeType::Acir(_)) { + // We can't remove double jumps in ACIR functions as this interferes with the `flatten_cfg` pass. + return; + } + + if !function.dfg[block].instructions().is_empty() + || !function.dfg[block].parameters().is_empty() + { + return; + } + + let Some(TerminatorInstruction::Jmp { destination: final_destination, arguments, .. }) = + function.dfg[block].terminator() + else { + return; + }; + + if !arguments.is_empty() { + return; + } + + let final_destination = *final_destination; + + let predecessors: Vec<_> = cfg.predecessors(block).collect(); + for predecessor_block in predecessors { + let terminator_instruction = function.dfg[predecessor_block].take_terminator(); + let redirected_terminator_instruction = match terminator_instruction { + TerminatorInstruction::JmpIf { + condition, + then_destination, + else_destination, + call_stack, + } => { + let then_destination = + if then_destination == block { final_destination } else { then_destination }; + let else_destination = + if else_destination == block { final_destination } else { else_destination }; + TerminatorInstruction::JmpIf { + condition, + then_destination, + else_destination, + call_stack, + } + } + TerminatorInstruction::Jmp { destination, arguments, call_stack } => { + assert_eq!( + destination, block, + "ICE: predecessor block doesn't jump to current block" + ); + assert!(arguments.is_empty(), "ICE: predecessor jmp has arguments"); + TerminatorInstruction::Jmp { destination: final_destination, arguments, call_stack } + } + TerminatorInstruction::Return { .. } => { + unreachable!("ICE: predecessor block should not have return terminator instruction") + } + }; + + function.dfg[predecessor_block].set_terminator(redirected_terminator_instruction); + cfg.recompute_block(function, predecessor_block); + } + cfg.recompute_block(function, block); +} + /// If the given block has block parameters, replace them with the jump arguments from the predecessor. /// /// Currently, if this function is needed, `try_inline_into_predecessor` will also always apply, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index 2e14761a1cc..30db8ad63fd 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs @@ -292,6 +292,7 @@ pub trait Recoverable { #[derive(Debug, PartialEq, Eq, Clone)] pub struct ModuleDeclaration { pub ident: Ident, + pub outer_attributes: Vec, } impl std::fmt::Display for ModuleDeclaration { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs index 3955e50b03e..0aeeed39dd0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs @@ -16,7 +16,7 @@ use crate::{ QuotedTypeId, }, parser::{Item, ItemKind, ParsedSubModule}, - token::Tokens, + token::{SecondaryAttribute, Tokens}, ParsedModule, QuotedType, }; @@ -432,6 +432,8 @@ pub trait Visitor { fn visit_struct_pattern(&mut self, _: &Path, _: &[(Ident, Pattern)], _: Span) -> bool { true } + + fn visit_secondary_attribute(&mut self, _: &SecondaryAttribute, _: Span) {} } impl ParsedModule { @@ -481,6 +483,9 @@ impl Item { ItemKind::ModuleDecl(module_declaration) => { module_declaration.accept(self.span, visitor); } + ItemKind::InnerAttribute(attribute) => { + attribute.accept(self.span, visitor); + } } } } @@ -1289,6 +1294,12 @@ impl Pattern { } } +impl SecondaryAttribute { + pub fn accept(&self, span: Span, visitor: &mut impl Visitor) { + visitor.visit_secondary_attribute(self, span); + } +} + fn visit_expressions(expressions: &[Expression], visitor: &mut impl Visitor) { for expression in expressions { expression.accept(visitor); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs index baa9c0ab371..7da5efd0b5a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -10,11 +10,12 @@ use crate::{ comptime::{Interpreter, InterpreterError, Value}, def_collector::{ dc_crate::{ - CollectedItems, CompilationError, UnresolvedFunctions, UnresolvedStruct, - UnresolvedTrait, UnresolvedTraitImpl, + CollectedItems, CompilationError, ModuleAttribute, UnresolvedFunctions, + UnresolvedStruct, UnresolvedTrait, UnresolvedTraitImpl, }, dc_mod, }, + def_map::ModuleId, resolution::errors::ResolverError, }, hir_def::expr::HirIdent, @@ -24,7 +25,7 @@ use crate::{ }, node_interner::{DefinitionKind, DependencyId, FuncId, TraitId}, parser::{self, TopLevelStatement}, - Type, TypeBindings, + Type, TypeBindings, UnificationError, }; use super::{Elaborator, FunctionContext, ResolverMeta}; @@ -96,25 +97,40 @@ impl<'context> Elaborator<'context> { generated_items: &mut CollectedItems, ) { for attribute in attributes { - if let SecondaryAttribute::Custom(name) = attribute { - if let Err(error) = - self.run_comptime_attribute_on_item(name, item.clone(), span, generated_items) - { - self.errors.push(error); - } - } + self.run_comptime_attribute_on_item(attribute, &item, span, generated_items); } } fn run_comptime_attribute_on_item( + &mut self, + attribute: &SecondaryAttribute, + item: &Value, + span: Span, + generated_items: &mut CollectedItems, + ) { + if let SecondaryAttribute::Custom(attribute) = attribute { + if let Err(error) = self.run_comptime_attribute_name_on_item( + &attribute.contents, + item.clone(), + span, + attribute.contents_span, + generated_items, + ) { + self.errors.push(error); + } + } + } + + fn run_comptime_attribute_name_on_item( &mut self, attribute: &str, item: Value, span: Span, + attribute_span: Span, generated_items: &mut CollectedItems, ) -> Result<(), (CompilationError, FileId)> { - let location = Location::new(span, self.file); - let Some((function, arguments)) = Self::parse_attribute(attribute, self.file)? else { + let location = Location::new(attribute_span, self.file); + let Some((function, arguments)) = Self::parse_attribute(attribute, location)? else { // Do not issue an error if the attribute is unknown return Ok(()); }; @@ -141,12 +157,17 @@ impl<'context> Elaborator<'context> { }; let mut interpreter = self.setup_interpreter(); - let mut arguments = - Self::handle_attribute_arguments(&mut interpreter, function, arguments, location) - .map_err(|error| { - let file = error.get_location().file; - (error.into(), file) - })?; + let mut arguments = Self::handle_attribute_arguments( + &mut interpreter, + &item, + function, + arguments, + location, + ) + .map_err(|error| { + let file = error.get_location().file; + (error.into(), file) + })?; arguments.insert(0, (item, location)); @@ -154,6 +175,8 @@ impl<'context> Elaborator<'context> { .call_function(function, arguments, TypeBindings::new(), location) .map_err(|error| error.into_compilation_error_pair())?; + self.debug_comptime(location, |interner| value.display(interner).to_string()); + if value != Value::Unit { let items = value .into_top_level_items(location, self.interner) @@ -170,33 +193,62 @@ impl<'context> Elaborator<'context> { #[allow(clippy::type_complexity)] pub(crate) fn parse_attribute( annotation: &str, - file: FileId, + location: Location, ) -> Result)>, (CompilationError, FileId)> { let (tokens, mut lexing_errors) = Lexer::lex(annotation); if !lexing_errors.is_empty() { - return Err((lexing_errors.swap_remove(0).into(), file)); + return Err((lexing_errors.swap_remove(0).into(), location.file)); } let expression = parser::expression() .parse(tokens) - .map_err(|mut errors| (errors.swap_remove(0).into(), file))?; + .map_err(|mut errors| (errors.swap_remove(0).into(), location.file))?; - Ok(match expression.kind { - ExpressionKind::Call(call) => Some((*call.func, call.arguments)), - ExpressionKind::Variable(_) => Some((expression, Vec::new())), - _ => None, - }) + let (mut func, mut arguments) = match expression.kind { + ExpressionKind::Call(call) => (*call.func, call.arguments), + ExpressionKind::Variable(_) => (expression, Vec::new()), + _ => return Ok(None), + }; + + func.span = func.span.shift_by(location.span.start()); + + for argument in &mut arguments { + argument.span = argument.span.shift_by(location.span.start()); + } + + Ok(Some((func, arguments))) } fn handle_attribute_arguments( interpreter: &mut Interpreter, + item: &Value, function: FuncId, arguments: Vec, location: Location, ) -> Result, InterpreterError> { let meta = interpreter.elaborator.interner.function_meta(&function); + let mut parameters = vecmap(&meta.parameters.0, |(_, typ, _)| typ.clone()); + if parameters.is_empty() { + return Err(InterpreterError::ArgumentCountMismatch { + expected: 0, + actual: arguments.len() + 1, + location, + }); + } + + let expected_type = item.get_type(); + let expected_type = expected_type.as_ref(); + + if ¶meters[0] != expected_type { + return Err(InterpreterError::TypeMismatch { + expected: parameters[0].clone(), + actual: expected_type.clone(), + location, + }); + } + // Remove the initial parameter for the comptime item since that is not included // in `arguments` at this point. parameters.remove(0); @@ -213,6 +265,7 @@ impl<'context> Elaborator<'context> { let mut varargs = im::Vector::new(); for (i, arg) in arguments.into_iter().enumerate() { + let arg_location = Location::new(arg.span, location.file); let param_type = parameters.get(i).or(varargs_elem_type).unwrap_or(&Type::Error); let mut push_arg = |arg| { @@ -233,9 +286,17 @@ impl<'context> Elaborator<'context> { }?; push_arg(Value::TraitDefinition(trait_id)); } else { - let expr_id = interpreter.elaborator.elaborate_expression(arg).0; + let (expr_id, expr_type) = interpreter.elaborator.elaborate_expression(arg); push_arg(interpreter.evaluate(expr_id)?); - } + + if let Err(UnificationError) = expr_type.unify(param_type) { + return Err(InterpreterError::TypeMismatch { + expected: param_type.clone(), + actual: expr_type, + location: arg_location, + }); + } + }; } if is_varargs { @@ -265,24 +326,24 @@ impl<'context> Elaborator<'context> { ) { match item { TopLevelStatement::Function(function) => { - let id = self.interner.push_empty_fn(); - let module = self.module_id(); - self.interner.push_function(id, &function.def, module, location); + let module_id = self.module_id(); - if self.interner.is_in_lsp_mode() - && !function.def.is_test() - && !function.def.is_private() - { - self.interner.register_function(id, &function.def); + if let Some(id) = dc_mod::collect_function( + self.interner, + self.def_maps.get_mut(&self.crate_id).unwrap(), + &function, + module_id, + self.file, + &mut self.errors, + ) { + let functions = vec![(self.local_module, id, function)]; + generated_items.functions.push(UnresolvedFunctions { + file_id: self.file, + functions, + trait_id: None, + self_type: None, + }); } - - let functions = vec![(self.local_module, id, function)]; - generated_items.functions.push(UnresolvedFunctions { - file_id: self.file, - functions, - trait_id: None, - self_type: None, - }); } TopLevelStatement::TraitImpl(mut trait_impl) => { let (methods, associated_types, associated_constants) = @@ -329,16 +390,33 @@ impl<'context> Elaborator<'context> { self.errors.push(error); } } + TopLevelStatement::Struct(struct_def) => { + if let Some((type_id, the_struct)) = dc_mod::collect_struct( + self.interner, + self.def_maps.get_mut(&self.crate_id).unwrap(), + struct_def, + self.file, + self.local_module, + self.crate_id, + &mut self.errors, + ) { + generated_items.types.insert(type_id, the_struct); + } + } + TopLevelStatement::Impl(r#impl) => { + let module = self.module_id(); + dc_mod::collect_impl(self.interner, generated_items, r#impl, self.file, module); + } + // Assume that an error has already been issued TopLevelStatement::Error => (), TopLevelStatement::Module(_) | TopLevelStatement::Import(..) - | TopLevelStatement::Struct(_) | TopLevelStatement::Trait(_) - | TopLevelStatement::Impl(_) | TopLevelStatement::TypeAlias(_) - | TopLevelStatement::SubModule(_) => { + | TopLevelStatement::SubModule(_) + | TopLevelStatement::InnerAttribute(_) => { let item = item.to_string(); let error = InterpreterError::UnsupportedTopLevelItemUnquote { item, location }; self.errors.push(error.into_compilation_error_pair()); @@ -377,6 +455,7 @@ impl<'context> Elaborator<'context> { traits: &BTreeMap, types: &BTreeMap, functions: &[UnresolvedFunctions], + module_attributes: &[ModuleAttribute], ) -> CollectedItems { let mut generated_items = CollectedItems::default(); @@ -399,9 +478,31 @@ impl<'context> Elaborator<'context> { } self.run_attributes_on_functions(functions, &mut generated_items); + + self.run_attributes_on_modules(module_attributes, &mut generated_items); + generated_items } + fn run_attributes_on_modules( + &mut self, + module_attributes: &[ModuleAttribute], + generated_items: &mut CollectedItems, + ) { + for module_attribute in module_attributes { + let local_id = module_attribute.module_id; + 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(); + + self.local_module = module_attribute.attribute_module_id; + self.file = module_attribute.attribute_file_id; + + self.run_comptime_attribute_on_item(attribute, &item, span, generated_items); + } + } + fn run_attributes_on_functions( &mut self, function_sets: &[UnresolvedFunctions], diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index e84ed76050d..161742029f6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -28,6 +28,7 @@ use crate::{ node_interner::{ DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, }, + token::CustomAtrribute, Shared, Type, TypeVariable, }; use crate::{ @@ -320,7 +321,12 @@ 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(&items.traits, &items.types, &items.functions); + let generated_items = 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 @@ -819,7 +825,7 @@ impl<'context> Elaborator<'context> { let attributes = func.secondary_attributes().iter(); let attributes = attributes.filter_map(|secondary_attribute| secondary_attribute.as_custom()); - let attributes = attributes.map(|str| str.to_string()).collect(); + let attributes: Vec = attributes.cloned().collect(); let meta = FuncMeta { name: name_ident, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index e41234a5be5..8dccd5f0344 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -36,7 +36,7 @@ use crate::{ TraitImplKind, TraitMethodId, }, Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, TypeVariable, - TypeVariableKind, + TypeVariableKind, UnificationError, }; use super::{lints, Elaborator}; @@ -713,9 +713,9 @@ impl<'context> Elaborator<'context> { expected: &Type, make_error: impl FnOnce() -> TypeCheckError, ) { - let mut errors = Vec::new(); - actual.unify(expected, &mut errors, make_error); - self.errors.extend(errors.into_iter().map(|error| (error.into(), self.file))); + if let Err(UnificationError) = actual.unify(expected) { + self.errors.push((make_error().into(), self.file)); + } } /// Wrapper of Type::unify_with_coercions using self.errors diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs index cfee6bcedac..f6585786eeb 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -202,6 +202,11 @@ pub enum InterpreterError { TypeAnnotationsNeededForMethodCall { location: Location, }, + ExpectedIdentForStructField { + value: String, + index: usize, + location: Location, + }, // These cases are not errors, they are just used to prevent us from running more code // until the loop can be resumed properly. These cases will never be displayed to users. @@ -269,6 +274,7 @@ impl InterpreterError { | InterpreterError::FailedToResolveTraitBound { location, .. } | InterpreterError::FunctionAlreadyResolved { location, .. } | InterpreterError::MultipleMatchingImpls { location, .. } + | InterpreterError::ExpectedIdentForStructField { location, .. } | InterpreterError::TypeAnnotationsNeededForMethodCall { location } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { @@ -488,7 +494,7 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { InterpreterError::UnsupportedTopLevelItemUnquote { item, location } => { let msg = "Unsupported statement type to unquote".into(); let secondary = - "Only functions, globals, and trait impls can be unquoted here".into(); + "Only functions, structs, globals, and impls can be unquoted here".into(); let mut error = CustomDiagnostic::simple_error(msg, secondary, location.span); error.add_note(format!("Unquoted item was:\n{item}")); error @@ -566,6 +572,13 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { error.add_note(message.to_string()); error } + InterpreterError::ExpectedIdentForStructField { value, index, location } => { + let msg = format!( + "Quoted value in index {index} of this slice is not a valid field name" + ); + let secondary = format!("`{value}` is not a valid field name for `set_fields`"); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index d8e62b66eca..9f559b7c5e6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -586,7 +586,19 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { consuming = false; if let Some(value) = values.pop_front() { - result.push_str(&value.display(self.elaborator.interner).to_string()); + // When interpolating a quoted value inside a format string, we don't include the + // surrounding `quote {` ... `}` as if we are unquoting the quoted value inside the string. + if let Value::Quoted(tokens) = value { + for (index, token) in tokens.iter().enumerate() { + if index > 0 { + result.push(' '); + } + result + .push_str(&token.display(self.elaborator.interner).to_string()); + } + } else { + result.push_str(&value.display(self.elaborator.interner).to_string()); + } } } other if !consuming => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 070749e45ba..65c9c3f018d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -7,9 +7,9 @@ use acvm::{AcirField, FieldElement}; use builtin_helpers::{ block_expression_to_value, check_argument_count, check_function_not_yet_resolved, check_one_argument, check_three_arguments, check_two_arguments, get_expr, get_field, - get_function_def, get_module, get_quoted, get_slice, get_struct, get_trait_constraint, - get_trait_def, get_trait_impl, get_tuple, get_type, get_typed_expr, get_u32, - get_unresolved_type, hir_pattern_to_tokens, mutate_func_meta_type, parse, + get_format_string, get_function_def, get_module, get_quoted, get_slice, get_struct, + get_trait_constraint, get_trait_def, get_trait_impl, get_tuple, get_type, get_typed_expr, + get_u32, get_unresolved_type, hir_pattern_to_tokens, mutate_func_meta_type, parse, replace_func_meta_parameters, replace_func_meta_return_type, }; use chumsky::{prelude::choice, Parser}; @@ -32,7 +32,8 @@ use crate::{ InterpreterError, Value, }, hir_def::function::FunctionBody, - macros_api::{HirExpression, HirLiteral, ModuleDefId, NodeInterner, Signedness}, + lexer::Lexer, + macros_api::{HirExpression, HirLiteral, Ident, ModuleDefId, NodeInterner, Signedness}, node_interner::{DefinitionKind, TraitImplKind}, parser::{self}, token::Token, @@ -95,6 +96,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "expr_is_continue" => expr_is_continue(interner, arguments, location), "expr_resolve" => expr_resolve(self, arguments, location), "is_unconstrained" => Ok(Value::Bool(true)), + "fmtstr_quoted_contents" => fmtstr_quoted_contents(interner, arguments, location), "function_def_body" => function_def_body(interner, arguments, location), "function_def_has_named_attribute" => { function_def_has_named_attribute(interner, arguments, location) @@ -108,6 +110,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { function_def_set_return_type(self, arguments, location) } "module_functions" => module_functions(self, arguments, location), + "module_has_named_attribute" => module_has_named_attribute(self, arguments, location), "module_is_contract" => module_is_contract(self, arguments, location), "module_name" => module_name(interner, arguments, location), "modulus_be_bits" => modulus_be_bits(interner, arguments, location), @@ -130,6 +133,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "struct_def_as_type" => struct_def_as_type(interner, arguments, location), "struct_def_fields" => struct_def_fields(interner, arguments, location), "struct_def_generics" => struct_def_generics(interner, arguments, location), + "struct_def_set_fields" => struct_def_set_fields(interner, arguments, location), "to_le_radix" => to_le_radix(arguments, return_type, location), "trait_constraint_eq" => trait_constraint_eq(interner, arguments, location), "trait_constraint_hash" => trait_constraint_hash(interner, arguments, location), @@ -323,6 +327,64 @@ fn struct_def_fields( Ok(Value::Slice(fields, typ)) } +/// fn set_fields(self, new_fields: [(Quoted, Type)]) {} +/// Returns (name, type) pairs of each field of this StructDefinition +fn struct_def_set_fields( + interner: &mut NodeInterner, + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let (the_struct, fields) = check_two_arguments(arguments, location)?; + let struct_id = get_struct(the_struct)?; + + let struct_def = interner.get_struct(struct_id); + let mut struct_def = struct_def.borrow_mut(); + + let field_location = fields.1; + let fields = get_slice(interner, fields)?.0; + + let new_fields = fields + .into_iter() + .flat_map(|field_pair| get_tuple(interner, (field_pair, field_location))) + .enumerate() + .map(|(index, mut field_pair)| { + if field_pair.len() == 2 { + let typ = field_pair.pop().unwrap(); + let name_value = field_pair.pop().unwrap(); + + let name_tokens = get_quoted((name_value.clone(), field_location))?; + let typ = get_type((typ, field_location))?; + + match name_tokens.first() { + Some(Token::Ident(name)) if name_tokens.len() == 1 => { + Ok((Ident::new(name.clone(), field_location.span), typ)) + } + _ => { + let value = name_value.display(interner).to_string(); + let location = field_location; + Err(InterpreterError::ExpectedIdentForStructField { + value, + index, + location, + }) + } + } + } else { + let type_var = interner.next_type_variable(); + let expected = Type::Tuple(vec![type_var.clone(), type_var]); + + let actual = + Type::Tuple(vecmap(&field_pair, |value| value.get_type().into_owned())); + + Err(InterpreterError::TypeMismatch { expected, actual, location }) + } + }) + .collect::, _>>()?; + + struct_def.set_fields(new_fields); + Ok(Value::Unit) +} + fn slice_remove( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, @@ -1575,6 +1637,23 @@ fn unwrap_expr_value(interner: &NodeInterner, mut expr_value: ExprValue) -> Expr expr_value } +// fn quoted_contents(self) -> Quoted +fn fmtstr_quoted_contents( + interner: &NodeInterner, + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let self_argument = check_one_argument(arguments, location)?; + let (string, _) = get_format_string(interner, self_argument)?; + let (tokens, _) = Lexer::lex(&string); + let mut tokens: Vec<_> = tokens.0.into_iter().map(|token| token.into_token()).collect(); + if let Some(Token::EOF) = tokens.last() { + tokens.pop(); + } + + Ok(Value::Quoted(Rc::new(tokens))) +} + // fn body(self) -> Expr fn function_def_body( interner: &NodeInterner, @@ -1609,7 +1688,7 @@ fn function_def_has_named_attribute( let name = name.iter().map(|token| token.to_string()).collect::>().join(""); for attribute in attributes { - let parse_result = Elaborator::parse_attribute(attribute, location.file); + let parse_result = Elaborator::parse_attribute(&attribute.contents, location); let Ok(Some((function, _arguments))) = parse_result else { continue; }; @@ -1816,6 +1895,38 @@ fn module_functions( Ok(Value::Slice(func_ids, slice_type)) } +// fn has_named_attribute(self, name: Quoted) -> bool +fn module_has_named_attribute( + interpreter: &Interpreter, + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let (self_argument, name) = check_two_arguments(arguments, location)?; + let module_id = get_module(self_argument)?; + let module_data = interpreter.elaborator.get_module(module_id); + let name = get_quoted(name)?; + + let name = name.iter().map(|token| token.to_string()).collect::>().join(""); + + let attributes = module_data.outer_attributes.iter().chain(&module_data.inner_attributes); + for attribute in attributes { + let parse_result = Elaborator::parse_attribute(attribute, location); + let Ok(Some((function, _arguments))) = parse_result else { + continue; + }; + + let ExpressionKind::Variable(path) = function.kind else { + continue; + }; + + if path.last_name() == name { + return Ok(Value::Bool(true)); + } + } + + Ok(Value::Bool(false)) +} + // fn is_contract(self) -> bool fn module_is_contract( interpreter: &Interpreter, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index 14a0e177544..ff3da6d253f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -189,6 +189,20 @@ pub(crate) fn get_expr( } } +pub(crate) fn get_format_string( + interner: &NodeInterner, + (value, location): (Value, Location), +) -> IResult<(Rc, Type)> { + match value { + Value::FormatString(value, typ) => Ok((value, typ)), + value => { + let n = Box::new(interner.next_type_variable()); + let e = Box::new(interner.next_type_variable()); + type_mismatch(value, Type::FmtString(n, e), location) + } + } +} + pub(crate) fn get_function_def((value, location): (Value, Location)) -> IResult { match value { Value::FunctionDefinition(id) => Ok(id), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs index 4c1adf9fca0..64b489422a0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -23,7 +23,13 @@ fn interpret_helper(src: &str) -> Result { let module_id = LocalModuleId(Index::unsafe_zeroed()); let mut modules = noirc_arena::Arena::default(); let location = Location::new(Default::default(), file); - let root = LocalModuleId(modules.insert(ModuleData::new(None, location, false))); + let root = LocalModuleId(modules.insert(ModuleData::new( + None, + location, + Vec::new(), + Vec::new(), + false, + ))); assert_eq!(root, module_id); let file_manager = FileManager::new(&PathBuf::new()); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs index c5818c20c57..7d6e4475c7b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -429,6 +429,9 @@ impl Value { location: Location, ) -> IResult> { let token = match self { + Value::Unit => { + return Ok(vec![Token::LeftParen, Token::RightParen]); + } Value::Quoted(tokens) => return Ok(unwrap_rc(tokens)), Value::Type(typ) => Token::QuotedType(interner.push_quoted_type(typ)), Value::Expr(ExprValue::Expression(expr)) => { @@ -443,6 +446,40 @@ impl Value { Value::UnresolvedType(typ) => { Token::InternedUnresolvedTypeData(interner.push_unresolved_type_data(typ)) } + Value::U1(bool) => Token::Bool(bool), + Value::U8(value) => Token::Int((value as u128).into()), + Value::U16(value) => Token::Int((value as u128).into()), + Value::U32(value) => Token::Int((value as u128).into()), + Value::U64(value) => Token::Int((value as u128).into()), + Value::I8(value) => { + if value < 0 { + return Ok(vec![Token::Minus, Token::Int((-value as u128).into())]); + } else { + Token::Int((value as u128).into()) + } + } + Value::I16(value) => { + if value < 0 { + return Ok(vec![Token::Minus, Token::Int((-value as u128).into())]); + } else { + Token::Int((value as u128).into()) + } + } + Value::I32(value) => { + if value < 0 { + return Ok(vec![Token::Minus, Token::Int((-value as u128).into())]); + } else { + Token::Int((value as u128).into()) + } + } + Value::I64(value) => { + if value < 0 { + return Ok(vec![Token::Minus, Token::Int((-value as u128).into())]); + } else { + Token::Int((value as u128).into()) + } + } + Value::Field(value) => Token::Int(value), other => Token::UnquoteMarker(other.into_hir_expression(interner, location)?), }; Ok(vec![token]) @@ -568,33 +605,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { write!(f, "quote {{")?; for token in tokens.iter() { write!(f, " ")?; - - match token { - Token::QuotedType(id) => { - write!(f, "{}", self.interner.get_quoted_type(*id))?; - } - Token::InternedExpr(id) => { - let value = Value::expression(ExpressionKind::Interned(*id)); - value.display(self.interner).fmt(f)?; - } - Token::InternedStatement(id) => { - let value = Value::statement(StatementKind::Interned(*id)); - value.display(self.interner).fmt(f)?; - } - Token::InternedLValue(id) => { - let value = Value::lvalue(LValue::Interned(*id, Span::default())); - value.display(self.interner).fmt(f)?; - } - Token::InternedUnresolvedTypeData(id) => { - let value = Value::UnresolvedType(UnresolvedTypeData::Interned(*id)); - value.display(self.interner).fmt(f)?; - } - Token::UnquoteMarker(id) => { - let value = Value::TypedExpr(TypedExpr::ExprId(*id)); - value.display(self.interner).fmt(f)?; - } - other => write!(f, "{other}")?, - } + token.display(self.interner).fmt(f)?; } write!(f, " }}") } @@ -676,6 +687,51 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { } } +impl Token { + pub fn display<'token, 'interner>( + &'token self, + interner: &'interner NodeInterner, + ) -> TokenPrinter<'token, 'interner> { + TokenPrinter { token: self, interner } + } +} + +pub struct TokenPrinter<'token, 'interner> { + token: &'token Token, + interner: &'interner NodeInterner, +} + +impl<'token, 'interner> Display for TokenPrinter<'token, 'interner> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.token { + Token::QuotedType(id) => { + write!(f, "{}", self.interner.get_quoted_type(*id)) + } + Token::InternedExpr(id) => { + let value = Value::expression(ExpressionKind::Interned(*id)); + value.display(self.interner).fmt(f) + } + Token::InternedStatement(id) => { + let value = Value::statement(StatementKind::Interned(*id)); + value.display(self.interner).fmt(f) + } + Token::InternedLValue(id) => { + let value = Value::lvalue(LValue::Interned(*id, Span::default())); + value.display(self.interner).fmt(f) + } + Token::InternedUnresolvedTypeData(id) => { + let value = Value::UnresolvedType(UnresolvedTypeData::Interned(*id)); + value.display(self.interner).fmt(f) + } + Token::UnquoteMarker(id) => { + let value = Value::TypedExpr(TypedExpr::ExprId(*id)); + value.display(self.interner).fmt(f) + } + other => write!(f, "{other}"), + } + } +} + fn display_trait_constraint(interner: &NodeInterner, trait_constraint: &TraitConstraint) -> String { let trait_ = interner.get_trait(trait_constraint.trait_id); format!("{}: {}{}", trait_constraint.typ, trait_.name, trait_constraint.trait_generics) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 6a6cabe593d..3cfa0989d7d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -7,6 +7,8 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::path_resolver; use crate::hir::type_check::TypeCheckError; +use crate::token::SecondaryAttribute; +use crate::usage_tracker::UnusedItem; use crate::{Generics, Type}; use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution}; @@ -111,6 +113,21 @@ pub struct UnresolvedGlobal { pub stmt_def: LetStatement, } +pub struct ModuleAttribute { + // The file in which the module is defined + 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) + 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, + pub attribute: SecondaryAttribute, + pub is_inner: bool, +} + /// Given a Crate root, collect all definitions in that crate pub struct DefCollector { pub(crate) def_map: CrateDefMap, @@ -127,6 +144,7 @@ pub struct CollectedItems { pub globals: Vec, pub(crate) impls: ImplMap, pub(crate) trait_impls: Vec, + pub(crate) module_attributes: Vec, } impl CollectedItems { @@ -238,6 +256,7 @@ impl DefCollector { impls: HashMap::default(), globals: vec![], trait_impls: vec![], + module_attributes: vec![], }, } } @@ -253,7 +272,7 @@ impl DefCollector { root_file_id: FileId, debug_comptime_in_file: Option<&str>, enable_arithmetic_generics: bool, - error_on_usage_tracker: bool, + error_on_unused_items: bool, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; @@ -388,20 +407,14 @@ impl DefCollector { let result = current_def_map.modules[resolved_import.module_scope.0] .import(name.clone(), visibility, module_def_id, is_prelude); - // Empty spans could come from implicitly injected imports, and we don't want to track those - if visibility != ItemVisibility::Public - && name.span().start() < name.span().end() - { - let module_id = ModuleId { - krate: crate_id, - local_id: resolved_import.module_scope, - }; - - context - .def_interner - .usage_tracker - .add_unused_import(module_id, name.clone()); - } + let module_id = + ModuleId { krate: crate_id, local_id: resolved_import.module_scope }; + context.def_interner.usage_tracker.add_unused_item( + module_id, + name.clone(), + UnusedItem::Import, + visibility, + ); if visibility != ItemVisibility::Private { let local_id = resolved_import.module_scope; @@ -476,26 +489,29 @@ impl DefCollector { ); } - if error_on_usage_tracker { - Self::check_usage_tracker(context, crate_id, &mut errors); + if error_on_unused_items { + Self::check_unused_items(context, crate_id, &mut errors); } errors } - fn check_usage_tracker( + fn check_unused_items( context: &Context, crate_id: CrateId, errors: &mut Vec<(CompilationError, FileId)>, ) { - let unused_imports = context.def_interner.usage_tracker.unused_imports().iter(); + let unused_imports = context.def_interner.usage_tracker.unused_items().iter(); let unused_imports = unused_imports.filter(|(module_id, _)| module_id.krate == crate_id); errors.extend(unused_imports.flat_map(|(module_id, usage_tracker)| { let module = &context.def_maps[&crate_id].modules()[module_id.local_id.0]; - usage_tracker.iter().map(|ident| { + usage_tracker.iter().map(|(ident, unused_item)| { let ident = ident.clone(); - let error = CompilationError::ResolverError(ResolverError::UnusedImport { ident }); + let error = CompilationError::ResolverError(ResolverError::UnusedItem { + ident, + item_type: unused_item.item_type(), + }); (error, module.location.file) }) })); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 590cdc541ce..6c1b7632a2e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -15,8 +15,10 @@ use crate::ast::{ TypeImpl, }; use crate::hir::resolution::errors::ResolverError; -use crate::macros_api::{Expression, NodeInterner, UnresolvedType, UnresolvedTypeData}; +use crate::macros_api::{Expression, NodeInterner, StructId, UnresolvedType, UnresolvedTypeData}; use crate::node_interner::ModuleAttributes; +use crate::token::SecondaryAttribute; +use crate::usage_tracker::UnusedItem; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, @@ -26,6 +28,8 @@ use crate::{ }; use crate::{Generics, Kind, ResolvedGeneric, Type, TypeVariable}; +use super::dc_crate::CollectedItems; +use super::dc_crate::ModuleAttribute; use super::{ dc_crate::{ CompilationError, DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTraitImpl, @@ -33,7 +37,7 @@ use super::{ }, errors::{DefCollectorErrorKind, DuplicateType}, }; -use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData, ModuleId}; +use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData, ModuleId, MAIN_FUNCTION}; use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; @@ -63,8 +67,10 @@ pub fn collect_defs( for decl in ast.module_decls { errors.extend(collector.parse_module_declaration( context, - &decl, + decl, crate_id, + file_id, + module_id, macro_processors, )); } @@ -72,6 +78,7 @@ pub fn collect_defs( errors.extend(collector.collect_submodules( context, crate_id, + module_id, ast.submodules, file_id, macro_processors, @@ -102,10 +109,40 @@ pub fn collect_defs( collector.collect_impls(context, ast.impls, crate_id); + collector.collect_attributes( + ast.inner_attributes, + file_id, + module_id, + file_id, + module_id, + true, + ); + errors } impl<'a> ModCollector<'a> { + fn collect_attributes( + &mut self, + attributes: Vec, + file_id: FileId, + module_id: LocalModuleId, + attribute_file_id: FileId, + attribute_module_id: LocalModuleId, + is_inner: bool, + ) { + for attribute in attributes { + self.def_collector.items.module_attributes.push(ModuleAttribute { + file_id, + module_id, + attribute_file_id, + attribute_module_id, + attribute, + is_inner, + }); + } + } + fn collect_globals( &mut self, context: &mut Context, @@ -136,24 +173,13 @@ impl<'a> ModCollector<'a> { let module_id = ModuleId { krate, local_id: self.module_id }; for r#impl in impls { - let mut unresolved_functions = UnresolvedFunctions { - file_id: self.file_id, - functions: Vec::new(), - trait_id: None, - self_type: None, - }; - - for (mut method, _) in r#impl.methods { - let func_id = context.def_interner.push_empty_fn(); - method.def.where_clause.extend(r#impl.where_clause.clone()); - let location = Location::new(method.span(), self.file_id); - context.def_interner.push_function(func_id, &method.def, module_id, location); - unresolved_functions.push_fn(self.module_id, func_id, method); - } - - let key = (r#impl.object_type, self.module_id); - let methods = self.def_collector.items.impls.entry(key).or_default(); - methods.push((r#impl.generics, r#impl.type_span, unresolved_functions)); + collect_impl( + &mut context.def_interner, + &mut self.def_collector.items, + r#impl, + self.file_id, + module_id, + ); } } @@ -223,28 +249,16 @@ impl<'a> ModCollector<'a> { let module = ModuleId { krate, local_id: self.module_id }; for function in functions { - // check if optional field attribute is compatible with native field - if let Some(field) = function.attributes().get_field_attribute() { - if !is_native_field(&field) { - continue; - } - } - - let name = function.name_ident().clone(); - let func_id = context.def_interner.push_empty_fn(); - let visibility = function.def.visibility; - - // First create dummy function in the DefInterner - // So that we can get a FuncId - let location = Location::new(function.span(), self.file_id); - context.def_interner.push_function(func_id, &function.def, module, location); - - if context.def_interner.is_in_lsp_mode() - && !function.def.is_test() - && !function.def.is_private() - { - context.def_interner.register_function(func_id, &function.def); - } + let Some(func_id) = collect_function( + &mut context.def_interner, + &mut self.def_collector.def_map, + &function, + module, + self.file_id, + &mut errors, + ) else { + continue; + }; // Now link this func_id to a crate level map with the noir function and the module id // Encountering a NoirFunction, we retrieve it's module_data to get the namespace @@ -253,19 +267,6 @@ impl<'a> ModCollector<'a> { // With this method we iterate each function in the Crate and not each module // This may not be great because we have to pull the module_data for each function unresolved_functions.push_fn(self.module_id, func_id, function); - - // Add function to scope/ns of the module - let result = self.def_collector.def_map.modules[self.module_id.0] - .declare_function(name, visibility, func_id); - - if let Err((first_def, second_def)) = result { - let error = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::Function, - first_def, - second_def, - }; - errors.push((error.into(), self.file_id)); - } } self.def_collector.items.functions.push(unresolved_functions); @@ -283,90 +284,21 @@ impl<'a> ModCollector<'a> { ) -> Vec<(CompilationError, FileId)> { let mut definition_errors = vec![]; for struct_definition in types { - self.check_duplicate_field_names(&struct_definition, &mut definition_errors); - - let name = struct_definition.name.clone(); - - let unresolved = UnresolvedStruct { - file_id: self.file_id, - module_id: self.module_id, - struct_def: struct_definition, - }; - - let resolved_generics = context.resolve_generics( - &unresolved.struct_def.generics, - &mut definition_errors, + if let Some((id, the_struct)) = collect_struct( + &mut context.def_interner, + &mut self.def_collector.def_map, + struct_definition, self.file_id, - ); - - // Create the corresponding module for the struct namespace - let id = match self.push_child_module( - context, - &name, - Location::new(name.span(), self.file_id), - false, - false, + self.module_id, + krate, + &mut definition_errors, ) { - Ok(module_id) => context.def_interner.new_struct( - &unresolved, - resolved_generics, - krate, - module_id.local_id, - self.file_id, - ), - Err(error) => { - definition_errors.push((error.into(), self.file_id)); - continue; - } - }; - - // Add the struct to scope so its path can be looked up later - let result = self.def_collector.def_map.modules[self.module_id.0] - .declare_struct(name.clone(), id); - - if let Err((first_def, second_def)) = result { - let error = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::TypeDefinition, - first_def, - second_def, - }; - definition_errors.push((error.into(), self.file_id)); - } - - // And store the TypeId -> StructType mapping somewhere it is reachable - self.def_collector.items.types.insert(id, unresolved); - - if context.def_interner.is_in_lsp_mode() { - let parent_module_id = ModuleId { krate, local_id: self.module_id }; - context.def_interner.register_struct(id, name.to_string(), parent_module_id); + self.def_collector.items.types.insert(id, the_struct); } } definition_errors } - fn check_duplicate_field_names( - &self, - struct_definition: &NoirStruct, - definition_errors: &mut Vec<(CompilationError, FileId)>, - ) { - let mut seen_field_names = std::collections::HashSet::new(); - for (field_name, _) in &struct_definition.fields { - if seen_field_names.insert(field_name) { - continue; - } - - let previous_field_name = *seen_field_names.get(field_name).unwrap(); - definition_errors.push(( - DefCollectorErrorKind::DuplicateField { - first_def: previous_field_name.clone(), - second_def: field_name.clone(), - } - .into(), - self.file_id, - )); - } - } - /// Collect any type aliases definitions declared within the ast. /// Returns a vector of errors if any type aliases were already defined. fn collect_type_aliases( @@ -386,7 +318,8 @@ impl<'a> ModCollector<'a> { type_alias_def: type_alias, }; - let resolved_generics = context.resolve_generics( + let resolved_generics = Context::resolve_generics( + &context.def_interner, &unresolved.type_alias_def.generics, &mut errors, self.file_id, @@ -436,6 +369,8 @@ impl<'a> ModCollector<'a> { context, &name, Location::new(name.span(), self.file_id), + Vec::new(), + Vec::new(), false, false, ) { @@ -587,8 +522,12 @@ impl<'a> ModCollector<'a> { } } - let resolved_generics = - context.resolve_generics(&trait_definition.generics, &mut errors, self.file_id); + let resolved_generics = Context::resolve_generics( + &context.def_interner, + &trait_definition.generics, + &mut errors, + self.file_id, + ); let unresolved = UnresolvedTrait { file_id: self.file_id, @@ -619,6 +558,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, crate_id: CrateId, + parent_module_id: LocalModuleId, submodules: Vec, file_id: FileId, macro_processors: &[&dyn MacroProcessor], @@ -629,10 +569,21 @@ impl<'a> ModCollector<'a> { context, &submodule.name, Location::new(submodule.name.span(), file_id), + submodule.outer_attributes.clone(), + submodule.contents.inner_attributes.clone(), true, submodule.is_contract, ) { Ok(child) => { + self.collect_attributes( + submodule.outer_attributes, + file_id, + child.local_id, + file_id, + parent_module_id, + false, + ); + errors.extend(collect_defs( self.def_collector, submodule.contents, @@ -657,8 +608,10 @@ impl<'a> ModCollector<'a> { fn parse_module_declaration( &mut self, context: &mut Context, - mod_decl: &ModuleDeclaration, + mod_decl: ModuleDeclaration, crate_id: CrateId, + parent_file_id: FileId, + parent_module_id: LocalModuleId, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; @@ -720,10 +673,21 @@ impl<'a> ModCollector<'a> { context, &mod_decl.ident, Location::new(Span::empty(0), child_file_id), + mod_decl.outer_attributes.clone(), + ast.inner_attributes.clone(), true, false, ) { Ok(child_mod_id) => { + self.collect_attributes( + mod_decl.outer_attributes, + child_file_id, + child_mod_id.local_id, + parent_file_id, + parent_module_id, + false, + ); + // Track that the "foo" in `mod foo;` points to the module "foo" context.def_interner.add_module_reference(child_mod_id, location); @@ -746,71 +710,28 @@ impl<'a> ModCollector<'a> { /// Add a child module to the current def_map. /// On error this returns None and pushes to `errors` + #[allow(clippy::too_many_arguments)] fn push_child_module( &mut self, context: &mut Context, mod_name: &Ident, mod_location: Location, + outer_attributes: Vec, + inner_attributes: Vec, add_to_parent_scope: bool, is_contract: bool, ) -> Result { - let parent = Some(self.module_id); - - // Note: the difference between `location` and `mod_location` is: - // - `mod_location` will point to either the token "foo" in `mod foo { ... }` - // if it's an inline module, or the first char of a the file if it's an external module. - // - `location` will always point to the token "foo" in `mod foo` regardless of whether - // it's inline or external. - // Eventually the location put in `ModuleData` is used for codelenses about `contract`s, - // so we keep using `location` so that it continues to work as usual. - let location = Location::new(mod_name.span(), mod_location.file); - let new_module = ModuleData::new(parent, location, is_contract); - let module_id = self.def_collector.def_map.modules.insert(new_module); - - let modules = &mut self.def_collector.def_map.modules; - - // Update the parent module to reference the child - modules[self.module_id.0].children.insert(mod_name.clone(), LocalModuleId(module_id)); - - let mod_id = ModuleId { - krate: self.def_collector.def_map.krate, - local_id: LocalModuleId(module_id), - }; - - // Add this child module into the scope of the parent module as a module definition - // module definitions are definitions which can only exist at the module level. - // ModuleDefinitionIds can be used across crates since they contain the CrateId - // - // We do not want to do this in the case of struct modules (each struct type corresponds - // to a child module containing its methods) since the module name should not shadow - // the struct name. - if add_to_parent_scope { - if let Err((first_def, second_def)) = - modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id) - { - let err = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::Module, - first_def, - second_def, - }; - return Err(err); - } - - context.def_interner.add_module_attributes( - mod_id, - ModuleAttributes { - name: mod_name.0.contents.clone(), - location: mod_location, - parent: Some(self.module_id), - }, - ); - - if context.def_interner.is_in_lsp_mode() { - context.def_interner.register_module(mod_id, mod_name.0.contents.clone()); - } - } - - Ok(mod_id) + push_child_module( + &mut context.def_interner, + &mut self.def_collector.def_map, + self.module_id, + mod_name, + mod_location, + outer_attributes, + inner_attributes, + add_to_parent_scope, + is_contract, + ) } fn resolve_associated_constant_type( @@ -831,6 +752,212 @@ impl<'a> ModCollector<'a> { } } +/// Add a child module to the current def_map. +/// On error this returns None and pushes to `errors` +#[allow(clippy::too_many_arguments)] +fn push_child_module( + interner: &mut NodeInterner, + def_map: &mut CrateDefMap, + parent: LocalModuleId, + mod_name: &Ident, + mod_location: Location, + outer_attributes: Vec, + inner_attributes: Vec, + add_to_parent_scope: bool, + is_contract: bool, +) -> Result { + // Note: the difference between `location` and `mod_location` is: + // - `mod_location` will point to either the token "foo" in `mod foo { ... }` + // if it's an inline module, or the first char of a the file if it's an external module. + // - `location` will always point to the token "foo" in `mod foo` regardless of whether + // it's inline or external. + // Eventually the location put in `ModuleData` is used for codelenses about `contract`s, + // so we keep using `location` so that it continues to work as usual. + let location = Location::new(mod_name.span(), mod_location.file); + let new_module = + ModuleData::new(Some(parent), location, outer_attributes, inner_attributes, is_contract); + + let module_id = def_map.modules.insert(new_module); + let modules = &mut def_map.modules; + + // Update the parent module to reference the child + modules[parent.0].children.insert(mod_name.clone(), LocalModuleId(module_id)); + + let mod_id = ModuleId { krate: def_map.krate, local_id: LocalModuleId(module_id) }; + + // Add this child module into the scope of the parent module as a module definition + // module definitions are definitions which can only exist at the module level. + // ModuleDefinitionIds can be used across crates since they contain the CrateId + // + // We do not want to do this in the case of struct modules (each struct type corresponds + // to a child module containing its methods) since the module name should not shadow + // the struct name. + if add_to_parent_scope { + if let Err((first_def, second_def)) = + modules[parent.0].declare_child_module(mod_name.to_owned(), mod_id) + { + let err = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::Module, + first_def, + second_def, + }; + return Err(err); + } + + interner.add_module_attributes( + mod_id, + ModuleAttributes { + name: mod_name.0.contents.clone(), + location: mod_location, + parent: Some(parent), + }, + ); + + if interner.is_in_lsp_mode() { + interner.register_module(mod_id, mod_name.0.contents.clone()); + } + } + + Ok(mod_id) +} + +pub fn collect_function( + interner: &mut NodeInterner, + def_map: &mut CrateDefMap, + function: &NoirFunction, + module: ModuleId, + file: FileId, + errors: &mut Vec<(CompilationError, FileId)>, +) -> Option { + if let Some(field) = function.attributes().get_field_attribute() { + if !is_native_field(&field) { + return None; + } + } + + let module_data = &mut def_map.modules[module.local_id.0]; + + let is_test = function.def.attributes.is_test_function(); + let is_entry_point_function = if module_data.is_contract { + function.attributes().is_contract_entry_point() + } else { + function.name() == MAIN_FUNCTION + }; + + let name = function.name_ident().clone(); + let func_id = interner.push_empty_fn(); + let visibility = function.def.visibility; + let location = Location::new(function.span(), file); + interner.push_function(func_id, &function.def, module, location); + if interner.is_in_lsp_mode() && !function.def.is_test() { + interner.register_function(func_id, &function.def); + } + + if !is_test && !is_entry_point_function { + let item = UnusedItem::Function(func_id); + interner.usage_tracker.add_unused_item(module, name.clone(), item, visibility); + } + + // Add function to scope/ns of the module + let result = def_map.modules[module.local_id.0].declare_function(name, visibility, func_id); + if let Err((first_def, second_def)) = result { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::Function, + first_def, + second_def, + }; + errors.push((error.into(), file)); + } + Some(func_id) +} + +pub fn collect_struct( + interner: &mut NodeInterner, + def_map: &mut CrateDefMap, + struct_definition: NoirStruct, + file_id: FileId, + module_id: LocalModuleId, + krate: CrateId, + definition_errors: &mut Vec<(CompilationError, FileId)>, +) -> Option<(StructId, UnresolvedStruct)> { + check_duplicate_field_names(&struct_definition, file_id, definition_errors); + + let name = struct_definition.name.clone(); + + let unresolved = UnresolvedStruct { file_id, module_id, struct_def: struct_definition }; + + let resolved_generics = Context::resolve_generics( + interner, + &unresolved.struct_def.generics, + definition_errors, + file_id, + ); + + // Create the corresponding module for the struct namespace + let location = Location::new(name.span(), file_id); + let id = match push_child_module( + interner, + def_map, + module_id, + &name, + location, + Vec::new(), + Vec::new(), + false, + false, + ) { + Ok(module_id) => { + interner.new_struct(&unresolved, resolved_generics, krate, module_id.local_id, file_id) + } + Err(error) => { + definition_errors.push((error.into(), file_id)); + return None; + } + }; + + // Add the struct to scope so its path can be looked up later + let result = def_map.modules[module_id.0].declare_struct(name.clone(), id); + + if let Err((first_def, second_def)) = result { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TypeDefinition, + first_def, + second_def, + }; + definition_errors.push((error.into(), file_id)); + } + + if interner.is_in_lsp_mode() { + let parent_module_id = ModuleId { krate, local_id: module_id }; + interner.register_struct(id, name.to_string(), parent_module_id); + } + + Some((id, unresolved)) +} + +pub fn collect_impl( + interner: &mut NodeInterner, + items: &mut CollectedItems, + r#impl: TypeImpl, + file_id: FileId, + module_id: ModuleId, +) { + let mut unresolved_functions = + UnresolvedFunctions { file_id, functions: Vec::new(), trait_id: None, self_type: None }; + + for (mut method, _) in r#impl.methods { + let func_id = interner.push_empty_fn(); + method.def.where_clause.extend(r#impl.where_clause.clone()); + let location = Location::new(method.span(), file_id); + interner.push_function(func_id, &method.def, module_id, location); + unresolved_functions.push_fn(module_id.local_id, func_id, method); + } + + let key = (r#impl.object_type, module_id.local_id); + let methods = items.impls.entry(key).or_default(); + methods.push((r#impl.generics, r#impl.type_span, unresolved_functions)); +} + fn find_module( file_manager: &FileManager, anchor: FileId, @@ -989,6 +1116,29 @@ pub(crate) fn collect_global( (global, error) } +fn check_duplicate_field_names( + struct_definition: &NoirStruct, + file: FileId, + definition_errors: &mut Vec<(CompilationError, FileId)>, +) { + let mut seen_field_names = std::collections::HashSet::new(); + for (field_name, _) in &struct_definition.fields { + if seen_field_names.insert(field_name) { + continue; + } + + let previous_field_name = *seen_field_names.get(field_name).unwrap(); + definition_errors.push(( + DefCollectorErrorKind::DuplicateField { + first_def: previous_field_name.clone(), + second_def: field_name.clone(), + } + .into(), + file, + )); + } +} + #[cfg(test)] mod find_module_tests { use super::*; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs index 758b4cf6e5c..a1c4d04cb30 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -111,7 +111,13 @@ impl CrateDefMap { // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); - let root = modules.insert(ModuleData::new(None, location, false)); + let root = modules.insert(ModuleData::new( + None, + location, + Vec::new(), + ast.inner_attributes.clone(), + false, + )); let def_map = CrateDefMap { root: LocalModuleId(root), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs index f9542094be7..e829df3572c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -5,6 +5,7 @@ use noirc_errors::Location; use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs}; use crate::ast::{Ident, ItemVisibility}; use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; +use crate::token::SecondaryAttribute; /// Contains the actual contents of a module: its parent (if one exists), /// children, and scope with all definitions defined within the scope. @@ -24,10 +25,25 @@ pub struct ModuleData { /// True if this module is a `contract Foo { ... }` module containing contract functions pub is_contract: bool, + + pub outer_attributes: Vec, + pub inner_attributes: Vec, } impl ModuleData { - pub fn new(parent: Option, location: Location, is_contract: bool) -> ModuleData { + pub fn new( + parent: Option, + location: Location, + outer_attributes: Vec, + inner_attributes: Vec, + is_contract: bool, + ) -> ModuleData { + let outer_attributes = outer_attributes.iter().filter_map(|attr| attr.as_custom()); + let outer_attributes = outer_attributes.map(|attr| attr.contents.to_string()).collect(); + + let inner_attributes = inner_attributes.iter().filter_map(|attr| attr.as_custom()); + let inner_attributes = inner_attributes.map(|attr| attr.contents.to_string()).collect(); + ModuleData { parent, children: HashMap::new(), @@ -35,6 +51,8 @@ impl ModuleData { definitions: ItemScope::default(), location, is_contract, + outer_attributes, + inner_attributes, } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/mod.rs index e4f000778d1..c631edfa889 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/mod.rs @@ -272,14 +272,14 @@ impl Context<'_, '_> { /// Each result is returned in a list rather than returned as a single result as to allow /// definition collection to provide an error for each ill-formed numeric generic. pub(crate) fn resolve_generics( - &mut self, + interner: &NodeInterner, generics: &UnresolvedGenerics, errors: &mut Vec<(CompilationError, FileId)>, file_id: FileId, ) -> Generics { vecmap(generics, |generic| { // Map the generic to a fresh type variable - let id = self.def_interner.next_type_variable_id(); + let id = interner.next_type_variable_id(); let type_var = TypeVariable::unbound(id); let ident = generic.ident(); let span = ident.0.span(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index c2038c646b5..e74468bdf18 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -20,8 +20,8 @@ pub enum ResolverError { DuplicateDefinition { name: String, first_span: Span, second_span: Span }, #[error("Unused variable")] UnusedVariable { ident: Ident }, - #[error("Unused import")] - UnusedImport { ident: Ident }, + #[error("Unused {item_type}")] + UnusedItem { ident: Ident, item_type: &'static str }, #[error("Could not find variable in this scope")] VariableNotDeclared { name: String, span: Span }, #[error("path is not an identifier")] @@ -158,12 +158,12 @@ impl<'a> From<&'a ResolverError> for Diagnostic { diagnostic.unnecessary = true; diagnostic } - ResolverError::UnusedImport { ident } => { + ResolverError::UnusedItem { ident, item_type } => { let name = &ident.0.contents; let mut diagnostic = Diagnostic::simple_warning( - format!("unused import {name}"), - "unused import ".to_string(), + format!("unused {item_type} {name}"), + format!("unused {item_type}"), ident.span(), ); diagnostic.unnecessary = true; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs index 7fa33746f31..8e3baef1d00 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs @@ -10,6 +10,7 @@ use crate::graph::CrateId; use crate::hir::def_map::LocalModuleId; use crate::macros_api::{BlockExpression, StructId}; use crate::node_interner::{ExprId, NodeInterner, TraitId, TraitImplId}; +use crate::token::CustomAtrribute; use crate::{ResolvedGeneric, Type}; /// A Hir function is a block expression with a list of statements. @@ -166,7 +167,7 @@ pub struct FuncMeta { pub self_type: Option, /// Custom attributes attached to this function. - pub custom_attributes: Vec, + pub custom_attributes: Vec, } #[derive(Debug, Clone)] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs index 638003d3fcd..113a4fb3888 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs @@ -259,7 +259,6 @@ impl StructType { /// created. Therefore, this method is used to set the fields once they /// become known. pub fn set_fields(&mut self, fields: Vec<(Ident, Type)>) { - assert!(self.fields.is_empty()); self.fields = fields; } @@ -1467,21 +1466,13 @@ impl Type { /// equal to the other type in the process. When comparing types, unification /// (including try_unify) are almost always preferred over Type::eq as unification /// will correctly handle generic types. - pub fn unify( - &self, - expected: &Type, - errors: &mut Vec, - make_error: impl FnOnce() -> TypeCheckError, - ) { + pub fn unify(&self, expected: &Type) -> Result<(), UnificationError> { let mut bindings = TypeBindings::new(); - match self.try_unify(expected, &mut bindings) { - Ok(()) => { - // Commit any type bindings on success - Self::apply_type_bindings(bindings); - } - Err(UnificationError) => errors.push(make_error()), - } + self.try_unify(expected, &mut bindings).map(|()| { + // Commit any type bindings on success + Self::apply_type_bindings(bindings); + }) } /// `try_unify` is a bit of a misnomer since although errors are not committed, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/errors.rs index be5180a777b..2440109af15 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/errors.rs @@ -20,6 +20,8 @@ pub enum LexerErrorKind { IntegerLiteralTooLarge { span: Span, limit: String }, #[error("{:?} is not a valid attribute", found)] MalformedFuncAttribute { span: Span, found: String }, + #[error("{:?} is not a valid inner attribute", found)] + InvalidInnerAttribute { span: Span, found: String }, #[error("Logical and used instead of bitwise and")] LogicalAnd { span: Span }, #[error("Unterminated block comment")] @@ -57,6 +59,7 @@ impl LexerErrorKind { LexerErrorKind::InvalidIntegerLiteral { span, .. } => *span, LexerErrorKind::IntegerLiteralTooLarge { span, .. } => *span, LexerErrorKind::MalformedFuncAttribute { span, .. } => *span, + LexerErrorKind::InvalidInnerAttribute { span, .. } => *span, LexerErrorKind::LogicalAnd { span } => *span, LexerErrorKind::UnterminatedBlockComment { span } => *span, LexerErrorKind::UnterminatedStringLiteral { span } => *span, @@ -103,6 +106,11 @@ impl LexerErrorKind { format!(" {found} is not a valid attribute"), *span, ), + LexerErrorKind::InvalidInnerAttribute { span, found } => ( + "Invalid inner attribute".to_string(), + format!(" {found} is not a valid inner attribute"), + *span, + ), LexerErrorKind::LogicalAnd { span } => ( "Noir has no logical-and (&&) operator since short-circuiting is much less efficient when compiling to circuits".to_string(), "Try `&` instead, or use `if` only if you require short-circuiting".to_string(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs index 0afcb02caac..b7492396c90 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs @@ -286,6 +286,13 @@ impl<'a> Lexer<'a> { fn eat_attribute(&mut self) -> SpannedTokenResult { let start = self.position; + let is_inner = if self.peek_char_is('!') { + self.next_char(); + true + } else { + false + }; + if !self.peek_char_is('[') { return Err(LexerErrorKind::UnexpectedCharacter { span: Span::single_char(self.position), @@ -295,8 +302,12 @@ impl<'a> Lexer<'a> { } self.next_char(); + let contents_start = self.position + 1; + let word = self.eat_while(None, |ch| ch != ']'); + let contents_end = self.position; + if !self.peek_char_is(']') { return Err(LexerErrorKind::UnexpectedCharacter { span: Span::single_char(self.position), @@ -308,9 +319,23 @@ impl<'a> Lexer<'a> { let end = self.position; - let attribute = Attribute::lookup_attribute(&word, Span::inclusive(start, end))?; - - Ok(attribute.into_span(start, end)) + let span = Span::inclusive(start, end); + let contents_span = Span::inclusive(contents_start, contents_end); + + let attribute = Attribute::lookup_attribute(&word, span, contents_span)?; + if is_inner { + match attribute { + Attribute::Function(attribute) => Err(LexerErrorKind::InvalidInnerAttribute { + span: Span::from(start..end), + found: attribute.to_string(), + }), + Attribute::Secondary(attribute) => { + Ok(Token::InnerAttribute(attribute).into_span(start, end)) + } + } + } else { + Ok(Token::Attribute(attribute).into_span(start, end)) + } } //XXX(low): Can increase performance if we use iterator semantic and utilize some of the methods on String. See below @@ -682,7 +707,7 @@ mod tests { use iter_extended::vecmap; use super::*; - use crate::token::{FunctionAttribute, SecondaryAttribute, TestScope}; + use crate::token::{CustomAtrribute, FunctionAttribute, SecondaryAttribute, TestScope}; #[test] fn test_single_double_char() { @@ -810,9 +835,11 @@ mod tests { let token = lexer.next_token().unwrap(); assert_eq!( token.token(), - &Token::Attribute(Attribute::Secondary(SecondaryAttribute::Custom( - "custom(hello)".to_string() - ))) + &Token::Attribute(Attribute::Secondary(SecondaryAttribute::Custom(CustomAtrribute { + contents: "custom(hello)".to_string(), + span: Span::from(0..16), + contents_span: Span::from(2..15) + }))) ); } @@ -898,6 +925,22 @@ mod tests { assert_eq!(sub_string, "test(invalid_scope)"); } + #[test] + fn test_inner_attribute() { + let input = r#"#![something]"#; + let mut lexer = Lexer::new(input); + + let token = lexer.next_token().unwrap(); + assert_eq!( + token.token(), + &Token::InnerAttribute(SecondaryAttribute::Custom(CustomAtrribute { + contents: "something".to_string(), + span: Span::from(0..13), + contents_span: Span::from(3..12), + })) + ); + } + #[test] fn test_int_type() { let input = "u16 i16 i108 u104.5"; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs index 585e22ce92b..7b805b5fd8d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs @@ -27,6 +27,7 @@ pub enum BorrowedToken<'input> { Keyword(Keyword), IntType(IntType), Attribute(Attribute), + InnerAttribute(SecondaryAttribute), LineComment(&'input str, Option), BlockComment(&'input str, Option), Quote(&'input Tokens), @@ -132,6 +133,7 @@ pub enum Token { Keyword(Keyword), IntType(IntType), Attribute(Attribute), + InnerAttribute(SecondaryAttribute), LineComment(String, Option), BlockComment(String, Option), // A `quote { ... }` along with the tokens in its token stream. @@ -244,6 +246,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::RawStr(ref b, hashes) => BorrowedToken::RawStr(b, *hashes), Token::Keyword(k) => BorrowedToken::Keyword(*k), Token::Attribute(ref a) => BorrowedToken::Attribute(a.clone()), + Token::InnerAttribute(ref a) => BorrowedToken::InnerAttribute(a.clone()), Token::LineComment(ref s, _style) => BorrowedToken::LineComment(s, *_style), Token::BlockComment(ref s, _style) => BorrowedToken::BlockComment(s, *_style), Token::Quote(stream) => BorrowedToken::Quote(stream), @@ -363,6 +366,7 @@ impl fmt::Display for Token { } Token::Keyword(k) => write!(f, "{k}"), Token::Attribute(ref a) => write!(f, "{a}"), + Token::InnerAttribute(ref a) => write!(f, "#![{a}]"), Token::LineComment(ref s, _style) => write!(f, "//{s}"), Token::BlockComment(ref s, _style) => write!(f, "/*{s}*/"), Token::Quote(ref stream) => { @@ -428,6 +432,7 @@ pub enum TokenKind { Literal, Keyword, Attribute, + InnerAttribute, Quote, QuotedType, InternedExpr, @@ -445,6 +450,7 @@ impl fmt::Display for TokenKind { TokenKind::Literal => write!(f, "literal"), TokenKind::Keyword => write!(f, "keyword"), TokenKind::Attribute => write!(f, "attribute"), + TokenKind::InnerAttribute => write!(f, "inner attribute"), TokenKind::Quote => write!(f, "quote"), TokenKind::QuotedType => write!(f, "quoted type"), TokenKind::InternedExpr => write!(f, "interned expr"), @@ -467,6 +473,7 @@ impl Token { | Token::FmtStr(_) => TokenKind::Literal, Token::Keyword(_) => TokenKind::Keyword, Token::Attribute(_) => TokenKind::Attribute, + Token::InnerAttribute(_) => TokenKind::InnerAttribute, Token::UnquoteMarker(_) => TokenKind::UnquoteMarker, Token::Quote(_) => TokenKind::Quote, Token::QuotedType(_) => TokenKind::QuotedType, @@ -697,7 +704,11 @@ impl fmt::Display for Attribute { impl Attribute { /// If the string is a fixed attribute return that, else /// return the custom attribute - pub(crate) fn lookup_attribute(word: &str, span: Span) -> Result { + pub(crate) fn lookup_attribute( + word: &str, + span: Span, + contents_span: Span, + ) -> Result { let word_segments: Vec<&str> = word .split(|c| c == '(' || c == ')') .filter(|string_segment| !string_segment.is_empty()) @@ -770,11 +781,15 @@ impl Attribute { ["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs), tokens => { tokens.iter().try_for_each(|token| validate(token))?; - Attribute::Secondary(SecondaryAttribute::Custom(word.to_owned())) + Attribute::Secondary(SecondaryAttribute::Custom(CustomAtrribute { + contents: word.to_owned(), + span, + contents_span, + })) } }; - Ok(Token::Attribute(attribute)) + Ok(attribute) } } @@ -863,17 +878,26 @@ pub enum SecondaryAttribute { ContractLibraryMethod, Export, Field(String), - Custom(String), + Custom(CustomAtrribute), Abi(String), /// A variable-argument comptime function. Varargs, } +#[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] +pub struct CustomAtrribute { + pub contents: String, + // The span of the entire attribute, including leading `#[` and trailing `]` + pub span: Span, + // The span for the attribute contents (what's inside `#[...]`) + pub contents_span: Span, +} + impl SecondaryAttribute { - pub(crate) fn as_custom(&self) -> Option<&str> { - if let Self::Custom(str) = self { - Some(str) + pub(crate) fn as_custom(&self) -> Option<&CustomAtrribute> { + if let Self::Custom(attribute) = self { + Some(attribute) } else { None } @@ -887,7 +911,7 @@ impl fmt::Display for SecondaryAttribute { SecondaryAttribute::Deprecated(Some(ref note)) => { write!(f, r#"#[deprecated("{note}")]"#) } - SecondaryAttribute::Custom(ref k) => write!(f, "#[{k}]"), + SecondaryAttribute::Custom(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})]"), @@ -916,9 +940,8 @@ impl AsRef for SecondaryAttribute { match self { SecondaryAttribute::Deprecated(Some(string)) => string, SecondaryAttribute::Deprecated(None) => "", - SecondaryAttribute::Custom(string) - | SecondaryAttribute::Field(string) - | SecondaryAttribute::Abi(string) => string, + SecondaryAttribute::Custom(attribute) => &attribute.contents, + SecondaryAttribute::Field(string) | SecondaryAttribute::Abi(string) => string, SecondaryAttribute::ContractLibraryMethod => "", SecondaryAttribute::Export => "", SecondaryAttribute::Varargs => "", diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index 4a73df6a15f..aa51779d24b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -655,7 +655,7 @@ impl Default for NodeInterner { auto_import_names: HashMap::default(), comptime_scopes: vec![HashMap::default()], trait_impl_associated_types: HashMap::default(), - usage_tracker: UsageTracker::default(), + usage_tracker: UsageTracker::new(), } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/mod.rs index c82906b69a2..596d15176bc 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/mod.rs @@ -16,7 +16,7 @@ use crate::ast::{ NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Recoverable, StatementKind, TypeImpl, UseTree, }; -use crate::token::{Keyword, Token}; +use crate::token::{Keyword, SecondaryAttribute, Token}; use chumsky::prelude::*; use chumsky::primitive::Container; @@ -41,6 +41,7 @@ pub enum TopLevelStatement { TypeAlias(NoirTypeAlias), SubModule(ParsedSubModule), Global(LetStatement), + InnerAttribute(SecondaryAttribute), Error, } @@ -57,6 +58,7 @@ impl TopLevelStatement { TopLevelStatement::TypeAlias(t) => Some(ItemKind::TypeAlias(t)), TopLevelStatement::SubModule(s) => Some(ItemKind::Submodules(s)), TopLevelStatement::Global(c) => Some(ItemKind::Global(c)), + TopLevelStatement::InnerAttribute(a) => Some(ItemKind::InnerAttribute(a)), TopLevelStatement::Error => None, } } @@ -247,6 +249,8 @@ pub struct SortedModule { /// Full submodules as in `mod foo { ... definitions ... }` pub submodules: Vec, + + pub inner_attributes: Vec, } impl std::fmt::Display for SortedModule { @@ -309,6 +313,7 @@ impl ParsedModule { ItemKind::Global(global) => module.push_global(global), ItemKind::ModuleDecl(mod_name) => module.push_module_decl(mod_name), ItemKind::Submodules(submodule) => module.push_submodule(submodule.into_sorted()), + ItemKind::InnerAttribute(attribute) => module.inner_attributes.push(attribute), } } @@ -334,6 +339,7 @@ pub enum ItemKind { Global(LetStatement), ModuleDecl(ModuleDeclaration), Submodules(ParsedSubModule), + InnerAttribute(SecondaryAttribute), } /// A submodule defined via `mod name { contents }` in some larger file. @@ -342,6 +348,7 @@ pub enum ItemKind { pub struct ParsedSubModule { pub name: Ident, pub contents: ParsedModule, + pub outer_attributes: Vec, pub is_contract: bool, } @@ -350,6 +357,7 @@ impl ParsedSubModule { SortedSubModule { name: self.name, contents: self.contents.into_sorted(), + outer_attributes: self.outer_attributes, is_contract: self.is_contract, } } @@ -371,6 +379,7 @@ impl std::fmt::Display for SortedSubModule { pub struct SortedSubModule { pub name: Ident, pub contents: SortedModule, + pub outer_attributes: Vec, pub is_contract: bool, } @@ -512,6 +521,7 @@ impl std::fmt::Display for TopLevelStatement { TopLevelStatement::TypeAlias(t) => t.fmt(f), TopLevelStatement::SubModule(s) => s.fmt(f), TopLevelStatement::Global(c) => c.fmt(f), + TopLevelStatement::InnerAttribute(a) => write!(f, "#![{}]", a), TopLevelStatement::Error => write!(f, "error"), } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs index bead1e69006..2bc7a88c6c5 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs @@ -26,6 +26,7 @@ use self::path::as_trait_path; use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable}; use self::types::{generic_type_args, maybe_comp_time}; +use attributes::{attributes, inner_attribute, validate_secondary_attributes}; pub use types::parse_type; use visibility::visibility_modifier; @@ -91,7 +92,7 @@ pub fn parse_program(source_program: &str) -> (ParsedModule, Vec) { let (module, mut parsing_errors) = program().parse_recovery_verbose(tokens); parsing_errors.extend(lexing_errors.into_iter().map(Into::into)); - let parsed_module = module.unwrap_or(ParsedModule { items: vec![] }); + let parsed_module = module.unwrap_or_default(); if cfg!(feature = "experimental_parser") { for parsed_item in &parsed_module.items { @@ -215,6 +216,7 @@ fn top_level_statement<'a>( module_declaration().then_ignore(force(just(Token::Semicolon))), use_statement().then_ignore(force(just(Token::Semicolon))), global_declaration().then_ignore(force(just(Token::Semicolon))), + inner_attribute().map(TopLevelStatement::InnerAttribute), )) .recover_via(top_level_statement_recovery()) } @@ -287,25 +289,39 @@ fn global_declaration() -> impl NoirParser { /// submodule: 'mod' ident '{' module '}' fn submodule(module_parser: impl NoirParser) -> impl NoirParser { - keyword(Keyword::Mod) - .ignore_then(ident()) + attributes() + .then_ignore(keyword(Keyword::Mod)) + .then(ident()) .then_ignore(just(Token::LeftBrace)) .then(module_parser) .then_ignore(just(Token::RightBrace)) - .map(|(name, contents)| { - TopLevelStatement::SubModule(ParsedSubModule { name, contents, is_contract: false }) + .validate(|((attributes, name), contents), span, emit| { + let attributes = validate_secondary_attributes(attributes, span, emit); + TopLevelStatement::SubModule(ParsedSubModule { + name, + contents, + outer_attributes: attributes, + is_contract: false, + }) }) } /// contract: 'contract' ident '{' module '}' fn contract(module_parser: impl NoirParser) -> impl NoirParser { - keyword(Keyword::Contract) - .ignore_then(ident()) + attributes() + .then_ignore(keyword(Keyword::Contract)) + .then(ident()) .then_ignore(just(Token::LeftBrace)) .then(module_parser) .then_ignore(just(Token::RightBrace)) - .map(|(name, contents)| { - TopLevelStatement::SubModule(ParsedSubModule { name, contents, is_contract: true }) + .validate(|((attributes, name), contents), span, emit| { + let attributes = validate_secondary_attributes(attributes, span, emit); + TopLevelStatement::SubModule(ParsedSubModule { + name, + contents, + outer_attributes: attributes, + is_contract: true, + }) }) } @@ -434,9 +450,12 @@ fn optional_type_annotation<'a>() -> impl NoirParser + 'a { } fn module_declaration() -> impl NoirParser { - keyword(Keyword::Mod) - .ignore_then(ident()) - .map(|ident| TopLevelStatement::Module(ModuleDeclaration { ident })) + attributes().then_ignore(keyword(Keyword::Mod)).then(ident()).validate( + |(attributes, ident), span, emit| { + let attributes = validate_secondary_attributes(attributes, span, emit); + TopLevelStatement::Module(ModuleDeclaration { ident, outer_attributes: attributes }) + }, + ) } fn use_statement() -> impl NoirParser { @@ -1522,9 +1541,22 @@ mod test { #[test] fn parse_module_declaration() { parse_with(module_declaration(), "mod foo").unwrap(); + parse_with(module_declaration(), "#[attr] mod foo").unwrap(); parse_with(module_declaration(), "mod 1").unwrap_err(); } + #[test] + fn parse_submodule_declaration() { + parse_with(submodule(module()), "mod foo {}").unwrap(); + parse_with(submodule(module()), "#[attr] mod foo {}").unwrap(); + } + + #[test] + fn parse_contract() { + parse_with(contract(module()), "contract foo {}").unwrap(); + parse_with(contract(module()), "#[attr] contract foo {}").unwrap(); + } + #[test] fn parse_use() { let valid_use_statements = [ diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs index 47add6f82e0..66d0ca29ca6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs @@ -67,3 +67,12 @@ pub(super) fn validate_secondary_attributes( struct_attributes } + +pub(super) fn inner_attribute() -> impl NoirParser { + token_kind(TokenKind::InnerAttribute).map(|token| match token { + Token::InnerAttribute(attribute) => attribute, + _ => unreachable!( + "Parser should have already errored due to token not being an inner attribute" + ), + }) +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index a30907211a3..04c4e414858 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -28,7 +28,8 @@ use crate::hir::def_collector::dc_crate::DefCollector; use crate::hir_def::expr::HirExpression; use crate::hir_def::stmt::HirStatement; use crate::monomorphization::monomorphize; -use crate::parser::ParserErrorReason; +use crate::parser::{ItemKind, ParserErrorReason}; +use crate::token::SecondaryAttribute; use crate::ParsedModule; use crate::{ hir::def_map::{CrateDefMap, LocalModuleId}, @@ -64,10 +65,28 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation remove_experimental_warnings(&mut errors); if !has_parser_error(&errors) { + let inner_attributes: Vec = program + .items + .iter() + .filter_map(|item| { + if let ItemKind::InnerAttribute(attribute) = &item.kind { + Some(attribute.clone()) + } else { + None + } + }) + .collect(); + // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); - let root = modules.insert(ModuleData::new(None, location, false)); + let root = modules.insert(ModuleData::new( + None, + location, + Vec::new(), + inner_attributes.clone(), + false, + )); let def_map = CrateDefMap { root: LocalModuleId(root), @@ -1362,7 +1381,7 @@ fn ban_mutable_globals() { fn deny_inline_attribute_on_unconstrained() { let src = r#" #[no_predicates] - unconstrained fn foo(x: Field, y: Field) { + unconstrained pub fn foo(x: Field, y: Field) { assert(x != y); } "#; @@ -1378,7 +1397,7 @@ fn deny_inline_attribute_on_unconstrained() { fn deny_fold_attribute_on_unconstrained() { let src = r#" #[fold] - unconstrained fn foo(x: Field, y: Field) { + unconstrained pub fn foo(x: Field, y: Field) { assert(x != y); } "#; @@ -1535,7 +1554,7 @@ fn struct_numeric_generic_in_function() { inner: u64 } - fn bar() { } + pub fn bar() { } "#; let errors = get_program_errors(src); assert_eq!(errors.len(), 1); @@ -1567,7 +1586,7 @@ fn struct_numeric_generic_in_struct() { #[test] fn bool_numeric_generic() { let src = r#" - fn read() -> Field { + pub fn read() -> Field { if N { 0 } else { @@ -1586,7 +1605,7 @@ fn bool_numeric_generic() { #[test] fn numeric_generic_binary_operation_type_mismatch() { let src = r#" - fn foo() -> bool { + pub fn foo() -> bool { let mut check: bool = true; check = N; check @@ -1603,7 +1622,7 @@ fn numeric_generic_binary_operation_type_mismatch() { #[test] fn bool_generic_as_loop_bound() { let src = r#" - fn read() { + pub fn read() { let mut fields = [0; N]; for i in 0..N { fields[i] = i + 1; @@ -1633,7 +1652,7 @@ fn bool_generic_as_loop_bound() { #[test] fn numeric_generic_in_function_signature() { let src = r#" - fn foo(arr: [Field; N]) -> [Field; N] { arr } + pub fn foo(arr: [Field; N]) -> [Field; N] { arr } "#; assert_no_errors(src); } @@ -1675,7 +1694,7 @@ fn normal_generic_as_array_length() { #[test] fn numeric_generic_as_param_type() { let src = r#" - fn foo(x: I) -> I { + pub fn foo(x: I) -> I { let _q: I = 5; x } @@ -1814,7 +1833,7 @@ fn numeric_generic_used_in_where_clause() { fn deserialize(fields: [Field; N]) -> Self; } - fn read() -> T where T: Deserialize { + pub fn read() -> T where T: Deserialize { let mut fields: [Field; N] = [0; N]; for i in 0..N { fields[i] = i as Field + 1; @@ -1828,12 +1847,12 @@ fn numeric_generic_used_in_where_clause() { #[test] fn numeric_generic_used_in_turbofish() { let src = r#" - fn double() -> u32 { + pub fn double() -> u32 { // Used as an expression N * 2 } - fn double_numeric_generics_test() { + pub fn double_numeric_generics_test() { // Example usage of a numeric generic arguments. assert(double::<9>() == 18); assert(double::<7 + 8>() == 30); @@ -1869,7 +1888,7 @@ fn normal_generic_used_when_numeric_expected_in_where_clause() { fn deserialize(fields: [Field; N]) -> Self; } - fn read() -> T where T: Deserialize { + pub fn read() -> T where T: Deserialize { T::deserialize([0, 1]) } "#; @@ -1885,7 +1904,7 @@ fn normal_generic_used_when_numeric_expected_in_where_clause() { fn deserialize(fields: [Field; N]) -> Self; } - fn read() -> T where T: Deserialize { + pub fn read() -> T where T: Deserialize { let mut fields: [Field; N] = [0; N]; for i in 0..N { fields[i] = i as Field + 1; @@ -2431,7 +2450,7 @@ fn use_super() { mod foo { use super::some_func; - fn bar() { + pub fn bar() { some_func(); } } @@ -2445,7 +2464,7 @@ fn use_super_in_path() { fn some_func() {} mod foo { - fn func() { + pub fn func() { super::some_func(); } } @@ -2736,7 +2755,7 @@ fn trait_constraint_on_tuple_type() { fn foo(self, x: A) -> bool; } - fn bar(x: (T, U), y: V) -> bool where (T, U): Foo { + pub fn bar(x: (T, U), y: V) -> bool where (T, U): Foo { x.foo(y) } @@ -3072,7 +3091,7 @@ fn trait_impl_for_a_type_that_implements_another_trait() { } } - fn use_it(t: T) -> i32 where T: Two { + pub fn use_it(t: T) -> i32 where T: Two { Two::two(t) } @@ -3112,7 +3131,7 @@ fn trait_impl_for_a_type_that_implements_another_trait_with_another_impl_used() } } - fn use_it(t: u32) -> i32 { + pub fn use_it(t: u32) -> i32 { Two::two(t) } @@ -3224,12 +3243,14 @@ fn errors_on_unused_private_import() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - let CompilationError::ResolverError(ResolverError::UnusedImport { ident }) = &errors[0].0 + let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) = + &errors[0].0 else { - panic!("Expected an unused import error"); + panic!("Expected an unused item error"); }; assert_eq!(ident.to_string(), "bar"); + assert_eq!(*item_type, "import"); } #[test] @@ -3258,12 +3279,14 @@ fn errors_on_unused_pub_crate_import() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - let CompilationError::ResolverError(ResolverError::UnusedImport { ident }) = &errors[0].0 + let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) = + &errors[0].0 else { - panic!("Expected an unused import error"); + panic!("Expected an unused item error"); }; assert_eq!(ident.to_string(), "bar"); + assert_eq!(*item_type, "import"); } #[test] @@ -3276,7 +3299,7 @@ fn warns_on_use_of_private_exported_item() { use bar::baz; - fn qux() { + pub fn qux() { baz(); } } @@ -3341,3 +3364,63 @@ fn warns_on_re_export_of_item_with_less_visibility() { ) )); } + +#[test] +fn unoquted_integer_as_integer_token() { + let src = r#" + trait Serialize { + fn serialize() {} + } + + #[attr] + pub fn foobar() {} + + fn attr(_f: FunctionDefinition) -> Quoted { + let serialized_len = 1; + // We are testing that when we unoqute $serialized_len, it's unquoted + // as the token `1` and not as something else that later won't be parsed correctly + // in the context of a generic argument. + quote { + impl Serialize<$serialized_len> for Field { + fn serialize() { } + } + } + } + + fn main() {} + "#; + + assert_no_errors(src); +} + +#[test] +fn errors_on_unused_function() { + let src = r#" + contract some_contract { + // This function is unused, but it's a contract entrypoint + // so it should not produce a warning + fn foo() -> pub Field { + 1 + } + } + + + fn foo() { + bar(); + } + + fn bar() {} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) = + &errors[0].0 + else { + panic!("Expected an unused item error"); + }; + + assert_eq!(ident.to_string(), "foo"); + assert_eq!(*item_type, "function"); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/usage_tracker.rs b/noir/noir-repo/compiler/noirc_frontend/src/usage_tracker.rs index d8b7b271734..836f9824436 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/usage_tracker.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/usage_tracker.rs @@ -1,26 +1,54 @@ -use std::collections::HashSet; +use std::collections::HashMap; -use rustc_hash::FxHashMap as HashMap; +use crate::{ + ast::{Ident, ItemVisibility}, + hir::def_map::ModuleId, + node_interner::FuncId, +}; -use crate::{ast::Ident, hir::def_map::ModuleId}; +#[derive(Debug)] +pub enum UnusedItem { + Import, + Function(FuncId), +} + +impl UnusedItem { + pub fn item_type(&self) -> &'static str { + match self { + UnusedItem::Import => "import", + UnusedItem::Function(_) => "function", + } + } +} -#[derive(Debug, Default)] +#[derive(Debug)] pub struct UsageTracker { - /// List of all unused imports in each module. Each time something is imported it's added - /// to the module's set. When it's used, it's removed. At the end of the program only unused imports remain. - unused_imports: HashMap>, + unused_items: HashMap>, } impl UsageTracker { - pub(crate) fn add_unused_import(&mut self, module_id: ModuleId, name: Ident) { - self.unused_imports.entry(module_id).or_default().insert(name); + pub(crate) fn new() -> Self { + Self { unused_items: HashMap::new() } + } + + pub(crate) fn add_unused_item( + &mut self, + module_id: ModuleId, + name: Ident, + item: UnusedItem, + visibility: ItemVisibility, + ) { + // Empty spans could come from implicitly injected imports, and we don't want to track those + if visibility != ItemVisibility::Public && name.span().start() < name.span().end() { + self.unused_items.entry(module_id).or_default().insert(name, item); + } } pub(crate) fn mark_as_used(&mut self, current_mod_id: ModuleId, name: &Ident) { - self.unused_imports.entry(current_mod_id).or_default().remove(name); + self.unused_items.entry(current_mod_id).or_default().remove(name); } - pub(crate) fn unused_imports(&self) -> &HashMap> { - &self.unused_imports + pub(crate) fn unused_items(&self) -> &HashMap> { + &self.unused_items } } diff --git a/noir/noir-repo/docs/docs/noir/concepts/comptime.md b/noir/noir-repo/docs/docs/noir/concepts/comptime.md index ed55a541fbd..ba078c763d0 100644 --- a/noir/noir-repo/docs/docs/noir/concepts/comptime.md +++ b/noir/noir-repo/docs/docs/noir/concepts/comptime.md @@ -183,7 +183,7 @@ comptime fn my_function_annotation(f: FunctionDefinition) { Anything returned from one of these functions will be inserted at top-level along with the original item. Note that expressions are not valid at top-level so you'll get an error trying to return `3` or similar just as if you tried to write a program containing `3; struct Foo {}`. -You can insert other top-level items such as traits, structs, or functions this way though. +You can insert other top-level items such as trait impls, structs, or functions this way though. For example, this is the mechanism used to insert additional trait implementations into the program when deriving a trait impl from a struct: #include_code derive-field-count-example noir_stdlib/src/meta/mod.nr rust diff --git a/noir/noir-repo/docs/docs/noir/standard_library/fmtstr.md b/noir/noir-repo/docs/docs/noir/standard_library/fmtstr.md new file mode 100644 index 00000000000..293793e23ff --- /dev/null +++ b/noir/noir-repo/docs/docs/noir/standard_library/fmtstr.md @@ -0,0 +1,13 @@ +--- +title: fmtstr +--- + +`fmtstr` is the type resulting from using format string (`f"..."`). + +## Methods + +### quoted_contents + +#include_code quoted_contents noir_stdlib/src/meta/format_string.nr rust + +Returns the format string contents (that is, without the leading and trailing double quotes) as a `Quoted` value. \ No newline at end of file diff --git a/noir/noir-repo/docs/docs/noir/standard_library/meta/module.md b/noir/noir-repo/docs/docs/noir/standard_library/meta/module.md index d283f2da8b2..870e366461c 100644 --- a/noir/noir-repo/docs/docs/noir/standard_library/meta/module.md +++ b/noir/noir-repo/docs/docs/noir/standard_library/meta/module.md @@ -20,6 +20,12 @@ Returns the name of the module. Returns each function in the module. +### has_named_attribute + +#include_code has_named_attribute noir_stdlib/src/meta/module.nr rust + +Returns true if this module has a custom attribute with the given name. + ### is_contract #include_code is_contract noir_stdlib/src/meta/module.nr rust diff --git a/noir/noir-repo/docs/docs/noir/standard_library/meta/struct_def.md b/noir/noir-repo/docs/docs/noir/standard_library/meta/struct_def.md index ab3ea4e0698..95e377dffd4 100644 --- a/noir/noir-repo/docs/docs/noir/standard_library/meta/struct_def.md +++ b/noir/noir-repo/docs/docs/noir/standard_library/meta/struct_def.md @@ -43,3 +43,32 @@ comptime fn example(foo: StructDefinition) { #include_code fields noir_stdlib/src/meta/struct_def.nr rust Returns each field of this struct as a pair of (field name, field type). + +### set_fields + +#include_code set_fields noir_stdlib/src/meta/struct_def.nr rust + +Sets the fields of this struct to the given fields list where each element +is a pair of the field's name and the field's type. Expects each field name +to be a single identifier. Note that this will override any previous fields +on this struct. If those should be preserved, use `.fields()` to retrieve the +current fields on the struct type and append the new fields from there. + +Example: + +```rust +// Change this struct to: +// struct Foo { +// a: u32, +// b: i8, +// } +#[mangle_fields] +struct Foo { x: Field } + +comptime fn mangle_fields(s: StructDefinition) { + s.set_fields(&[ + (quote { a }, quote { u32 }.as_type()), + (quote { b }, quote { i8 }.as_type()), + ]); +} +``` diff --git a/noir/noir-repo/noir_stdlib/src/meta/format_string.nr b/noir/noir-repo/noir_stdlib/src/meta/format_string.nr new file mode 100644 index 00000000000..44b69719efe --- /dev/null +++ b/noir/noir-repo/noir_stdlib/src/meta/format_string.nr @@ -0,0 +1,6 @@ +impl fmtstr { + #[builtin(fmtstr_quoted_contents)] + // docs:start:quoted_contents + fn quoted_contents(self) -> Quoted {} + // docs:end:quoted_contents +} diff --git a/noir/noir-repo/noir_stdlib/src/meta/mod.nr b/noir/noir-repo/noir_stdlib/src/meta/mod.nr index 24398054467..9fc399ddbf9 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/mod.nr @@ -1,4 +1,5 @@ mod expr; +mod format_string; mod function_def; mod module; mod op; diff --git a/noir/noir-repo/noir_stdlib/src/meta/module.nr b/noir/noir-repo/noir_stdlib/src/meta/module.nr index 6ea3ca55fb1..b3f76812b8a 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/module.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/module.nr @@ -1,4 +1,9 @@ impl Module { + #[builtin(module_has_named_attribute)] + // docs:start:has_named_attribute + fn has_named_attribute(self, name: Quoted) -> bool {} + // docs:end:has_named_attribute + #[builtin(module_is_contract)] // docs:start:is_contract fn is_contract(self) -> bool {} diff --git a/noir/noir-repo/noir_stdlib/src/meta/struct_def.nr b/noir/noir-repo/noir_stdlib/src/meta/struct_def.nr index 60fdeba21aa..1ca1b6a3925 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/struct_def.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/struct_def.nr @@ -18,4 +18,13 @@ impl StructDefinition { // docs:start:fields fn fields(self) -> [(Quoted, Type)] {} // docs:end:fields + + /// Sets the fields of this struct to the given fields list. + /// All existing fields of the struct will be overridden with the given fields. + /// Each element of the fields list corresponds to the name and type of a field. + /// Each name is expected to be a single identifier. + #[builtin(struct_def_set_fields)] + // docs:start:set_fields + fn set_fields(self, new_fields: [(Quoted, Type)]) {} + // docs:end:set_fields } diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr index 705a1b2ab4e..0e2d459a00f 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr @@ -12,4 +12,18 @@ fn main() { }; assert_eq(s1, "x is 4, fake interpolation: {y}, y is 5"); assert_eq(s2, "\0\0\0\0"); + + // Mainly test fmtstr::quoted_contents + call!(glue(quote { hello }, quote { world })); +} + +fn glue(x: Quoted, y: Quoted) -> Quoted { + f"{x}_{y}".quoted_contents() } + +fn hello_world() {} + +comptime fn call(x: Quoted) -> Quoted { + quote { $x() } +} + diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr index 8d834381fea..5722d42ca26 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -1,10 +1,45 @@ +#[outer_attribute] mod foo { + #![some_attribute] fn x() {} fn y() {} } contract bar {} +#[some_attribute] +mod another_module {} + +#[outer_attribute_func] +mod yet_another_module { + #![super::inner_attribute_func] + fn foo() {} +} + +#[outer_attribute_separate_module] +mod separate_module; + +comptime mut global counter = 0; + +fn increment_counter() { + counter += 1; +} + +fn outer_attribute_func(m: Module) { + assert_eq(m.name(), quote { yet_another_module }); + increment_counter(); +} + +fn inner_attribute_func(m: Module) { + assert_eq(m.name(), quote { yet_another_module }); + increment_counter(); +} + +fn outer_attribute_separate_module(m: Module) { + assert_eq(m.name(), quote { separate_module }); + increment_counter(); +} + fn main() { comptime { @@ -15,6 +50,8 @@ fn main() { let bar = quote { bar }.as_module().unwrap(); assert(bar.is_contract()); + let another_module = quote { another_module }.as_module().unwrap(); + // Check Module::functions assert_eq(foo.functions().len(), 2); assert_eq(bar.functions().len(), 0); @@ -22,7 +59,15 @@ fn main() { // Check Module::name assert_eq(foo.name(), quote { foo }); assert_eq(bar.name(), quote { bar }); + + // Check Module::has_named_attribute + assert(foo.has_named_attribute(quote { some_attribute })); + assert(foo.has_named_attribute(quote { outer_attribute })); + assert(!bar.has_named_attribute(quote { some_attribute })); + assert(another_module.has_named_attribute(quote { some_attribute })); } + + assert_eq(counter, 4); } // docs:start:as_module_example diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/separate_module.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/separate_module.nr new file mode 100644 index 00000000000..53784101507 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/separate_module.nr @@ -0,0 +1,5 @@ +#![inner_attribute_separate_module] +fn inner_attribute_separate_module(m: Module) { + assert_eq(m.name(), quote { separate_module }); + super::increment_counter(); +} diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_type_definition/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_type_definition/src/main.nr index cdfc9bd6b75..aca8d067dde 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_type_definition/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_type_definition/src/main.nr @@ -6,8 +6,21 @@ struct MyType { field2: (B, C), } +#[mutate_struct_fields] +struct I32AndField { + z: i8, +} + comptime fn my_comptime_fn(typ: StructDefinition) { let _ = typ.as_type(); assert_eq(typ.generics().len(), 3); assert_eq(typ.fields().len(), 2); } + +comptime fn mutate_struct_fields(s: StructDefinition) { + let fields = &[ + (quote[x], quote[i32].as_type()), + (quote[y], quote[Field].as_type()) + ]; + s.set_fields(fields); +} diff --git a/noir/noir-repo/test_programs/compile_success_empty/unquote_function/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/unquote_function/Nargo.toml new file mode 100644 index 00000000000..aa56a5798df --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/unquote_function/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "unquote_function" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/unquote_function/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/unquote_function/src/main.nr new file mode 100644 index 00000000000..273a091b26d --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/unquote_function/src/main.nr @@ -0,0 +1,12 @@ +fn main() { + bar(); +} + +#[output_function] +fn foo() {} + +comptime fn output_function(_f: FunctionDefinition) -> Quoted { + quote { + fn bar() {} + } +} diff --git a/noir/noir-repo/test_programs/compile_success_empty/unquote_struct/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/unquote_struct/Nargo.toml new file mode 100644 index 00000000000..c40d6a07093 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/unquote_struct/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "unquote_struct" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/unquote_struct/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/unquote_struct/src/main.nr new file mode 100644 index 00000000000..603440b5c76 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/unquote_struct/src/main.nr @@ -0,0 +1,30 @@ +fn main() { + let foo = Foo { x: 4, y: 4 }; + foo.assert_equal(); +} + +#[output_struct] +fn foo(x: Field, y: u32) -> u32 { + x as u32 + y +} + +// Given a function, wrap its parameters in a struct definition +comptime fn output_struct(f: FunctionDefinition) -> Quoted { + let fields = f.parameters().map( + |param: (Quoted, Type)| { + let name = param.0; + let typ = param.1; + quote { $name: $typ, } + } + ).join(quote {}); + + quote { + struct Foo { $fields } + + impl Foo { + fn assert_equal(self) { + assert_eq(self.x as u32, self.y); + } + } + } +} diff --git a/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Nargo.toml b/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Nargo.toml new file mode 100644 index 00000000000..fa4614c8351 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "empty_composite_array_get" +type = "bin" +authors = [""] +compiler_version = ">=0.32.0" + +[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Prover.toml b/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Prover.toml new file mode 100644 index 00000000000..ff52a8c5bce --- /dev/null +++ b/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Prover.toml @@ -0,0 +1 @@ +empty_input = [] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/src/main.nr b/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/src/main.nr new file mode 100644 index 00000000000..1ed9fe4a5e0 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/src/main.nr @@ -0,0 +1,5 @@ +fn main(empty_input: [(Field, Field); 0]) { + let empty_array: [(Field, Field); 0] = []; + let _ = empty_input[0]; + let _ = empty_array[0]; +} diff --git a/noir/noir-repo/tooling/lsp/src/modules.rs b/noir/noir-repo/tooling/lsp/src/modules.rs index 54074dbd94c..d78da15a8ff 100644 --- a/noir/noir-repo/tooling/lsp/src/modules.rs +++ b/noir/noir-repo/tooling/lsp/src/modules.rs @@ -47,10 +47,11 @@ pub(crate) fn module_full_path( current_module_id: ModuleId, current_module_parent_id: Option, interner: &NodeInterner, + def_maps: &BTreeMap, ) -> Option { let full_path; if let ModuleDefId::ModuleId(module_id) = module_def_id { - if !is_visible(visibility, current_module_id, module_id) { + if !is_visible(module_id, current_module_id, visibility, def_maps) { return None; } @@ -61,7 +62,7 @@ pub(crate) fn module_full_path( return None; }; - if !is_visible(visibility, current_module_id, parent_module) { + if !is_visible(parent_module, current_module_id, visibility, def_maps) { return None; } diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs index d07d117a317..25accc8a008 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -52,6 +52,7 @@ impl<'a> CodeActionFinder<'a> { self.module_id, current_module_parent_id, self.interner, + self.def_maps, ) else { continue; }; diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion.rs b/noir/noir-repo/tooling/lsp/src/requests/completion.rs index 987746d37ce..59758f4b972 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion.rs @@ -22,10 +22,7 @@ use noirc_frontend::{ UnresolvedGenerics, UnresolvedType, UseTree, UseTreeKind, Visitor, }, graph::{CrateId, Dependency}, - hir::{ - def_map::{CrateDefMap, LocalModuleId, ModuleId}, - resolution::import::can_reference_module_id, - }, + hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, hir_def::traits::Trait, macros_api::{ModuleDefId, NodeInterner}, node_interner::ReferenceId, @@ -34,7 +31,7 @@ use noirc_frontend::{ }; use sort_text::underscore_sort_text; -use crate::{requests::to_lsp_location, utils, LspState}; +use crate::{requests::to_lsp_location, utils, visibility::is_visible, LspState}; use super::process_request; @@ -1263,21 +1260,6 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option, -) -> bool { - can_reference_module_id( - def_maps, - current_module_id.krate, - current_module_id.local_id, - target_module_id, - visibility, - ) -} - #[cfg(test)] mod completion_name_matches_tests { use crate::requests::completion::name_matches; diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs index bbd471dfea1..d8823794999 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs @@ -53,6 +53,7 @@ impl<'a> NodeFinder<'a> { self.module_id, current_module_parent_id, self.interner, + self.def_maps, ) else { continue; }; diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs index d621ca21bb8..a7cfa77a73d 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs @@ -336,7 +336,7 @@ mod completion_tests { fo>|< } "#; - assert_completion(src, vec![module_completion_item("foobar")]).await; + assert_completion_excluding_auto_import(src, vec![module_completion_item("foobar")]).await; } #[test] @@ -1863,4 +1863,29 @@ mod completion_tests { Some("(use bar::foobar)".to_string()), ); } + + #[test] + async fn test_auto_import_suggests_private_function_if_visibile() { + let src = r#" + mod foo { + fn qux() { + barba>|< + } + } + + fn barbaz() {} + + fn main() {} + "#; + + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "barbaz()"); + assert_eq!( + item.label_details.as_ref().unwrap().detail, + Some("(use super::barbaz)".to_string()), + ); + } } diff --git a/noir/noir-repo/tooling/lsp/src/visibility.rs b/noir/noir-repo/tooling/lsp/src/visibility.rs index aad8b47fbbe..d6e26f7bc48 100644 --- a/noir/noir-repo/tooling/lsp/src/visibility.rs +++ b/noir/noir-repo/tooling/lsp/src/visibility.rs @@ -1,13 +1,25 @@ -use noirc_frontend::{ast::ItemVisibility, hir::def_map::ModuleId}; +use std::collections::BTreeMap; + +use noirc_frontend::{ + ast::ItemVisibility, + graph::CrateId, + hir::{ + def_map::{CrateDefMap, ModuleId}, + resolution::import::can_reference_module_id, + }, +}; pub(super) fn is_visible( + target_module_id: ModuleId, + current_module_id: ModuleId, visibility: ItemVisibility, - current_module: ModuleId, - target_module: ModuleId, + def_maps: &BTreeMap, ) -> bool { - match visibility { - ItemVisibility::Public => true, - ItemVisibility::Private => false, - ItemVisibility::PublicCrate => current_module.krate == target_module.krate, - } + can_reference_module_id( + def_maps, + current_module_id.krate, + current_module_id.local_id, + target_module_id, + visibility, + ) } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/visitor/item.rs b/noir/noir-repo/tooling/nargo_fmt/src/visitor/item.rs index 0e2d07f13d0..9e556e0fcbe 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/visitor/item.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/visitor/item.rs @@ -165,6 +165,11 @@ impl super::FmtVisitor<'_> { continue; } + for attribute in module.outer_attributes { + self.push_str(&format!("#[{}]\n", attribute.as_ref())); + self.push_str(&self.indent.to_string()); + } + let name = module.name; let after_brace = self.span_after(span, Token::LeftBrace).start(); self.last_position = after_brace; @@ -227,7 +232,8 @@ impl super::FmtVisitor<'_> { | ItemKind::TraitImpl(_) | ItemKind::TypeAlias(_) | ItemKind::Global(_) - | ItemKind::ModuleDecl(_) => { + | ItemKind::ModuleDecl(_) + | ItemKind::InnerAttribute(_) => { self.push_rewrite(self.slice(span).to_string(), span); self.last_position = span.end(); } diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/expected/module.nr b/noir/noir-repo/tooling/nargo_fmt/tests/expected/module.nr index e419543dbc4..0a051a1b50f 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/expected/module.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/expected/module.nr @@ -1,3 +1,6 @@ +#![inner] +#![inner2] + mod a { mod b { struct Data { @@ -13,6 +16,8 @@ mod a { Data2 { a } } + #[custom] + #[another_custom] mod tests { #[test] fn test() { @@ -20,4 +25,11 @@ mod a { data2(1); } } + + #[attr] + mod baz; + + mod empty { + #![inner] + } } diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/input/module.nr b/noir/noir-repo/tooling/nargo_fmt/tests/input/module.nr index e419543dbc4..0a051a1b50f 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/input/module.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/input/module.nr @@ -1,3 +1,6 @@ +#![inner] +#![inner2] + mod a { mod b { struct Data { @@ -13,6 +16,8 @@ mod a { Data2 { a } } + #[custom] + #[another_custom] mod tests { #[test] fn test() { @@ -20,4 +25,11 @@ mod a { data2(1); } } + + #[attr] + mod baz; + + mod empty { + #![inner] + } }