diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 97fc66688e1..fa134f8a0dd 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -237,11 +237,8 @@ pub fn check_crate( deny_warnings: bool, disable_macros: bool, ) -> CompilationResult<()> { - let macros: Vec<&dyn MacroProcessor> = if disable_macros { - vec![] - } else { - vec![&aztec_macros::AztecMacro as &dyn MacroProcessor] - }; + let macros: &[&dyn MacroProcessor] = + if disable_macros { &[] } else { &[&aztec_macros::AztecMacro as &dyn MacroProcessor] }; let mut errors = vec![]; let diagnostics = CrateDefMap::collect_defs(crate_id, context, macros); diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 67ec851d46d..09117bdc3b7 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -24,6 +24,9 @@ use serde::{ #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] pub struct DebugVarId(pub u32); +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] +pub struct DebugFnId(pub u32); + #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] pub struct DebugTypeId(pub u32); @@ -33,7 +36,14 @@ pub struct DebugVariable { pub debug_type_id: DebugTypeId, } +#[derive(Debug, Clone, Hash, Deserialize, Serialize)] +pub struct DebugFunction { + pub name: String, + pub arg_names: Vec, +} + pub type DebugVariables = BTreeMap; +pub type DebugFunctions = BTreeMap; pub type DebugTypes = BTreeMap; #[serde_as] @@ -45,6 +55,7 @@ pub struct DebugInfo { #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, pub variables: DebugVariables, + pub functions: DebugFunctions, pub types: DebugTypes, } @@ -60,9 +71,10 @@ impl DebugInfo { pub fn new( locations: BTreeMap>, variables: DebugVariables, + functions: DebugFunctions, types: DebugTypes, ) -> Self { - Self { locations, variables, types } + Self { locations, variables, functions, types } } /// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified. diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 0bb81efe977..56cb76adbe4 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -88,6 +88,7 @@ pub fn create_circuit( ) -> Result<(Circuit, DebugInfo, Vec, Vec, Vec), RuntimeError> { let debug_variables = program.debug_variables.clone(); let debug_types = program.debug_types.clone(); + let debug_functions = program.debug_functions.clone(); let func_sig = program.main_function_signature.clone(); let recursive = program.recursive; let mut generated_acir = optimize_into_acir( @@ -130,7 +131,7 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations, debug_variables, debug_types); + let mut debug_info = DebugInfo::new(locations, debug_variables, debug_functions, debug_types); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 943a57c1bc0..46f1e7a2765 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -199,22 +199,47 @@ struct Context<'f> { /// found, the top of this conditions stack is popped since we are no longer under that /// condition. If we are under multiple conditions (a nested if), the topmost condition is /// the most recent condition combined with all previous conditions via `And` instructions. - conditions: Vec<(BasicBlockId, ValueId)>, + condition_stack: Vec, /// Maps SSA array values with a slice type to their size. /// This is maintained by appropriate calls to the `SliceCapacityTracker` and is used by the `ValueMerger`. slice_sizes: HashMap, + + /// Stack of block arguments + /// When processing a block, we pop this stack to get its arguments + /// and at the end we push the arguments for his successor + arguments_stack: Vec>, } +#[derive(Clone)] pub(crate) struct Store { old_value: ValueId, new_value: ValueId, } -struct Branch { - condition: ValueId, +#[derive(Clone)] +struct ConditionalBranch { + // Contains the last processed block during the processing of the branch. last_block: BasicBlockId, + // The unresolved condition of the branch + old_condition: ValueId, + // The condition of the branch + condition: ValueId, + // The store values accumulated when processing the branch store_values: HashMap, + // The allocations accumulated when processing the branch + local_allocations: HashSet, +} + +struct ConditionalContext { + // Condition from the conditional statement + condition: ValueId, + // Block containing the conditional statement + entry_block: BasicBlockId, + // First block of the then branch + then_branch: ConditionalBranch, + // First block of the else branch + else_branch: Option, } fn flatten_function_cfg(function: &mut Function) { @@ -233,90 +258,117 @@ fn flatten_function_cfg(function: &mut Function) { store_values: HashMap::default(), local_allocations: HashSet::new(), branch_ends, - conditions: Vec::new(), slice_sizes: HashMap::default(), + condition_stack: Vec::new(), + arguments_stack: Vec::new(), }; context.flatten(); } impl<'f> Context<'f> { fn flatten(&mut self) { - // Start with following the terminator of the entry block since we don't - // need to flatten the entry block into itself. - self.handle_terminator(self.inserter.function.entry_block()); + // Flatten the CFG by inlining all instructions from the queued blocks + // until all blocks have been flattened. + // We follow the terminator of each block to determine which blocks to + // process next + let mut queue = vec![self.inserter.function.entry_block()]; + while let Some(block) = queue.pop() { + self.inline_block(block); + let to_process = self.handle_terminator(block, &queue); + for incoming_block in to_process { + if !queue.contains(&incoming_block) { + queue.push(incoming_block); + } + } + } } - /// Check the terminator of the given block and recursively inline any blocks reachable from - /// it. Since each block from a jmpif terminator is inlined successively, we must handle - /// instructions with side effects like constrain and store specially to preserve correctness. - /// For these instructions we must keep track of what the current condition is and modify - /// the instructions according to the module-level comment at the top of this file. Note that - /// the current condition is all the jmpif conditions required to reach the current block, - /// combined via `And` instructions. - /// - /// Returns the last block to be inlined. This is either the return block of the function or, - /// if self.conditions is not empty, the end block of the most recent condition. - fn handle_terminator(&mut self, block: BasicBlockId) -> BasicBlockId { - // As we recursively flatten inner blocks, we need to track the slice information - // for the outer block before we start recursively inlining - let outer_block_instructions = self.inserter.function.dfg[block].instructions(); - let mut capacity_tracker = SliceCapacityTracker::new(&self.inserter.function.dfg); - for instruction in outer_block_instructions { - let results = self.inserter.function.dfg.instruction_results(*instruction); - let instruction = &self.inserter.function.dfg[*instruction]; + /// Returns the updated condition so that + /// it is 'AND-ed' with the previous condition (if any) + fn link_condition(&mut self, condition: ValueId) -> ValueId { + // Retrieve the previous condition + if let Some(context) = self.condition_stack.last() { + let previous_branch = context.else_branch.as_ref().unwrap_or(&context.then_branch); + let and = Instruction::binary(BinaryOp::And, previous_branch.condition, condition); + self.insert_instruction(and, CallStack::new()) + } else { + condition + } + } + + /// Returns the current condition + fn get_last_condition(&self) -> Option { + self.condition_stack.last().map(|context| match &context.else_branch { + Some(else_branch) => else_branch.condition, + None => context.then_branch.condition, + }) + } + + // Inline all instructions from the given block into the entry block, and track slice capacities + fn inline_block(&mut self, block: BasicBlockId) { + if self.inserter.function.entry_block() == block { + // we do not inline the entry block into itself + // for the outer block before we start inlining + let outer_block_instructions = self.inserter.function.dfg[block].instructions(); + let mut capacity_tracker = SliceCapacityTracker::new(&self.inserter.function.dfg); + for instruction in outer_block_instructions { + let results = self.inserter.function.dfg.instruction_results(*instruction); + let instruction = &self.inserter.function.dfg[*instruction]; + capacity_tracker.collect_slice_information( + instruction, + &mut self.slice_sizes, + results.to_vec(), + ); + } + + return; + } + + let arguments = self.arguments_stack.pop().unwrap(); + self.inserter.remember_block_params(block, &arguments); + + // If this is not a separate variable, clippy gets confused and says the to_vec is + // unnecessary, when removing it actually causes an aliasing/mutability error. + let instructions = self.inserter.function.dfg[block].instructions().to_vec(); + for instruction in instructions.iter() { + let results = self.push_instruction(*instruction); + let (instruction, _) = self.inserter.map_instruction(*instruction); + let mut capacity_tracker = SliceCapacityTracker::new(&self.inserter.function.dfg); capacity_tracker.collect_slice_information( - instruction, + &instruction, &mut self.slice_sizes, - results.to_vec(), + results, ); } + } - match self.inserter.function.dfg[block].unwrap_terminator() { + /// Returns the list of blocks that need to be processed after the given block + /// For a normal block, it would be its successor + /// For blocks related to a conditional statement, we ensure to process + /// the 'then-branch', then the 'else-branch' (if it exists), and finally the end block + fn handle_terminator( + &mut self, + block: BasicBlockId, + work_list: &[BasicBlockId], + ) -> Vec { + let terminator = self.inserter.function.dfg[block].unwrap_terminator().clone(); + match &terminator { TerminatorInstruction::JmpIf { condition, then_destination, else_destination } => { - let old_condition = *condition; - let then_block = *then_destination; - let else_block = *else_destination; - let then_condition = self.inserter.resolve(old_condition); - - let one = FieldElement::one(); - let then_branch = - self.inline_branch(block, then_block, old_condition, then_condition, one); - - let else_condition = - self.insert_instruction(Instruction::Not(then_condition), CallStack::new()); - let zero = FieldElement::zero(); - - // Make sure the else branch sees the previous values of each store - // rather than any values created in the 'then' branch. - self.undo_stores_in_then_branch(&then_branch); - - let else_branch = - self.inline_branch(block, else_block, old_condition, else_condition, zero); - - // We must remember to reset whether side effects are enabled when both branches - // end, in addition to resetting the value of old_condition since it is set to - // known to be true/false within the then/else branch respectively. - self.insert_current_side_effects_enabled(); - - // We must map back to `then_condition` here. Mapping `old_condition` to itself would - // lose any previous mappings. - self.inserter.map_value(old_condition, then_condition); - - // While there is a condition on the stack we don't compile outside the condition - // until it is popped. This ensures we inline the full then and else branches - // before continuing from the end of the conditional here where they can be merged properly. - let end = self.branch_ends[&block]; - self.inline_branch_end(end, then_branch, else_branch) + self.arguments_stack.push(vec![]); + self.if_start(condition, then_destination, else_destination, &block) } TerminatorInstruction::Jmp { destination, arguments, call_stack: _ } => { - if let Some((end_block, _)) = self.conditions.last() { - if destination == end_block { - return block; + let arguments = vecmap(arguments.clone(), |value| self.inserter.resolve(value)); + self.arguments_stack.push(arguments); + if work_list.contains(destination) { + if work_list.last() == Some(destination) { + self.else_stop(&block) + } else { + self.then_stop(&block) } + } else { + vec![*destination] } - let destination = *destination; - let arguments = vecmap(arguments.clone(), |value| self.inserter.resolve(value)); - self.inline_block(destination, &arguments) } TerminatorInstruction::Return { return_values, call_stack } => { let call_stack = call_stack.clone(); @@ -326,133 +378,133 @@ impl<'f> Context<'f> { let entry = self.inserter.function.entry_block(); self.inserter.function.dfg.set_block_terminator(entry, new_return); - block + vec![] } } } - /// Push a condition to the stack of conditions. - /// - /// This condition should be present while we're inlining each block reachable from the 'then' - /// branch of a jmpif instruction, until the branches eventually join back together. Likewise, - /// !condition should be present while we're inlining each block reachable from the 'else' - /// branch of a jmpif instruction until the join block. - fn push_condition(&mut self, start_block: BasicBlockId, condition: ValueId) { - let end_block = self.branch_ends[&start_block]; - - if let Some((_, previous_condition)) = self.conditions.last() { - let and = Instruction::binary(BinaryOp::And, *previous_condition, condition); - let new_condition = self.insert_instruction(and, CallStack::new()); - self.conditions.push((end_block, new_condition)); - } else { - self.conditions.push((end_block, condition)); + /// Process a conditional statement + fn if_start( + &mut self, + condition: &ValueId, + then_destination: &BasicBlockId, + else_destination: &BasicBlockId, + if_entry: &BasicBlockId, + ) -> Vec { + // manage conditions + let old_condition = *condition; + let then_condition = self.inserter.resolve(old_condition); + + let one = FieldElement::one(); + let old_stores = std::mem::take(&mut self.store_values); + let old_allocations = std::mem::take(&mut self.local_allocations); + let branch = ConditionalBranch { + old_condition, + condition: self.link_condition(then_condition), + store_values: old_stores, + local_allocations: old_allocations, + last_block: *then_destination, + }; + let cond_context = ConditionalContext { + condition: then_condition, + entry_block: *if_entry, + then_branch: branch, + else_branch: None, + }; + self.condition_stack.push(cond_context); + self.insert_current_side_effects_enabled(); + // Optimization: within the then branch we know the condition to be true, so replace + // any references of it within this branch with true. Likewise, do the same with false + // with the else branch. We must be careful not to replace the condition if it is a + // known constant, otherwise we can end up setting 1 = 0 or vice-versa. + if self.inserter.function.dfg.get_numeric_constant(old_condition).is_none() { + let known_value = self.inserter.function.dfg.make_constant(one, Type::bool()); + + self.inserter.map_value(old_condition, known_value); } + vec![self.branch_ends[if_entry], *else_destination, *then_destination] } - /// Insert a new instruction into the function's entry block. - /// Unlike push_instruction, this function will not map any ValueIds. - /// within the given instruction, nor will it modify self.values in any way. - fn insert_instruction(&mut self, instruction: Instruction, call_stack: CallStack) -> ValueId { - let block = self.inserter.function.entry_block(); - self.inserter - .function - .dfg - .insert_instruction_and_results(instruction, block, None, call_stack) - .first() + /// Switch context to the 'else-branch' + fn then_stop(&mut self, block: &BasicBlockId) -> Vec { + let mut cond_context = self.condition_stack.pop().unwrap(); + cond_context.then_branch.last_block = *block; + + let else_condition = + self.insert_instruction(Instruction::Not(cond_context.condition), CallStack::new()); + let else_condition = self.link_condition(else_condition); + + let zero = FieldElement::zero(); + // Make sure the else branch sees the previous values of each store + // rather than any values created in the 'then' branch. + let old_stores = std::mem::take(&mut cond_context.then_branch.store_values); + cond_context.then_branch.store_values = std::mem::take(&mut self.store_values); + self.undo_stores_in_then_branch(&cond_context.then_branch.store_values); + + let old_allocations = std::mem::take(&mut self.local_allocations); + let else_branch = ConditionalBranch { + old_condition: cond_context.then_branch.old_condition, + condition: else_condition, + store_values: old_stores, + local_allocations: old_allocations, + last_block: *block, + }; + let old_condition = else_branch.old_condition; + cond_context.then_branch.local_allocations.clear(); + cond_context.else_branch = Some(else_branch); + self.condition_stack.push(cond_context); + + self.insert_current_side_effects_enabled(); + // Optimization: within the then branch we know the condition to be true, so replace + // any references of it within this branch with true. Likewise, do the same with false + // with the else branch. We must be careful not to replace the condition if it is a + // known constant, otherwise we can end up setting 1 = 0 or vice-versa. + if self.inserter.function.dfg.get_numeric_constant(old_condition).is_none() { + let known_value = self.inserter.function.dfg.make_constant(zero, Type::bool()); + + self.inserter.map_value(old_condition, known_value); + } + assert_eq!(self.cfg.successors(*block).len(), 1); + vec![self.cfg.successors(*block).next().unwrap()] } - /// Inserts a new instruction into the function's entry block, using the given - /// control type variables to specify result types if needed. - /// Unlike push_instruction, this function will not map any ValueIds. - /// within the given instruction, nor will it modify self.values in any way. - fn insert_instruction_with_typevars( - &mut self, - instruction: Instruction, - ctrl_typevars: Option>, - ) -> InsertInstructionResult { - let block = self.inserter.function.entry_block(); - self.inserter.function.dfg.insert_instruction_and_results( - instruction, - block, - ctrl_typevars, - CallStack::new(), - ) - } + /// Process the 'exit' block of a conditional statement + fn else_stop(&mut self, block: &BasicBlockId) -> Vec { + let mut cond_context = self.condition_stack.pop().unwrap(); + if cond_context.else_branch.is_none() { + // then_stop() has not been called, this means that the conditional statement has no else branch + // so we simply do the then_stop() now + self.condition_stack.push(cond_context); + self.then_stop(block); + cond_context = self.condition_stack.pop().unwrap(); + } - /// Checks the branch condition on the top of the stack and uses it to build and insert an - /// `EnableSideEffects` instruction into the entry block. - /// - /// If the stack is empty, a "true" u1 constant is taken to be the active condition. This is - /// necessary for re-enabling side-effects when re-emerging to a branch depth of 0. - fn insert_current_side_effects_enabled(&mut self) { - let condition = match self.conditions.last() { - Some((_, cond)) => *cond, - None => { - self.inserter.function.dfg.make_constant(FieldElement::one(), Type::unsigned(1)) - } - }; - let enable_side_effects = Instruction::EnableSideEffects { condition }; - self.insert_instruction_with_typevars(enable_side_effects, None); - } + let mut else_branch = cond_context.else_branch.unwrap(); + let stores_in_branch = std::mem::replace(&mut self.store_values, else_branch.store_values); + self.local_allocations = std::mem::take(&mut else_branch.local_allocations); + else_branch.last_block = *block; + else_branch.store_values = stores_in_branch; + cond_context.else_branch = Some(else_branch); - /// Inline one branch of a jmpif instruction. - /// - /// This will continue inlining recursively until the next end block is reached where each branch - /// of the jmpif instruction is joined back into a single block. - /// - /// Within a branch of a jmpif instruction, we can assume the condition of the jmpif to be - /// always true or false, depending on which branch we're in. - /// - /// Returns the ending block / join block of this branch. - fn inline_branch( - &mut self, - jmpif_block: BasicBlockId, - destination: BasicBlockId, - old_condition: ValueId, - new_condition: ValueId, - condition_value: FieldElement, - ) -> Branch { - if destination == self.branch_ends[&jmpif_block] { - // If the branch destination is the same as the end of the branch, this must be the - // 'else' case of an if with no else - so there is no else branch. - Branch { - condition: new_condition, - // The last block here is somewhat arbitrary. It only matters that it has no Jmp - // args that will be merged by inline_branch_end. Since jmpifs don't have - // block arguments, it is safe to use the jmpif block here. - last_block: jmpif_block, - store_values: HashMap::default(), - } - } else { - self.push_condition(jmpif_block, new_condition); - self.insert_current_side_effects_enabled(); - let old_stores = std::mem::take(&mut self.store_values); - let old_allocations = std::mem::take(&mut self.local_allocations); - - // Optimization: within the then branch we know the condition to be true, so replace - // any references of it within this branch with true. Likewise, do the same with false - // with the else branch. We must be careful not to replace the condition if it is a - // known constant, otherwise we can end up setting 1 = 0 or vice-versa. - if self.inserter.function.dfg.get_numeric_constant(old_condition).is_none() { - let known_value = - self.inserter.function.dfg.make_constant(condition_value, Type::bool()); - - self.inserter.map_value(old_condition, known_value); - } + // We must remember to reset whether side effects are enabled when both branches + // end, in addition to resetting the value of old_condition since it is set to + // known to be true/false within the then/else branch respectively. + self.insert_current_side_effects_enabled(); - let final_block = self.inline_block(destination, &[]); + // We must map back to `then_condition` here. Mapping `old_condition` to itself would + // lose any previous mappings. + self.inserter + .map_value(cond_context.then_branch.old_condition, cond_context.then_branch.condition); - self.conditions.pop(); + // While there is a condition on the stack we don't compile outside the condition + // until it is popped. This ensures we inline the full then and else branches + // before continuing from the end of the conditional here where they can be merged properly. + let end = self.branch_ends[&cond_context.entry_block]; - let stores_in_branch = std::mem::replace(&mut self.store_values, old_stores); - self.local_allocations = old_allocations; + // Merge arguments and stores from the else/end branches + self.inline_branch_end(end, cond_context); - Branch { - condition: new_condition, - last_block: final_block, - store_values: stores_in_branch, - } - } + vec![self.cfg.successors(*block).next().unwrap()] } /// Inline the ending block of a branch, the point where all blocks from a jmpif instruction @@ -467,15 +519,17 @@ impl<'f> Context<'f> { fn inline_branch_end( &mut self, destination: BasicBlockId, - then_branch: Branch, - else_branch: Branch, + cond_context: ConditionalContext, ) -> BasicBlockId { assert_eq!(self.cfg.predecessors(destination).len(), 2); + let last_then = cond_context.then_branch.last_block; + let mut else_args = Vec::new(); + if cond_context.else_branch.is_some() { + let last_else = cond_context.else_branch.clone().unwrap().last_block; + else_args = self.inserter.function.dfg[last_else].terminator_arguments().to_vec(); + } - let then_args = - self.inserter.function.dfg[then_branch.last_block].terminator_arguments().to_vec(); - let else_args = - self.inserter.function.dfg[else_branch.last_block].terminator_arguments().to_vec(); + let then_args = self.inserter.function.dfg[last_then].terminator_arguments().to_vec(); let params = self.inserter.function.dfg.block_parameters(destination); assert_eq!(params.len(), then_args.len()); @@ -500,17 +554,64 @@ impl<'f> Context<'f> { // Cannot include this in the previous vecmap since it requires exclusive access to self let args = vecmap(args, |(then_arg, else_arg)| { value_merger.merge_values( - then_branch.condition, - else_branch.condition, + cond_context.then_branch.condition, + cond_context.else_branch.clone().unwrap().condition, then_arg, else_arg, ) }); - self.merge_stores(then_branch, else_branch); + self.merge_stores(cond_context.then_branch, cond_context.else_branch); + self.arguments_stack.pop(); + self.arguments_stack.pop(); + self.arguments_stack.push(args); + destination + } - // insert merge instruction - self.inline_block(destination, &args) + /// Insert a new instruction into the function's entry block. + /// Unlike push_instruction, this function will not map any ValueIds. + /// within the given instruction, nor will it modify self.values in any way. + fn insert_instruction(&mut self, instruction: Instruction, call_stack: CallStack) -> ValueId { + let block = self.inserter.function.entry_block(); + self.inserter + .function + .dfg + .insert_instruction_and_results(instruction, block, None, call_stack) + .first() + } + + /// Inserts a new instruction into the function's entry block, using the given + /// control type variables to specify result types if needed. + /// Unlike push_instruction, this function will not map any ValueIds. + /// within the given instruction, nor will it modify self.values in any way. + fn insert_instruction_with_typevars( + &mut self, + instruction: Instruction, + ctrl_typevars: Option>, + ) -> InsertInstructionResult { + let block = self.inserter.function.entry_block(); + self.inserter.function.dfg.insert_instruction_and_results( + instruction, + block, + ctrl_typevars, + CallStack::new(), + ) + } + + /// Checks the branch condition on the top of the stack and uses it to build and insert an + /// `EnableSideEffects` instruction into the entry block. + /// + /// If the stack is empty, a "true" u1 constant is taken to be the active condition. This is + /// necessary for re-enabling side-effects when re-emerging to a branch depth of 0. + fn insert_current_side_effects_enabled(&mut self) { + let condition = match self.get_last_condition() { + Some(cond) => cond, + None => { + self.inserter.function.dfg.make_constant(FieldElement::one(), Type::unsigned(1)) + } + }; + let enable_side_effects = Instruction::EnableSideEffects { condition }; + self.insert_instruction_with_typevars(enable_side_effects, None); } /// Merge any store instructions found in each branch. @@ -518,7 +619,11 @@ impl<'f> Context<'f> { /// This function relies on the 'then' branch being merged before the 'else' branch of a jmpif /// instruction. If this ordering is changed, the ordering that store values are merged within /// this function also needs to be changed to reflect that. - fn merge_stores(&mut self, then_branch: Branch, else_branch: Branch) { + fn merge_stores( + &mut self, + then_branch: ConditionalBranch, + else_branch: Option, + ) { // Address -> (then_value, else_value, value_before_the_if) let mut new_map = BTreeMap::new(); @@ -526,11 +631,13 @@ impl<'f> Context<'f> { new_map.insert(address, (store.new_value, store.old_value, store.old_value)); } - for (address, store) in else_branch.store_values { - if let Some(entry) = new_map.get_mut(&address) { - entry.1 = store.new_value; - } else { - new_map.insert(address, (store.old_value, store.new_value, store.old_value)); + if else_branch.is_some() { + for (address, store) in else_branch.clone().unwrap().store_values { + if let Some(entry) = new_map.get_mut(&address) { + entry.1 = store.new_value; + } else { + new_map.insert(address, (store.old_value, store.new_value, store.old_value)); + } } } @@ -544,8 +651,11 @@ impl<'f> Context<'f> { } let then_condition = then_branch.condition; - let else_condition = else_branch.condition; - + let else_condition = if let Some(branch) = else_branch { + branch.condition + } else { + self.inserter.function.dfg.make_constant(FieldElement::zero(), Type::bool()) + }; let block = self.inserter.function.entry_block(); let mut value_merger = @@ -607,35 +717,6 @@ impl<'f> Context<'f> { } } - /// Inline all instructions from the given destination block into the entry block. - /// Afterwards, check the block's terminator and continue inlining recursively. - /// - /// Returns the final block that was inlined. - /// - /// Expects that the `arguments` given are already translated via self.inserter.resolve. - /// If they are not, it is possible some values which no longer exist, such as block - /// parameters, will be kept in the program. - fn inline_block(&mut self, destination: BasicBlockId, arguments: &[ValueId]) -> BasicBlockId { - self.inserter.remember_block_params(destination, arguments); - - // If this is not a separate variable, clippy gets confused and says the to_vec is - // unnecessary, when removing it actually causes an aliasing/mutability error. - let instructions = self.inserter.function.dfg[destination].instructions().to_vec(); - - for instruction in instructions.iter() { - let results = self.push_instruction(*instruction); - let (instruction, _) = self.inserter.map_instruction(*instruction); - let mut capacity_tracker = SliceCapacityTracker::new(&self.inserter.function.dfg); - capacity_tracker.collect_slice_information( - &instruction, - &mut self.slice_sizes, - results, - ); - } - - self.handle_terminator(destination) - } - /// Push the given instruction to the end of the entry block of the current function. /// /// Note that each ValueId of the instruction will be mapped via self.inserter.resolve. @@ -666,7 +747,7 @@ impl<'f> Context<'f> { instruction: Instruction, call_stack: CallStack, ) -> Instruction { - if let Some((_, condition)) = self.conditions.last().copied() { + if let Some(condition) = self.get_last_condition() { match instruction { Instruction::Constrain(lhs, rhs, message) => { // Replace constraint `lhs == rhs` with `condition * lhs == condition * rhs`. @@ -741,8 +822,8 @@ impl<'f> Context<'f> { } } - fn undo_stores_in_then_branch(&mut self, then_branch: &Branch) { - for (address, store) in &then_branch.store_values { + fn undo_stores_in_then_branch(&mut self, store_values: &HashMap) { + for (address, store) in store_values { let address = *address; let value = store.old_value; self.insert_instruction_with_typevars(Instruction::Store { address, value }, None); diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index f39b71405d3..387840b57c4 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -240,6 +240,17 @@ pub trait Recoverable { fn error(span: Span) -> Self; } +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct ModuleDeclaration { + pub ident: Ident, +} + +impl std::fmt::Display for ModuleDeclaration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "mod {}", self.ident) + } +} + #[derive(Debug, PartialEq, Eq, Clone)] pub struct ImportStatement { pub path: Path, diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index a88567fcaf9..05916502d73 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -4,9 +4,11 @@ use crate::{ ast::{Path, PathKind}, parser::{Item, ItemKind}, }; +use noirc_errors::debug_info::{DebugFnId, DebugFunction}; use noirc_errors::{Span, Spanned}; use std::collections::HashMap; use std::collections::VecDeque; +use std::mem::take; const MAX_MEMBER_ASSIGN_DEPTH: usize = 8; @@ -26,8 +28,12 @@ pub struct DebugInstrumenter { // all field names referenced when assigning to a member of a variable pub field_names: HashMap, + // all collected function metadata (name + argument names) + pub functions: HashMap, + next_var_id: u32, next_field_name_id: u32, + next_fn_id: u32, // last seen variable names and their IDs grouped by scope scope: Vec>, @@ -38,9 +44,11 @@ impl Default for DebugInstrumenter { Self { variables: HashMap::default(), field_names: HashMap::default(), + functions: HashMap::default(), scope: vec![], next_var_id: 0, next_field_name_id: 1, + next_fn_id: 0, } } } @@ -76,10 +84,22 @@ impl DebugInstrumenter { field_name_id } + fn insert_function(&mut self, fn_name: String, arguments: Vec) -> DebugFnId { + let fn_id = DebugFnId(self.next_fn_id); + self.next_fn_id += 1; + self.functions.insert(fn_id, DebugFunction { name: fn_name, arg_names: arguments }); + fn_id + } + fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) { + let func_name = func.name.0.contents.clone(); + let func_args = + func.parameters.iter().map(|param| pattern_to_string(¶m.pattern)).collect(); + let fn_id = self.insert_function(func_name, func_args); + let enter_stmt = build_debug_call_stmt("enter", fn_id, func.span); self.scope.push(HashMap::default()); - let set_fn_params = func + let set_fn_params: Vec<_> = func .parameters .iter() .flat_map(|param| { @@ -93,10 +113,21 @@ impl DebugInstrumenter { }) .collect(); - self.walk_scope(&mut func.body.0, func.span); + let func_body = &mut func.body.0; + let mut statements = take(func_body); + + self.walk_scope(&mut statements, func.span); - // prepend fn params: - func.body.0 = [set_fn_params, func.body.0.clone()].concat(); + // walk_scope ensures that the last statement is the return value of the function + let last_stmt = statements.pop().expect("at least one statement after walk_scope"); + let exit_stmt = build_debug_call_stmt("exit", fn_id, last_stmt.span); + + // rebuild function body + func_body.push(enter_stmt); + func_body.extend(set_fn_params); + func_body.extend(statements); + func_body.push(exit_stmt); + func_body.push(last_stmt); } // Modify a vector of statements in-place, adding instrumentation for sets and drops. @@ -427,6 +458,8 @@ impl DebugInstrumenter { use dep::__debug::{{ __debug_var_assign, __debug_var_drop, + __debug_fn_enter, + __debug_fn_exit, __debug_dereference_assign, {member_assigns}, }};"# @@ -451,14 +484,32 @@ pub fn build_debug_crate_file() -> String { } #[oracle(__debug_var_drop)] - unconstrained fn __debug_var_drop_oracle(_var_id: u32) {} - unconstrained fn __debug_var_drop_inner(var_id: u32) { + unconstrained fn __debug_var_drop_oracle(_var_id: u32) {} + unconstrained fn __debug_var_drop_inner(var_id: u32) { __debug_var_drop_oracle(var_id); } - pub fn __debug_var_drop(var_id: u32) { + pub fn __debug_var_drop(var_id: u32) { __debug_var_drop_inner(var_id); } + #[oracle(__debug_fn_enter)] + unconstrained fn __debug_fn_enter_oracle(_fn_id: u32) {} + unconstrained fn __debug_fn_enter_inner(fn_id: u32) { + __debug_fn_enter_oracle(fn_id); + } + pub fn __debug_fn_enter(fn_id: u32) { + __debug_fn_enter_inner(fn_id); + } + + #[oracle(__debug_fn_exit)] + unconstrained fn __debug_fn_exit_oracle(_fn_id: u32) {} + unconstrained fn __debug_fn_exit_inner(fn_id: u32) { + __debug_fn_exit_oracle(fn_id); + } + pub fn __debug_fn_exit(fn_id: u32) { + __debug_fn_exit_inner(fn_id); + } + #[oracle(__debug_dereference_assign)] unconstrained fn __debug_dereference_assign_oracle(_var_id: u32, _value: T) {} unconstrained fn __debug_dereference_assign_inner(var_id: u32, value: T) { @@ -561,6 +612,21 @@ fn build_assign_member_stmt( ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } } +fn build_debug_call_stmt(fname: &str, fn_id: DebugFnId, span: Span) -> ast::Statement { + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident(&format!["__debug_fn_{fname}"], span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments: vec![uint_expr(fn_id.0 as u128, span)], + })); + ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } +} + fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { let mut vars = vec![]; let mut stack = VecDeque::from([(pattern, false)]); @@ -585,6 +651,30 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { vars } +fn pattern_to_string(pattern: &ast::Pattern) -> String { + match pattern { + ast::Pattern::Identifier(id) => id.0.contents.clone(), + ast::Pattern::Mutable(mpat, _, _) => format!("mut {}", pattern_to_string(mpat.as_ref())), + ast::Pattern::Tuple(elements, _) => format!( + "({})", + elements.iter().map(pattern_to_string).collect::>().join(", ") + ), + ast::Pattern::Struct(name, fields, _) => { + format!( + "{} {{ {} }}", + name, + fields + .iter() + .map(|(field_ident, field_pattern)| { + format!("{}: {}", &field_ident.0.contents, pattern_to_string(field_pattern)) + }) + .collect::>() + .join(", "), + ) + } + } +} + fn ident(s: &str, span: Span) -> ast::Ident { ast::Ident(Spanned::from(span, s.to_string())) } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 7f36af5b30e..27b1d376f11 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -207,7 +207,7 @@ impl DefCollector { context: &mut Context, ast: SortedModule, root_file_id: FileId, - macro_processors: Vec<&dyn MacroProcessor>, + macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; let crate_id = def_map.krate; @@ -220,11 +220,7 @@ impl DefCollector { let crate_graph = &context.crate_graph[crate_id]; for dep in crate_graph.dependencies.clone() { - errors.extend(CrateDefMap::collect_defs( - dep.crate_id, - context, - macro_processors.clone(), - )); + errors.extend(CrateDefMap::collect_defs(dep.crate_id, context, macro_processors)); let dep_def_root = context.def_map(&dep.crate_id).expect("ice: def map was just created").root; @@ -257,7 +253,7 @@ impl DefCollector { context.def_maps.insert(crate_id, def_collector.def_map); // TODO(#4653): generalize this function - for macro_processor in ¯o_processors { + for macro_processor in macro_processors { macro_processor .process_unresolved_traits_impls( &crate_id, @@ -282,7 +278,7 @@ impl DefCollector { // Resolve unresolved imports collected from the crate, one by one. for collected_import in def_collector.collected_imports { - match resolve_import(crate_id, collected_import, &context.def_maps) { + match resolve_import(crate_id, &collected_import, &context.def_maps) { Ok(resolved_import) => { // Populate module namespaces according to the imports used let current_def_map = context.def_maps.get_mut(&crate_id).unwrap(); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index fc777ac877b..188b262498d 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -9,8 +9,8 @@ use crate::{ hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, node_interner::{FunctionModifiers, TraitId, TypeAliasId}, parser::{SortedModule, SortedSubModule}, - FunctionDefinition, Ident, ItemVisibility, LetStatement, NoirFunction, NoirStruct, NoirTrait, - NoirTraitImpl, NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl, + FunctionDefinition, Ident, ItemVisibility, LetStatement, ModuleDeclaration, NoirFunction, + NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl, }; use super::{ @@ -523,15 +523,15 @@ impl<'a> ModCollector<'a> { fn parse_module_declaration( &mut self, context: &mut Context, - mod_name: &Ident, + mod_decl: &ModuleDeclaration, crate_id: CrateId, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; let child_file_id = - match find_module(&context.file_manager, self.file_id, &mod_name.0.contents) { + match find_module(&context.file_manager, self.file_id, &mod_decl.ident.0.contents) { Ok(child_file_id) => child_file_id, Err(expected_path) => { - let mod_name = mod_name.clone(); + let mod_name = mod_decl.ident.clone(); let err = DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path }; errors.push((err.into(), self.file_id)); @@ -539,17 +539,17 @@ impl<'a> ModCollector<'a> { } }; - let location = Location { file: self.file_id, span: mod_name.span() }; + let location = Location { file: self.file_id, span: mod_decl.ident.span() }; if let Some(old_location) = context.visited_files.get(&child_file_id) { let error = DefCollectorErrorKind::ModuleAlreadyPartOfCrate { - mod_name: mod_name.clone(), + mod_name: mod_decl.ident.clone(), span: location.span, }; errors.push((error.into(), location.file)); let error = DefCollectorErrorKind::ModuleOriginallyDefined { - mod_name: mod_name.clone(), + mod_name: mod_decl.ident.clone(), span: old_location.span, }; errors.push((error.into(), old_location.file)); @@ -567,7 +567,7 @@ impl<'a> ModCollector<'a> { ); // Add module into def collector and get a ModuleId - match self.push_child_module(mod_name, child_file_id, true, false) { + match self.push_child_module(&mod_decl.ident, child_file_id, true, false) { Ok(child_mod_id) => { errors.extend(collect_defs( self.def_collector, diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 8721bdb6c3c..1326ffca9f7 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -73,7 +73,7 @@ impl CrateDefMap { pub fn collect_defs( crate_id: CrateId, context: &mut Context, - macro_processors: Vec<&dyn MacroProcessor>, + macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { // Check if this Crate has already been compiled // XXX: There is probably a better alternative for this. @@ -90,7 +90,7 @@ impl CrateDefMap { let (ast, parsing_errors) = context.parsed_file_results(root_file_id); let mut ast = ast.into_sorted(); - for macro_processor in ¯o_processors { + for macro_processor in macro_processors { match macro_processor.process_untyped_ast(ast.clone(), &crate_id, context) { Ok(processed_ast) => { ast = processed_ast; @@ -115,13 +115,7 @@ impl CrateDefMap { }; // Now we want to populate the CrateDefMap using the DefCollector - errors.extend(DefCollector::collect( - def_map, - context, - ast, - root_file_id, - macro_processors.clone(), - )); + errors.extend(DefCollector::collect(def_map, context, ast, root_file_id, macro_processors)); errors.extend( parsing_errors.iter().map(|e| (e.clone().into(), root_file_id)).collect::>(), diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index d2fe67da38c..1049599f079 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -80,8 +80,6 @@ pub enum ResolverError { PrivateFunctionCalled { name: String, span: Span }, #[error("{name} is not visible from the current crate")] NonCrateFunctionCalled { name: String, span: Span }, - #[error("Only sized types may be used in the entry point to a program")] - InvalidTypeForEntryPoint { span: Span }, #[error("Nested slices are not supported")] NestedSlices { span: Span }, #[error("#[recursive] attribute is only allowed on entry points to a program")] @@ -309,9 +307,6 @@ impl From for Diagnostic { ResolverError::NonCrateFunctionCalled { span, name } => Diagnostic::simple_warning( format!("{name} is not visible from the current crate"), format!("{name} is only visible within its crate"), span), - ResolverError::InvalidTypeForEntryPoint { span } => Diagnostic::simple_error( - "Only sized types may be used in the entry point to a program".to_string(), - "Slices, references, or any type containing them may not be used in main or a contract function".to_string(), span), ResolverError::NestedSlices { span } => Diagnostic::simple_error( "Nested slices are not supported".into(), "Try to use a constant sized array instead".into(), diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index e6ac33053a0..9c8418daf80 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -52,7 +52,7 @@ impl From for CustomDiagnostic { pub fn resolve_import( crate_id: CrateId, - import_directive: ImportDirective, + import_directive: &ImportDirective, def_maps: &BTreeMap, ) -> Result { let def_map = &def_maps[&crate_id]; @@ -62,10 +62,10 @@ pub fn resolve_import( let module_scope = import_directive.module_id; let resolved_namespace = - resolve_path_to_ns(&import_directive, def_map, def_maps, allow_contracts) + resolve_path_to_ns(import_directive, def_map, def_maps, allow_contracts) .map_err(|error| (error, module_scope))?; - let name = resolve_path_name(&import_directive); + let name = resolve_path_name(import_directive); Ok(ResolvedImport { name, resolved_namespace, diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 48cd44c9d46..069463cfb12 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -911,10 +911,6 @@ impl<'a> Resolver<'a> { }); } - if self.is_entry_point_function(func) { - self.verify_type_valid_for_program_input(&typ); - } - let pattern = self.resolve_pattern(pattern, DefinitionKind::Local(None)); let typ = self.resolve_type_inner(typ, &mut generics); @@ -991,6 +987,7 @@ impl<'a> Resolver<'a> { return_distinctness: func.def.return_distinctness, has_body: !func.def.body.is_empty(), trait_constraints: self.resolve_trait_constraints(&func.def.where_clause), + is_entry_point: self.is_entry_point_function(func), } } @@ -2001,89 +1998,6 @@ impl<'a> Resolver<'a> { } HirLiteral::FmtStr(str, fmt_str_idents) } - - /// Only sized types are valid to be used as main's parameters or the parameters to a contract - /// function. If the given type is not sized (e.g. contains a slice or NamedGeneric type), an - /// error is issued. - fn verify_type_valid_for_program_input(&mut self, typ: &UnresolvedType) { - match &typ.typ { - UnresolvedTypeData::FieldElement - | UnresolvedTypeData::Integer(_, _) - | UnresolvedTypeData::Bool - | UnresolvedTypeData::Unit - | UnresolvedTypeData::Error => (), - - UnresolvedTypeData::MutableReference(_) - | UnresolvedTypeData::Function(_, _, _) - | UnresolvedTypeData::FormatString(_, _) - | UnresolvedTypeData::TraitAsType(..) - | UnresolvedTypeData::Unspecified => { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - - UnresolvedTypeData::Array(length, element) => { - if let Some(length) = length { - self.verify_type_expression_valid_for_program_input(length); - } else { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - self.verify_type_valid_for_program_input(element); - } - UnresolvedTypeData::Expression(expression) => { - self.verify_type_expression_valid_for_program_input(expression); - } - UnresolvedTypeData::String(length) => { - if let Some(length) = length { - self.verify_type_expression_valid_for_program_input(length); - } else { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - } - UnresolvedTypeData::Named(path, generics, _) => { - // Since the type is named, we need to resolve it to see what it actually refers to - // in order to check whether it is valid. Since resolving it may lead to a - // resolution error, we have to truncate our error count to the previous count just - // in case. This is to ensure resolution errors are not issued twice when this type - // is later resolved properly. - let error_count = self.errors.len(); - let resolved = self.resolve_named_type(path.clone(), generics.clone(), &mut vec![]); - self.errors.truncate(error_count); - - if !resolved.is_valid_for_program_input() { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - } - UnresolvedTypeData::Tuple(elements) => { - for element in elements { - self.verify_type_valid_for_program_input(element); - } - } - UnresolvedTypeData::Parenthesized(typ) => self.verify_type_valid_for_program_input(typ), - } - } - - fn verify_type_expression_valid_for_program_input(&mut self, expr: &UnresolvedTypeExpression) { - match expr { - UnresolvedTypeExpression::Constant(_, _) => (), - UnresolvedTypeExpression::Variable(path) => { - let error_count = self.errors.len(); - let resolved = self.resolve_named_type(path.clone(), vec![], &mut vec![]); - self.errors.truncate(error_count); - - if !resolved.is_valid_for_program_input() { - self.push_err(ResolverError::InvalidTypeForEntryPoint { span: path.span() }); - } - } - UnresolvedTypeExpression::BinaryOperation(lhs, _, rhs, _) => { - self.verify_type_expression_valid_for_program_input(lhs); - self.verify_type_expression_valid_for_program_input(rhs); - } - } - } } /// Gives an error if a user tries to create a mutable reference diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 96d30100d8b..cba2400441f 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -122,6 +122,8 @@ pub enum TypeCheckError { ConstrainedReferenceToUnconstrained { span: Span }, #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] UnconstrainedSliceReturnToConstrained { span: Span }, + #[error("Only sized types may be used in the entry point to a program")] + InvalidTypeForEntryPoint { span: Span }, } impl TypeCheckError { @@ -284,6 +286,9 @@ impl From for Diagnostic { let msg = format!("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope"); Diagnostic::simple_warning(msg, "Unnecessary trait constraint in where clause".into(), span) } + TypeCheckError::InvalidTypeForEntryPoint { span } => Diagnostic::simple_error( + "Only sized types may be used in the entry point to a program".to_string(), + "Slices, references, or any type containing them may not be used in main or a contract function".to_string(), span), } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 7b854e58fca..c5287d35caf 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -863,7 +863,13 @@ impl<'interner> TypeChecker<'interner> { span: op.location.span, }); - self.comparator_operand_type_rules(x_type, y_type, op, span) + let (_, use_impl) = self.comparator_operand_type_rules(x_type, y_type, op, span)?; + + // If the size is not constant, we must fall back to a user-provided impl for + // equality on slices. + let size = x_size.follow_bindings(); + let use_impl = use_impl || size.evaluate_to_u64().is_none(); + Ok((Bool, use_impl)) } (String(x_size), String(y_size)) => { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 21d1c75a0f2..aab793ec867 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -14,7 +14,7 @@ mod stmt; pub use errors::TypeCheckError; use crate::{ - hir_def::{expr::HirExpression, stmt::HirStatement, traits::TraitConstraint}, + hir_def::{expr::HirExpression, function::Param, stmt::HirStatement, traits::TraitConstraint}, node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, Type, }; @@ -74,6 +74,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec, + func_id: FuncId, + param: &Param, + errors: &mut Vec, +) { + let meta = type_checker.interner.function_meta(&func_id); + if meta.is_entry_point && !param.1.is_valid_for_program_input() { + let span = param.0.span(); + errors.push(TypeCheckError::InvalidTypeForEntryPoint { span }); + } +} + fn function_info(interner: &NodeInterner, function_body_id: &ExprId) -> (noirc_errors::Span, bool) { let (expr_span, empty_function) = if let HirExpression::Block(block) = interner.expression(function_body_id) { @@ -329,6 +346,7 @@ mod test { trait_impl: None, return_type: FunctionReturnType::Default(Span::default()), trait_constraints: Vec::new(), + is_entry_point: true, }; interner.push_fn_meta(func_meta, func_id); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index d3ab2a9393b..82bbe1aa5b6 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -112,6 +112,10 @@ pub struct FuncMeta { /// The trait impl this function belongs to, if any pub trait_impl: Option, + + /// True if this function is an entry point to the program. + /// For non-contracts, this means the function is `main`. + pub is_entry_point: bool, } impl FuncMeta { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index b70aa43701c..5ab036eef5b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -703,10 +703,10 @@ impl Type { | Type::TraitAsType(..) | Type::NotConstant => false, - // This function is called during name resolution before we've verified aliases - // are not cyclic. As a result, it wouldn't be safe to check this alias' definition - // to see if the aliased type is valid. - Type::Alias(..) => false, + Type::Alias(alias, generics) => { + let alias = alias.borrow(); + alias.get_type(generics).is_valid_for_program_input() + } Type::Array(length, element) => { length.is_valid_for_program_input() && element.is_valid_for_program_input() @@ -1768,9 +1768,11 @@ impl From<&Type> for PrintableType { Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(_, _, env) => { - PrintableType::Function { env: Box::new(env.as_ref().into()) } - } + Type::Function(arguments, return_type, env) => PrintableType::Function { + arguments: arguments.iter().map(|arg| arg.into()).collect(), + return_type: Box::new(return_type.as_ref().into()), + env: Box::new(env.as_ref().into()), + }, Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index e4e619d5d92..7fcf8e87792 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -1,7 +1,7 @@ use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{ - debug_info::{DebugTypes, DebugVariables}, + debug_info::{DebugFunctions, DebugTypes, DebugVariables}, Location, }; @@ -253,6 +253,7 @@ pub struct Program { /// Indicates to a backend whether a SNARK-friendly prover should be used. pub recursive: bool, pub debug_variables: DebugVariables, + pub debug_functions: DebugFunctions, pub debug_types: DebugTypes, } @@ -266,6 +267,7 @@ impl Program { return_visibility: Visibility, recursive: bool, debug_variables: DebugVariables, + debug_functions: DebugFunctions, debug_types: DebugTypes, ) -> Program { Program { @@ -276,6 +278,7 @@ impl Program { return_visibility, recursive, debug_variables, + debug_functions, debug_types, } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index a8ff4399f99..cf4e0ab792e 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -76,7 +76,7 @@ impl<'interner> Monomorphizer<'interner> { let var_type = self.interner.id_type(call.arguments[DEBUG_VALUE_ARG_SLOT]); let source_var_id = source_var_id.to_u128().into(); // then update the ID used for tracking at runtime - let var_id = self.debug_type_tracker.insert_var(source_var_id, var_type); + let var_id = self.debug_type_tracker.insert_var(source_var_id, &var_type); let interned_var_id = self.intern_var_id(var_id, &call.location); arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id)?; Ok(()) diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs index fea073d394f..16b82d1e7b9 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug_types.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -3,7 +3,8 @@ use crate::{ hir_def::types::Type, }; use noirc_errors::debug_info::{ - DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, + DebugFnId, DebugFunction, DebugFunctions, DebugTypeId, DebugTypes, DebugVarId, DebugVariable, + DebugVariables, }; use noirc_printable_type::PrintableType; use std::collections::HashMap; @@ -30,7 +31,10 @@ pub struct DebugTypeTracker { // All instances of tracked variables variables: HashMap, - // Types of tracked variables + // Function metadata collected during instrumentation injection + functions: HashMap, + + // Types of tracked variables and functions types: HashMap, types_reverse: HashMap, @@ -43,34 +47,29 @@ impl DebugTypeTracker { DebugTypeTracker { source_variables: instrumenter.variables.clone(), source_field_names: instrumenter.field_names.clone(), + functions: instrumenter.functions.clone(), ..DebugTypeTracker::default() } } - pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugTypes) { + pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugFunctions, DebugTypes) { let debug_variables = self .variables .clone() .into_iter() .map(|(var_id, (source_var_id, type_id))| { - ( - var_id, - DebugVariable { - name: self.source_variables.get(&source_var_id).cloned().unwrap_or_else( - || { - unreachable!( - "failed to retrieve variable name for {source_var_id:?}" - ); - }, - ), - debug_type_id: type_id, - }, - ) + let var_name = + self.source_variables.get(&source_var_id).cloned().unwrap_or_else(|| { + unreachable!("failed to retrieve variable name for {source_var_id:?}"); + }); + (var_id, DebugVariable { name: var_name, debug_type_id: type_id }) }) .collect(); + + let debug_functions = self.functions.clone().into_iter().collect(); let debug_types = self.types.clone().into_iter().collect(); - (debug_variables, debug_types) + (debug_variables, debug_functions, debug_types) } pub fn resolve_field_index( @@ -83,19 +82,24 @@ impl DebugTypeTracker { .and_then(|field_name| get_field(cursor_type, field_name)) } - pub fn insert_var(&mut self, source_var_id: SourceVarId, var_type: Type) -> DebugVarId { - if !self.source_variables.contains_key(&source_var_id) { - unreachable!("cannot find source debug variable {source_var_id:?}"); - } - - let ptype: PrintableType = var_type.follow_bindings().into(); - let type_id = self.types_reverse.get(&ptype).copied().unwrap_or_else(|| { + fn insert_type(&mut self, the_type: &Type) -> DebugTypeId { + let ptype: PrintableType = the_type.follow_bindings().into(); + self.types_reverse.get(&ptype).copied().unwrap_or_else(|| { let type_id = DebugTypeId(self.next_type_id); self.next_type_id += 1; self.types_reverse.insert(ptype.clone(), type_id); self.types.insert(type_id, ptype); type_id - }); + }) + } + + pub fn insert_var(&mut self, source_var_id: SourceVarId, var_type: &Type) -> DebugVarId { + if !self.source_variables.contains_key(&source_var_id) { + unreachable!("cannot find source debug variable {source_var_id:?}"); + } + + let type_id = self.insert_type(var_type); + // check if we need to instantiate the var with a new type let existing_var_id = self.source_to_debug_vars.get(&source_var_id).and_then(|var_id| { let (_, existing_type_id) = self.variables.get(var_id).unwrap(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ce880401d77..cfd9a61d13f 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -165,7 +165,8 @@ pub fn monomorphize_debug( let FuncMeta { return_distinctness, return_visibility, kind, .. } = monomorphizer.interner.function_meta(&main); - let (debug_variables, debug_types) = monomorphizer.debug_type_tracker.extract_vars_and_types(); + let (debug_variables, debug_functions, debug_types) = + monomorphizer.debug_type_tracker.extract_vars_and_types(); let program = Program::new( functions, function_sig, @@ -174,6 +175,7 @@ pub fn monomorphize_debug( *return_visibility, *kind == FunctionKind::Recursive, debug_variables, + debug_functions, debug_types, ); Ok(program) diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 0ff7819c00f..ea96dee8a47 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -14,8 +14,8 @@ mod parser; use crate::token::{Keyword, Token}; use crate::{ast::ImportStatement, Expression, NoirStruct}; use crate::{ - Ident, LetStatement, NoirFunction, NoirTrait, NoirTraitImpl, NoirTypeAlias, Recoverable, - StatementKind, TypeImpl, UseTree, + Ident, LetStatement, ModuleDeclaration, NoirFunction, NoirTrait, NoirTraitImpl, NoirTypeAlias, + Recoverable, StatementKind, TypeImpl, UseTree, }; use chumsky::prelude::*; @@ -28,7 +28,7 @@ pub use parser::parse_program; #[derive(Debug, Clone)] pub(crate) enum TopLevelStatement { Function(NoirFunction), - Module(Ident), + Module(ModuleDeclaration), Import(UseTree), Struct(NoirStruct), Trait(NoirTrait), @@ -220,7 +220,7 @@ pub struct SortedModule { pub globals: Vec, /// Module declarations like `mod foo;` - pub module_decls: Vec, + pub module_decls: Vec, /// Full submodules as in `mod foo { ... definitions ... }` pub submodules: Vec, @@ -229,7 +229,7 @@ pub struct SortedModule { impl std::fmt::Display for SortedModule { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { for decl in &self.module_decls { - writeln!(f, "mod {decl};")?; + writeln!(f, "{decl};")?; } for import in &self.imports { @@ -309,7 +309,7 @@ pub enum ItemKind { Impl(TypeImpl), TypeAlias(NoirTypeAlias), Global(LetStatement), - ModuleDecl(Ident), + ModuleDecl(ModuleDeclaration), Submodules(ParsedSubModule), } @@ -380,8 +380,8 @@ impl SortedModule { self.imports.extend(import_stmt.desugar(None)); } - fn push_module_decl(&mut self, mod_name: Ident) { - self.module_decls.push(mod_name); + fn push_module_decl(&mut self, mod_decl: ModuleDeclaration) { + self.module_decls.push(mod_decl); } fn push_submodule(&mut self, submodule: SortedSubModule) { @@ -474,7 +474,7 @@ impl std::fmt::Display for TopLevelStatement { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { TopLevelStatement::Function(fun) => fun.fmt(f), - TopLevelStatement::Module(m) => write!(f, "mod {m}"), + TopLevelStatement::Module(m) => m.fmt(f), TopLevelStatement::Import(tree) => write!(f, "use {tree}"), TopLevelStatement::Trait(t) => t.fmt(f), TopLevelStatement::TraitImpl(i) => i.fmt(f), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 75f4a6359bf..383a1ffafc9 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -40,9 +40,9 @@ use crate::parser::{force, ignore_then_commit, statement_recovery}; use crate::token::{Keyword, Token, TokenKind}; use crate::{ BinaryOp, BinaryOpKind, BlockExpression, Distinctness, ForLoopStatement, ForRange, - FunctionReturnType, Ident, IfExpression, InfixExpression, LValue, Literal, NoirTypeAlias, - Param, Path, Pattern, Recoverable, Statement, TraitBound, TypeImpl, UnresolvedTraitConstraint, - UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, + FunctionReturnType, Ident, IfExpression, InfixExpression, LValue, Literal, ModuleDeclaration, + NoirTypeAlias, Param, Path, Pattern, Recoverable, Statement, TraitBound, TypeImpl, + UnresolvedTraitConstraint, UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, }; use chumsky::prelude::*; @@ -370,7 +370,9 @@ fn optional_type_annotation<'a>() -> impl NoirParser + 'a { } fn module_declaration() -> impl NoirParser { - keyword(Keyword::Mod).ignore_then(ident()).map(TopLevelStatement::Module) + keyword(Keyword::Mod) + .ignore_then(ident()) + .map(|ident| TopLevelStatement::Module(ModuleDeclaration { ident })) } fn use_statement() -> impl NoirParser { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c661cc92eef..9be6252b10a 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -81,7 +81,7 @@ mod test { &mut context, program.clone().into_sorted(), root_file_id, - Vec::new(), // No macro processors + &[], // No macro processors )); } (program, context, errors) @@ -1206,4 +1206,13 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { "#; assert_eq!(get_program_errors(src).len(), 1); } + + #[test] + fn type_aliases_in_entry_point() { + let src = r#" + type Foo = u8; + fn main(_x: Foo) {} + "#; + assert_eq!(get_program_errors(src).len(), 0); + } } diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 24f4f275a14..60f233cd86d 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -33,6 +33,8 @@ pub enum PrintableType { length: u64, }, Function { + arguments: Vec, + return_type: Box, env: Box, }, MutableReference { @@ -176,8 +178,8 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str("false"); } } - (PrintableValue::Field(_), PrintableType::Function { .. }) => { - output.push_str("<>"); + (PrintableValue::Field(_), PrintableType::Function { arguments, return_type, .. }) => { + output.push_str(&format!("< {:?}>>", arguments, return_type,)); } (_, PrintableType::MutableReference { .. }) => { output.push_str("<>"); @@ -350,7 +352,7 @@ pub fn decode_value( PrintableValue::Struct(struct_map) } - PrintableType::Function { env } => { + PrintableType::Function { env, .. } => { let field_element = field_iterator.next().unwrap(); let func_ref = PrintableValue::Field(field_element); // we want to consume the fields from the environment, but for now they are not actually printed diff --git a/compiler/wasm/src/noir/package.ts b/compiler/wasm/src/noir/package.ts index 81178e6ae96..2856798273a 100644 --- a/compiler/wasm/src/noir/package.ts +++ b/compiler/wasm/src/noir/package.ts @@ -105,7 +105,12 @@ export class Package { handles .filter((handle) => SOURCE_EXTENSIONS.find((ext) => handle.endsWith(ext))) .map(async (file) => { - const suffix = file.replace(this.#srcPath, ''); + // Github deps are directly added to the file manager, which causes them to be missing the absolute path to the source file + // and only include the extraction directory relative to the fm root directory + // This regexp ensures we remove the "real" source path for all dependencies, providing the compiler with what it expects for each source file: + // -> for bin/contract packages + // -> for libs + const suffix = file.replace(new RegExp(`.*${this.#srcPath}`), ''); return { path: this.getType() === 'lib' ? `${alias ? alias : this.#config.package.name}${suffix}` : file, source: (await fm.readFile(file, 'utf-8')).toString(), diff --git a/test_programs/execution_success/regression_4449/Nargo.toml b/test_programs/execution_success/regression_4449/Nargo.toml new file mode 100644 index 00000000000..925420a03a8 --- /dev/null +++ b/test_programs/execution_success/regression_4449/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_4449" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_4449/Prover.toml b/test_programs/execution_success/regression_4449/Prover.toml new file mode 100644 index 00000000000..81af476bcc9 --- /dev/null +++ b/test_programs/execution_success/regression_4449/Prover.toml @@ -0,0 +1,3 @@ + +x = 0xbd +result = [204, 59, 83, 197, 18, 1, 128, 43, 247, 28, 104, 225, 106, 13, 20, 187, 42, 26, 67, 150, 48, 75, 238, 168, 121, 247, 142, 160, 71, 222, 97, 188] \ No newline at end of file diff --git a/test_programs/execution_success/regression_4449/src/main.nr b/test_programs/execution_success/regression_4449/src/main.nr new file mode 100644 index 00000000000..454a93f5d1a --- /dev/null +++ b/test_programs/execution_success/regression_4449/src/main.nr @@ -0,0 +1,14 @@ +// Regression test for issue #4449 +use dep::std; + +fn main(x: u8, result: [u8; 32]) { + let x = x % 31; + let mut digest = [0; 32]; + for i in 0..70 { + let y = x + i; + let a = [y, x, 32, 0, y + 1, y - 1, y - 2, 5]; + digest = std::sha256::digest(a); + } + + assert(digest == result); +} diff --git a/test_programs/execution_success/slices/src/main.nr b/test_programs/execution_success/slices/src/main.nr index eca42a660c4..6823bf05d96 100644 --- a/test_programs/execution_success/slices/src/main.nr +++ b/test_programs/execution_success/slices/src/main.nr @@ -50,6 +50,8 @@ fn main(x: Field, y: pub Field) { // The parameters to this function must come from witness values (inputs to main) regression_merge_slices(x, y); regression_2370(); + + regression_4506(); } // Ensure that slices of struct/tuple values work. fn regression_2083() { @@ -297,3 +299,8 @@ fn regression_2370() { let mut slice = []; slice = [1, 2, 3]; } + +fn regression_4506() { + let slice: [Field] = [1, 2, 3]; + assert(slice == slice); +} diff --git a/tooling/bb_abstraction_leaks/build.rs b/tooling/bb_abstraction_leaks/build.rs index f9effd5d991..0bd2b1c076a 100644 --- a/tooling/bb_abstraction_leaks/build.rs +++ b/tooling/bb_abstraction_leaks/build.rs @@ -10,7 +10,7 @@ use const_format::formatcp; const USERNAME: &str = "AztecProtocol"; const REPO: &str = "aztec-packages"; -const VERSION: &str = "0.24.0"; +const VERSION: &str = "0.26.3"; const TAG: &str = formatcp!("aztec-packages-v{}", VERSION); const API_URL: &str = diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index c472e828739..231d4d897a9 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -1,21 +1,13 @@ array_dynamic_blackbox_input -array_sort -assign_ex +bigint bit_shifts_comptime -brillig_cow -brillig_nested_arrays brillig_references brillig_to_bytes_integration debug_logs double_verify_nested_proof double_verify_proof modulus -nested_array_dynamic -nested_array_in_slice -nested_arrays_from_brillig references scalar_mul signed_comparison -simple_2d_array to_bytes_integration -bigint diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 515edf0bb06..a3ee89263a4 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -8,11 +8,14 @@ use acvm::pwg::{ }; use acvm::{BlackBoxFunctionSolver, FieldElement}; -use nargo::artifacts::debug::DebugArtifact; +use codespan_reporting::files::{Files, SimpleFile}; +use fm::FileId; +use nargo::artifacts::debug::{DebugArtifact, StackFrame}; use nargo::errors::{ExecutionError, Location}; use nargo::NargoError; -use noirc_printable_type::{PrintableType, PrintableValue}; +use noirc_driver::DebugFile; +use std::collections::BTreeMap; use std::collections::{hash_set::Iter, HashSet}; #[derive(Debug)] @@ -29,6 +32,7 @@ pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> { foreign_call_executor: Box, debug_artifact: &'a DebugArtifact, breakpoints: HashSet, + source_to_opcodes: BTreeMap>, } impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { @@ -39,12 +43,14 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness: WitnessMap, foreign_call_executor: Box, ) -> Self { + let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact); Self { acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), brillig_solver: None, foreign_call_executor, debug_artifact, breakpoints: HashSet::new(), + source_to_opcodes, } } @@ -100,10 +106,50 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { self.debug_artifact .file_map .get(&location.file) - .map(|file| file.path.starts_with("__debug/")) + .map(is_debug_file_in_debug_crate) .unwrap_or(false) } + /// Find an opcode location matching a source code location + // We apply some heuristics here, and there are four possibilities for the + // return value of this function: + // 1. the source location is not found -> None + // 2. an exact unique location is found (very rare) -> Some(opcode_location) + // 3. an exact but not unique location is found, ie. a source location may + // be mapped to multiple opcodes, and those may be disjoint, for example for + // functions called multiple times throughout the program + // -> return the first opcode in program order that matches the source location + // 4. exact location is not found, so an opcode for a nearby source location + // is returned (this again could actually be more than one opcodes) + // -> return the opcode for the next source line that is mapped + pub(super) fn find_opcode_for_source_location( + &self, + file_id: &FileId, + line: i64, + ) -> Option { + let line = line as usize; + let Some(line_to_opcodes) = self.source_to_opcodes.get(file_id) else { + return None; + }; + let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) { + Ok(index) => { + // move backwards to find the first opcode which matches the line + let mut index = index; + while index > 0 && line_to_opcodes[index - 1].0 == line { + index -= 1; + } + line_to_opcodes[index].1 + } + Err(index) => { + if index >= line_to_opcodes.len() { + return None; + } + line_to_opcodes[index].1 + } + }; + Some(found_index) + } + /// Returns the callstack in source code locations for the currently /// executing opcode. This can be `None` if the execution finished (and /// `get_current_opcode_location()` returns `None`) or if the opcode is not @@ -128,6 +174,9 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { &self, opcode_location: &OpcodeLocation, ) -> Vec { + // TODO: this assumes we're debugging a program (ie. the DebugArtifact + // will contain a single DebugInfo), but this assumption doesn't hold + // for contracts self.debug_artifact.debug_symbols[0] .opcode_location(opcode_location) .map(|source_locations| { @@ -467,10 +516,14 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } - pub(super) fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + pub(super) fn get_variables(&self) -> Vec { return self.foreign_call_executor.get_variables(); } + pub(super) fn current_stack_frame(&self) -> Option { + return self.foreign_call_executor.current_stack_frame(); + } + fn breakpoint_reached(&self) -> bool { if let Some(location) = self.get_current_opcode_location() { self.breakpoints.contains(&location) @@ -526,6 +579,52 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } +fn is_debug_file_in_debug_crate(debug_file: &DebugFile) -> bool { + debug_file.path.starts_with("__debug/") +} + +/// Builds a map from FileId to an ordered vector of tuples with line +/// numbers and opcode locations corresponding to those line numbers +fn build_source_to_opcode_debug_mappings( + debug_artifact: &DebugArtifact, +) -> BTreeMap> { + if debug_artifact.debug_symbols.is_empty() { + return BTreeMap::new(); + } + let locations = &debug_artifact.debug_symbols[0].locations; + let simple_files: BTreeMap<_, _> = debug_artifact + .file_map + .iter() + .filter(|(_, debug_file)| !is_debug_file_in_debug_crate(debug_file)) + .map(|(file_id, debug_file)| { + ( + file_id, + SimpleFile::new(debug_file.path.to_str().unwrap(), debug_file.source.as_str()), + ) + }) + .collect(); + + let mut result: BTreeMap> = BTreeMap::new(); + locations.iter().for_each(|(opcode_location, source_locations)| { + source_locations.iter().for_each(|source_location| { + let span = source_location.span; + let file_id = source_location.file; + let Some(file) = simple_files.get(&file_id) else { + return; + }; + let Ok(line_index) = file.line_index((), span.start() as usize) else { + return; + }; + let line_number = line_index + 1; + + result.entry(file_id).or_default().push((line_number, *opcode_location)); + }); + }); + result.iter_mut().for_each(|(_, file_locations)| file_locations.sort_by_key(|x| (x.0, x.1))); + + result +} + #[cfg(test)] mod tests { use super::*; diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index 7e67a26b257..7c722ed0a61 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -5,7 +5,6 @@ use std::str::FromStr; use acvm::acir::circuit::{Circuit, OpcodeLocation}; use acvm::acir::native_types::WitnessMap; use acvm::BlackBoxFunctionSolver; -use codespan_reporting::files::{Files, SimpleFile}; use crate::context::DebugCommandResult; use crate::context::DebugContext; @@ -30,15 +29,16 @@ use nargo::artifacts::debug::DebugArtifact; use fm::FileId; use noirc_driver::CompiledProgram; +type BreakpointId = i64; + pub struct DapSession<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> { server: Server, context: DebugContext<'a, B>, debug_artifact: &'a DebugArtifact, running: bool, - source_to_opcodes: BTreeMap>, - next_breakpoint_id: i64, - instruction_breakpoints: Vec<(OpcodeLocation, i64)>, - source_breakpoints: BTreeMap>, + next_breakpoint_id: BreakpointId, + instruction_breakpoints: Vec<(OpcodeLocation, BreakpointId)>, + source_breakpoints: BTreeMap>, } enum ScopeReferences { @@ -57,8 +57,6 @@ impl From for ScopeReferences { } } -// BTreeMap - impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { pub fn new( server: Server, @@ -67,7 +65,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, ) -> Self { - let source_to_opcodes = Self::build_source_to_opcode_debug_mappings(debug_artifact); let context = DebugContext::new( solver, circuit, @@ -79,7 +76,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { server, context, debug_artifact, - source_to_opcodes, running: false, next_breakpoint_id: 1, instruction_breakpoints: vec![], @@ -87,46 +83,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } } - /// Builds a map from FileId to an ordered vector of tuples with line - /// numbers and opcode locations corresponding to those line numbers - fn build_source_to_opcode_debug_mappings( - debug_artifact: &'a DebugArtifact, - ) -> BTreeMap> { - if debug_artifact.debug_symbols.is_empty() { - return BTreeMap::new(); - } - let locations = &debug_artifact.debug_symbols[0].locations; - let simple_files: BTreeMap<_, _> = debug_artifact - .file_map - .iter() - .map(|(file_id, debug_file)| { - ( - file_id, - SimpleFile::new(debug_file.path.to_str().unwrap(), debug_file.source.as_str()), - ) - }) - .collect(); - - let mut result: BTreeMap> = BTreeMap::new(); - locations.iter().for_each(|(opcode_location, source_locations)| { - if source_locations.is_empty() { - return; - } - let source_location = source_locations[0]; - let span = source_location.span; - let file_id = source_location.file; - let Ok(line_index) = &simple_files[&file_id].line_index((), span.start() as usize) - else { - return; - }; - let line_number = line_index + 1; - - result.entry(file_id).or_default().push((line_number, *opcode_location)); - }); - result.iter_mut().for_each(|(_, file_locations)| file_locations.sort_by_key(|x| x.0)); - result - } - fn send_stopped_event(&mut self, reason: StoppedEventReason) -> Result<(), ServerError> { let description = format!("{:?}", &reason); self.server.send_event(Event::Stopped(StoppedEventBody { @@ -230,6 +186,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_stack_trace(&self) -> Vec { + let stack_frames = self.context.get_variables(); + self.context .get_source_call_stack() .iter() @@ -239,9 +197,15 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { self.debug_artifact.location_line_number(*source_location).unwrap(); let column_number = self.debug_artifact.location_column_number(*source_location).unwrap(); + + let name = match stack_frames.get(index) { + Some(frame) => format!("{} {}", frame.function_name, index), + None => format!("frame #{index}"), + }; + StackFrame { id: index as i64, - name: format!("frame #{index}"), + name, source: Some(Source { path: self.debug_artifact.file_map[&source_location.file] .path @@ -422,7 +386,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { Ok(()) } - fn get_next_breakpoint_id(&mut self) -> i64 { + fn get_next_breakpoint_id(&mut self) -> BreakpointId { let id = self.next_breakpoint_id; self.next_breakpoint_id += 1; id @@ -493,36 +457,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { found.map(|iter| *iter.0) } - // TODO: there are four possibilities for the return value of this function: - // 1. the source location is not found -> None - // 2. an exact unique location is found -> Some(opcode_location) - // 3. an exact but not unique location is found (ie. a source location may - // be mapped to multiple opcodes, and those may be disjoint, for example for - // functions called multiple times throughout the program) - // 4. exact location is not found, so an opcode for a nearby source location - // is returned (this again could actually be more than one opcodes) - // Case 3 is not supported yet, and 4 is not correctly handled. - fn find_opcode_for_source_location( - &self, - file_id: &FileId, - line: i64, - ) -> Option { - let line = line as usize; - let Some(line_to_opcodes) = self.source_to_opcodes.get(file_id) else { - return None; - }; - let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) { - Ok(index) => line_to_opcodes[index].1, - Err(index) => { - if index >= line_to_opcodes.len() { - return None; - } - line_to_opcodes[index].1 - } - }; - Some(found_index) - } - fn map_source_breakpoints(&mut self, args: &SetBreakpointsArguments) -> Vec { let Some(ref source) = &args.source.path else { return vec![]; @@ -539,7 +473,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { .iter() .map(|breakpoint| { let line = breakpoint.line; - let Some(location) = self.find_opcode_for_source_location(&file_id, line) else { + let Some(location) = self.context.find_opcode_for_source_location(&file_id, line) + else { return Breakpoint { verified: false, message: Some(String::from( @@ -608,16 +543,20 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_local_variables(&self) -> Vec { - let mut variables: Vec<_> = self - .context - .get_variables() + let Some(current_stack_frame) = self.context.current_stack_frame() else { + return vec![]; + }; + + let mut variables = current_stack_frame + .variables .iter() .map(|(name, value, _var_type)| Variable { name: String::from(*name), value: format!("{:?}", *value), ..Variable::default() }) - .collect(); + .collect::>(); + variables.sort_by(|a, b| a.name.partial_cmp(&b.name).unwrap()); variables } diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 68c4d3947b0..25f126ff490 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -3,17 +3,19 @@ use acvm::{ pwg::ForeignCallWaitInfo, }; use nargo::{ - artifacts::debug::{DebugArtifact, DebugVars}, + artifacts::debug::{DebugArtifact, DebugVars, StackFrame}, ops::{DefaultForeignCallExecutor, ForeignCallExecutor, NargoForeignCallResult}, }; -use noirc_errors::debug_info::DebugVarId; -use noirc_printable_type::{ForeignCallError, PrintableType, PrintableValue}; +use noirc_errors::debug_info::{DebugFnId, DebugVarId}; +use noirc_printable_type::ForeignCallError; pub(crate) enum DebugForeignCall { VarAssign, VarDrop, MemberAssign(u32), DerefAssign, + FnEnter, + FnExit, } impl DebugForeignCall { @@ -28,13 +30,16 @@ impl DebugForeignCall { "__debug_var_assign" => Some(DebugForeignCall::VarAssign), "__debug_var_drop" => Some(DebugForeignCall::VarDrop), "__debug_deref_assign" => Some(DebugForeignCall::DerefAssign), + "__debug_fn_enter" => Some(DebugForeignCall::FnEnter), + "__debug_fn_exit" => Some(DebugForeignCall::FnExit), _ => None, } } } pub trait DebugForeignCallExecutor: ForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)>; + fn get_variables(&self) -> Vec; + fn current_stack_frame(&self) -> Option; } pub struct DefaultDebugForeignCallExecutor { @@ -57,23 +62,33 @@ impl DefaultDebugForeignCallExecutor { } pub fn load_artifact(&mut self, artifact: &DebugArtifact) { - artifact.debug_symbols.iter().for_each(|info| { - self.debug_vars.insert_variables(&info.variables); - self.debug_vars.insert_types(&info.types); - }); + // TODO: handle loading from the correct DebugInfo when we support + // debugging contracts + let Some(info) = artifact.debug_symbols.get(0) else { + return; + }; + self.debug_vars.insert_debug_info(info); } } impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + fn get_variables(&self) -> Vec { self.debug_vars.get_variables() } + + fn current_stack_frame(&self) -> Option { + self.debug_vars.current_stack_frame() + } } fn debug_var_id(value: &Value) -> DebugVarId { DebugVarId(value.to_u128() as u32) } +fn debug_fn_id(value: &Value) -> DebugFnId { + DebugFnId(value.to_u128() as u32) +} + impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { fn execute( &mut self, @@ -136,6 +151,19 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { } Ok(ForeignCallResult::default().into()) } + Some(DebugForeignCall::FnEnter) => { + let fcp_fn_id = &foreign_call.inputs[0]; + let ForeignCallParam::Single(fn_id_value) = fcp_fn_id else { + panic!("unexpected foreign call parameter in fn enter: {fcp_fn_id:?}") + }; + let fn_id = debug_fn_id(fn_id_value); + self.debug_vars.push_fn(fn_id); + Ok(ForeignCallResult::default().into()) + } + Some(DebugForeignCall::FnExit) => { + self.debug_vars.pop_fn(); + Ok(ForeignCallResult::default().into()) + } None => self.executor.execute(foreign_call), } } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 8441dbde9be..41dbf604f99 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -337,11 +337,13 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } pub fn show_vars(&self) { - let vars = self.context.get_variables(); - for (var_name, value, var_type) in vars.iter() { - let printable_value = - PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); - println!("{var_name}:{var_type:?} = {}", printable_value); + for frame in self.context.get_variables() { + println!("{}({})", frame.function_name, frame.function_params.join(", ")); + for (var_name, value, var_type) in frame.variables.iter() { + let printable_value = + PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); + println!(" {var_name}:{var_type:?} = {}", printable_value); + } } } @@ -530,7 +532,7 @@ pub fn run( .add( "vars", command! { - "show variable values available at this point in execution", + "show variables for each function scope available at this point in execution", () => || { ref_context.borrow_mut().show_vars(); Ok(CommandStatus::Done) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index b5ffdb12d01..e298eb8aadd 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -30,7 +30,7 @@ struct LocationPrintContext { // Given a DebugArtifact and an OpcodeLocation, prints all the source code // locations the OpcodeLocation maps to, with some surrounding context and // visual aids to highlight the location itself. -pub(crate) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { +pub(super) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { let locations = locations.iter(); for loc in locations { @@ -269,8 +269,12 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = - vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; + let debug_symbols = vec![DebugInfo::new( + opcode_locations, + BTreeMap::default(), + BTreeMap::default(), + BTreeMap::default(), + )]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_rendered: Vec<_> = render_location(&debug_artifact, &loc).collect(); diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs index 4cb678192b8..143ee7987f8 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -12,7 +12,7 @@ mod tests { let nargo_bin = cargo_bin("nargo").into_os_string().into_string().expect("Cannot parse nargo path"); - let mut dbg_session = spawn_bash(Some(10000)).expect("Could not start bash session"); + let mut dbg_session = spawn_bash(Some(15000)).expect("Could not start bash session"); // Set backend to `/dev/null` to force an error if nargo tries to speak to a backend. dbg_session diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index a249ecb03ad..fbdf59805c9 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -8,7 +8,7 @@ use std::{ ops::Range, }; -pub use super::debug_vars::DebugVars; +pub use super::debug_vars::{DebugVars, StackFrame}; use fm::{FileId, FileManager, PathString}; /// A Debug Artifact stores, for a given program, the debug info for every function @@ -231,8 +231,12 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = - vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; + let debug_symbols = vec![DebugInfo::new( + opcode_locations, + BTreeMap::default(), + BTreeMap::default(), + BTreeMap::default(), + )]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_in_line = debug_artifact.location_in_line(loc).expect("Expected a range"); diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 20f2637f7d6..66568bec833 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -1,54 +1,85 @@ use acvm::brillig_vm::brillig::Value; use noirc_errors::debug_info::{ - DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, + DebugFnId, DebugFunction, DebugInfo, DebugTypeId, DebugVarId, DebugVariable, }; use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; #[derive(Debug, Default, Clone)] pub struct DebugVars { variables: HashMap, + functions: HashMap, types: HashMap, - active: HashSet, - values: HashMap, + frames: Vec<(DebugFnId, HashMap)>, +} + +pub struct StackFrame<'a> { + pub function_name: &'a str, + pub function_params: Vec<&'a str>, + pub variables: Vec<(&'a str, &'a PrintableValue, &'a PrintableType)>, } impl DebugVars { - pub fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { - self.active - .iter() - .filter_map(|var_id| { - self.variables.get(var_id).and_then(|debug_var| { - let Some(value) = self.values.get(var_id) else { - return None; - }; - let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { - return None; - }; - Some((debug_var.name.as_str(), value, ptype)) - }) - }) - .collect() + pub fn insert_debug_info(&mut self, info: &DebugInfo) { + self.variables.extend(info.variables.clone()); + self.types.extend(info.types.clone()); + self.functions.extend(info.functions.clone()); + } + + pub fn get_variables(&self) -> Vec { + self.frames.iter().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)).collect() } - pub fn insert_variables(&mut self, vars: &DebugVariables) { - self.variables.extend(vars.clone()); + pub fn current_stack_frame(&self) -> Option { + self.frames.last().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)) } - pub fn insert_types(&mut self, types: &DebugTypes) { - self.types.extend(types.clone()); + fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableType)> { + self.variables.get(&var_id).and_then(|debug_var| { + let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { + return None; + }; + Some((debug_var.name.as_str(), ptype)) + }) + } + + fn build_stack_frame<'a>( + &'a self, + fn_id: &DebugFnId, + frame: &'a HashMap, + ) -> StackFrame { + let debug_fn = &self.functions.get(fn_id).expect("failed to find function metadata"); + + let params: Vec<&str> = + debug_fn.arg_names.iter().map(|arg_name| arg_name.as_str()).collect(); + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame + .iter() + .filter_map(|(var_id, var_value)| { + self.lookup_var(*var_id).map(|(name, typ)| (name, var_value, typ)) + }) + .collect(); + + StackFrame { + function_name: debug_fn.name.as_str(), + function_params: params, + variables: vars, + } } pub fn assign_var(&mut self, var_id: DebugVarId, values: &[Value]) { - self.active.insert(var_id); let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; let ptype = self.types.get(type_id).unwrap(); - self.values.insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); + + self.frames + .last_mut() + .expect("unexpected empty stack frames") + .1 + .insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); } pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[Value]) { - let mut cursor: &mut PrintableValue = self - .values + let current_frame = &mut self.frames.last_mut().expect("unexpected empty stack frames").1; + let mut cursor: &mut PrintableValue = current_frame .get_mut(&var_id) .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); let cursor_type_id = &self @@ -102,7 +133,6 @@ impl DebugVars { }; } *cursor = decode_value(&mut values.iter().map(|v| v.to_field()), cursor_type); - self.active.insert(var_id); } pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[Value]) { @@ -114,6 +144,14 @@ impl DebugVars { } pub fn drop_var(&mut self, var_id: DebugVarId) { - self.active.remove(&var_id); + self.frames.last_mut().expect("unexpected empty stack frames").1.remove(&var_id); + } + + pub fn push_fn(&mut self, fn_id: DebugFnId) { + self.frames.push((fn_id, HashMap::default())); + } + + pub fn pop_fn(&mut self) { + self.frames.pop(); } } diff --git a/tooling/noir_js_backend_barretenberg/package.json b/tooling/noir_js_backend_barretenberg/package.json index 28c3609fd14..06c034725e3 100644 --- a/tooling/noir_js_backend_barretenberg/package.json +++ b/tooling/noir_js_backend_barretenberg/package.json @@ -42,7 +42,7 @@ "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" }, "dependencies": { - "@aztec/bb.js": "0.24.0", + "@aztec/bb.js": "0.26.3", "@noir-lang/types": "workspace:*", "fflate": "^0.8.0" }, diff --git a/yarn.lock b/yarn.lock index f5f3a29f08a..49485193532 100644 --- a/yarn.lock +++ b/yarn.lock @@ -221,9 +221,9 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@npm:0.24.0": - version: 0.24.0 - resolution: "@aztec/bb.js@npm:0.24.0" +"@aztec/bb.js@npm:0.26.3": + version: 0.26.3 + resolution: "@aztec/bb.js@npm:0.26.3" dependencies: comlink: ^4.4.1 commander: ^10.0.1 @@ -231,7 +231,7 @@ __metadata: tslib: ^2.4.0 bin: bb.js: dest/node/main.js - checksum: a086dabf30084cfa526e512148b9c02f0a0770dcc19b7dca4af9a3e98612b716acc7eaac6b52c0f12d985932e866d1cb9e534ded6ac9d747f3dd021afe25de27 + checksum: 74c2b7ef5405f56472cf7c41d1c13261df07b1d5019e3ede9b63d218378e0fb73ccf5c52f1cc524505efad5799b347b497349d7c9b6fe82286014b1574f12309 languageName: node linkType: hard @@ -4396,7 +4396,7 @@ __metadata: version: 0.0.0-use.local resolution: "@noir-lang/backend_barretenberg@workspace:tooling/noir_js_backend_barretenberg" dependencies: - "@aztec/bb.js": 0.24.0 + "@aztec/bb.js": 0.26.3 "@noir-lang/types": "workspace:*" "@types/node": ^20.6.2 "@types/prettier": ^3