From 2433740d38825d585bd07d14b192be3647343559 Mon Sep 17 00:00:00 2001 From: synthia Date: Wed, 1 Nov 2023 19:49:28 -0700 Subject: [PATCH 01/11] feat: instrumentation for runtime variable inspection in debug mode --- compiler/noirc_errors/src/debug_info.rs | 22 +- compiler/noirc_evaluator/src/ssa.rs | 3 +- compiler/noirc_frontend/src/debug/mod.rs | 202 ++++++++++++++++++ .../src/hir/def_collector/dc_mod.rs | 5 +- .../noirc_frontend/src/hir/def_map/mod.rs | 7 +- compiler/noirc_frontend/src/hir/mod.rs | 25 ++- .../src/hir/resolution/import.rs | 12 +- compiler/noirc_frontend/src/lib.rs | 1 + tooling/debugger/Cargo.toml | 2 + tooling/debugger/src/context.rs | 14 +- tooling/debugger/src/repl.rs | 15 ++ tooling/nargo/src/artifacts/debug.rs | 28 +-- tooling/nargo/src/artifacts/debug_vars.rs | 45 ++++ tooling/nargo/src/artifacts/mod.rs | 1 + tooling/nargo/src/lib.rs | 1 + tooling/nargo/src/ops/foreign_calls.rs | 34 +++ tooling/nargo_cli/src/cli/compile_cmd.rs | 9 +- 17 files changed, 386 insertions(+), 40 deletions(-) create mode 100644 compiler/noirc_frontend/src/debug/mod.rs create mode 100644 tooling/nargo/src/artifacts/debug_vars.rs diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 888c24adc1a..706f6962208 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -1,10 +1,10 @@ use acvm::acir::circuit::OpcodeLocation; use acvm::compiler::AcirTransformationMap; +use fm::FileId; use serde_with::serde_as; use serde_with::DisplayFromStr; -use std::collections::BTreeMap; -use std::collections::HashMap; +use std::collections::{BTreeMap,HashMap}; use std::mem; use crate::Location; @@ -18,6 +18,7 @@ pub struct DebugInfo { /// that they should be serialized to/from strings. #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, + pub variables: HashMap, } /// Holds OpCodes Counts for Acir and Brillig Opcodes @@ -29,8 +30,11 @@ pub struct OpCodesCount { } impl DebugInfo { - pub fn new(locations: BTreeMap>) -> Self { - DebugInfo { locations } + pub fn new( + locations: BTreeMap>, + variables: HashMap, + ) -> Self { + Self { locations, variables } } /// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified. @@ -85,4 +89,14 @@ impl DebugInfo { counted_opcodes } + + pub fn get_file_ids(&self) -> Vec { + self + .locations + .values() + .filter_map(|call_stack| { + call_stack.last().map(|location| location.file) + }) + .collect() + } } diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 24521b98bd1..f369121d2bd 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -126,7 +126,8 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations); + let variables = context.debug_state.get_variables(); + let mut debug_info = DebugInfo::new(locations, variables); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs new file mode 100644 index 00000000000..fc83d110e14 --- /dev/null +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -0,0 +1,202 @@ +use std::collections::HashMap; +use crate::parser::{ParsedModule,parse_program}; +use crate::{ast, parser::{Item,ItemKind}, ast::{Path,PathKind}}; +use noirc_errors::{Span, Spanned}; +use std::collections::VecDeque; + +#[derive(Debug,Clone)] +pub struct DebugState { + var_id_to_name: HashMap, + var_name_to_id: HashMap, + next_var_id: u32, + pub enabled: bool, +} + +impl Default for DebugState { + fn default() -> Self { + Self { + var_id_to_name: HashMap::new(), + var_name_to_id: HashMap::new(), + next_var_id: 0, + enabled: true, // TODO + } + } +} + +impl DebugState { + pub fn new(vars: HashMap) -> Self { + let mut debug_state = Self::default(); + for (var_name, var_id) in vars.iter() { + debug_state.var_id_to_name.insert(*var_id, var_name.clone()); + debug_state.var_name_to_id.insert(var_name.clone(), *var_id); + debug_state.next_var_id = debug_state.next_var_id.max(var_id+1); + } + debug_state + } + + pub fn get_variables(&self) -> HashMap { + self.var_name_to_id.clone() + } + + fn insert_var(&mut self, var_name: &str) -> u32 { + let var_id = self.next_var_id; + self.next_var_id += 1; + self.var_id_to_name.insert(var_id, var_name.to_string()); + self.var_name_to_id.insert(var_name.to_string(), var_id); + var_id + } + + pub fn insert_symbols(&mut self, module: &mut ParsedModule) { + if !self.enabled { return } + self.insert_state_set_oracle(module); + + module.items.iter_mut().for_each(|item| { + match item { + Item { kind: ItemKind::Function(f), .. } => { + // todo: f.def.parameters + f.def.body.0.iter_mut().for_each(|stmt| self.walk_statement(stmt)); + }, + _ => {}, + } + }); + } + + fn wrap_var_expr(&mut self, var_name: &str, expr: ast::Expression) -> ast::Expression { + let var_id = self.insert_var(var_name); + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident("__debug_state_set")], + kind: PathKind::Plain + }), + span: Span::single_char(0), + }), + arguments: vec![ + ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Integer( + (var_id as u128).into() + )), + span: Span::single_char(0), + }, + expr + ], + })); + ast::Expression { kind, span: Span::single_char(0) } + } + + fn wrap_let_statement(&mut self, let_stmt: &ast::LetStatement) -> ast::Statement { + // rewrites let statements written like this: + // let (((a,b,c),D { d }),e,f) = x; + // + // into statements like this: + // + // let (a,b,c,d,e,f,g) = { + // let (((a,b,c),D { d }),e,f) = x; + // wrap(a); + // wrap(b); + // ... + // wrap(f); + // (a,b,c,d,e,f,g) + // }; + + let vars = pattern_vars(&let_stmt.pattern); + let vars_pattern: Vec = vars.iter().map(|id| { + ast::Pattern::Identifier(id.clone()) + }).collect(); + let vars_exprs: Vec = vars.iter().map(|id| id_expr(id)).collect(); + + let mut block_stmts = vec![ + ast::Statement { + kind: ast::StatementKind::Let(let_stmt.clone()), + span: Span::single_char(0), + }, + ]; + block_stmts.extend(vars.iter().map(|id| { + let var_name = &id.0.contents; + ast::Statement { + kind: ast::StatementKind::Semi(self.wrap_var_expr(var_name, id_expr(id))), + span: Span::single_char(0), + } + })); + block_stmts.push(ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Tuple(vars_exprs), + span: Span::single_char(0), + }), + span: Span::single_char(0), + }); + + ast::Statement { + kind: ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Tuple(vars_pattern, Span::single_char(0)), + r#type: ast::UnresolvedType::unspecified(), + expression: ast::Expression { + kind: ast::ExpressionKind::Block(ast::BlockExpression(block_stmts)), + span: Span::single_char(0), + }, + }), + span: Span::single_char(0), + } + } + + fn walk_statement(&mut self, stmt: &mut ast::Statement) { + match &mut stmt.kind { + ast::StatementKind::Let(let_stmt) => { + *stmt = self.wrap_let_statement(&let_stmt); + }, + _ => {}, + } + } + + fn insert_state_set_oracle(&self, module: &mut ParsedModule) { + let (program, errors) = parse_program(r#" + #[oracle(__debug_state_set)] + unconstrained fn __debug_state_set_oracle(_var_id: u32, _input: T) {} + + unconstrained pub fn __debug_state_set(var_id: u32, value: T) -> T { + __debug_state_set_oracle(var_id, value); + value + } + "#); + if !errors.is_empty() { panic!("errors parsing internal oracle definitions: {errors:?}") } + module.items.extend(program.items); + } +} + +fn pattern_vars(pattern: &ast::Pattern) -> Vec { + let mut vars = vec![]; + let mut stack = VecDeque::from([ pattern ]); + while stack.front().is_some() { + let pattern = stack.pop_front().unwrap(); + match pattern { + ast::Pattern::Identifier(id) => { + vars.push(id.clone()); + }, + ast::Pattern::Mutable(pattern, _) => { + stack.push_back(pattern); + }, + ast::Pattern::Tuple(patterns, _) => { + stack.extend(patterns.iter()); + }, + ast::Pattern::Struct(_, pids, _) => { + stack.extend(pids.iter().map(|(_, pattern)| pattern)); + vars.extend(pids.iter().map(|(id, _)| id.clone())); + }, + } + } + vars +} + +fn ident(s: &str) -> ast::Ident { + ast::Ident(Spanned::from(Span::single_char(0), s.to_string())) +} + +fn id_expr(id: &ast::Ident) -> ast::Expression { + ast::Expression { + kind: ast::ExpressionKind::Variable(Path { + segments: vec![id.clone()], + kind: PathKind::Plain, + }), + span: Span::single_char(0), + } +} 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 17aa5e9951f..197238cba2d 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -20,7 +20,7 @@ use super::{ }, errors::{DefCollectorErrorKind, DuplicateType}, }; -use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleId}; +use crate::hir::def_map::{LocalModuleId, ModuleData, ModuleId}; use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; @@ -539,8 +539,7 @@ impl<'a> ModCollector<'a> { context.visited_files.insert(child_file_id, location); // Parse the AST for the module we just found and then recursively look for it's defs - let (ast, parsing_errors) = parse_file(&context.file_manager, child_file_id); - let ast = ast.into_sorted(); + let (ast, parsing_errors) = context.parse_file(child_file_id, crate_id); errors.extend( parsing_errors.iter().map(|e| (e.clone().into(), child_file_id)).collect::>(), diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 345e5447bf5..9093dfb97a5 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -81,9 +81,8 @@ impl CrateDefMap { } // First parse the root file. - let root_file_id = context.crate_graph[crate_id].root_file_id; - let (ast, parsing_errors) = parse_file(&context.file_manager, root_file_id); - let ast = ast.into_sorted(); + let root_file_id = context.get_root_id(crate_id); + let (ast, parsing_errors) = context.parse_file(root_file_id, crate_id); #[cfg(feature = "aztec")] let ast = match super::aztec_library::transform(ast, &crate_id, context) { @@ -257,7 +256,7 @@ pub struct Contract { pub events: Vec, } -/// Given a FileId, fetch the File, from the FileManager and parse it's content +/// Given a FileId, fetch the File, from the FileManager and parse it's content. pub fn parse_file(fm: &FileManager, file_id: FileId) -> (ParsedModule, Vec) { let file = fm.fetch_file(file_id); parse_program(file.source()) diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index 5a28d7b779a..a32c3b9bf58 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -9,9 +9,11 @@ pub(crate) mod aztec_library; use crate::graph::{CrateGraph, CrateId}; use crate::hir_def::function::FuncMeta; +use crate::debug::DebugState; use crate::node_interner::{FuncId, NodeInterner, StructId}; -use def_map::{Contract, CrateDefMap}; -use fm::FileManager; +use crate::parser::{SortedModule, ParserError}; +use def_map::{Contract, CrateDefMap, parse_file}; +use fm::{FileManager, FileId}; use noirc_errors::Location; use std::collections::BTreeMap; @@ -25,6 +27,8 @@ pub struct Context { pub crate_graph: CrateGraph, pub(crate) def_maps: BTreeMap, pub file_manager: FileManager, + pub root_crate_id: CrateId, + pub debug_state: DebugState, /// A map of each file that already has been visited from a prior `mod foo;` declaration. /// This is used to issue an error if a second `mod foo;` is declared to the same file. @@ -53,6 +57,8 @@ impl Context { crate_graph, file_manager, storage_slots: BTreeMap::new(), + root_crate_id: CrateId::Dummy, + debug_state: DebugState::default(), } } @@ -212,4 +218,19 @@ impl Context { *next_slot }) } + + /// Given a FileId, fetch the File, from the FileManager and parse its content, + /// applying sorting and debug transforms if debug mode is enabled. + pub fn parse_file(&mut self, file_id: FileId, crate_id: CrateId) -> (SortedModule, Vec) { + let (mut ast, parsing_errors) = parse_file(&self.file_manager, file_id); + + if crate_id == self.root_crate_id { + self.debug_state.insert_symbols(&mut ast); + } + (ast.into_sorted(), parsing_errors) + } + + pub fn get_root_id(&self, crate_id: CrateId) -> FileId { + self.crate_graph[crate_id].root_file_id + } } diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 6f3140a65d4..d296ebf3b6a 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -35,11 +35,13 @@ pub struct ResolvedImport { impl From for CustomDiagnostic { fn from(error: PathResolutionError) -> Self { match error { - PathResolutionError::Unresolved(ident) => CustomDiagnostic::simple_error( - format!("Could not resolve '{ident}' in path"), - String::new(), - ident.span(), - ), + PathResolutionError::Unresolved(ident) => { + CustomDiagnostic::simple_error( + format!("Could not resolve '{ident}' in path"), + String::new(), + ident.span(), + ) + } PathResolutionError::ExternalContractUsed(ident) => CustomDiagnostic::simple_error( format!("Contract variable '{ident}' referenced from outside the contract"), "Contracts may only be referenced from within a contract".to_string(), diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 74057240de1..324df78ab1d 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -11,6 +11,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] pub mod ast; +pub mod debug; pub mod graph; pub mod lexer; pub mod monomorphization; diff --git a/tooling/debugger/Cargo.toml b/tooling/debugger/Cargo.toml index 53c71754da4..c815c5a6b39 100644 --- a/tooling/debugger/Cargo.toml +++ b/tooling/debugger/Cargo.toml @@ -10,7 +10,9 @@ license.workspace = true [dependencies] acvm.workspace = true +fm.workspace = true nargo.workspace = true +noirc_frontend.workspace = true noirc_printable_type.workspace = true noirc_errors.workspace = true thiserror.workspace = true diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 33862ed641f..1b196984b74 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -6,7 +6,7 @@ use acvm::pwg::{ }; use acvm::{BlackBoxFunctionSolver, FieldElement}; -use nargo::artifacts::debug::DebugArtifact; +use nargo::artifacts::debug::{DebugArtifact, DebugVars}; use nargo::errors::{ExecutionError, Location}; use nargo::ops::{DefaultForeignCallExecutor, ForeignCallExecutor}; use nargo::NargoError; @@ -26,6 +26,7 @@ pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> { brillig_solver: Option>, foreign_call_executor: DefaultForeignCallExecutor, debug_artifact: &'a DebugArtifact, + pub debug_vars: DebugVars, breakpoints: HashSet, } @@ -36,11 +37,17 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, ) -> Self { + // TODO: move this into the other context constructor + let mut debug_vars = DebugVars::default(); + debug_artifact.debug_symbols.iter().for_each(|info| { + debug_vars.insert_variables(&info.variables); + }); Self { acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), brillig_solver: None, foreign_call_executor: DefaultForeignCallExecutor::new(true), debug_artifact, + debug_vars, breakpoints: HashSet::new(), } } @@ -117,7 +124,10 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } fn handle_foreign_call(&mut self, foreign_call: ForeignCallWaitInfo) -> DebugCommandResult { - let foreign_call_result = self.foreign_call_executor.execute(&foreign_call); + let foreign_call_result = self.foreign_call_executor.execute_with_debug_vars( + &foreign_call, + &mut self.debug_vars, + ); match foreign_call_result { Ok(foreign_call_result) => { self.acvm.resolve_pending_foreign_call(foreign_call_result); diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 23abe83c38d..c8ddcf67889 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -9,6 +9,7 @@ use nargo::NargoError; use easy_repl::{command, CommandStatus, Repl}; use std::cell::RefCell; +use std::collections::HashMap; use codespan_reporting::files::Files; use noirc_errors::Location; @@ -550,6 +551,20 @@ pub fn run( } }, ) + .add( + "vars", + command! { + "show variable values available at this point in execution", + () => || { + let mut ctx = ref_context.borrow_mut(); + let vars = ctx.context.debug_vars.get_values(); + println!("{:?}", vars.iter().map(|(var_name,value)| { + (var_name, value.to_field()) + }).collect::>()); + Ok(CommandStatus::Done) + } + }, + ) .build() .expect("Failed to initialize debugger repl"); diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 40acc7db8f8..f6836e95075 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -4,11 +4,12 @@ use noirc_errors::{debug_info::DebugInfo, Location}; use noirc_evaluator::errors::SsaReport; use serde::{Deserialize, Serialize}; use std::{ - collections::{BTreeMap, BTreeSet}, + collections::BTreeMap, ops::Range, }; use fm::{FileId, FileManager, PathString}; +pub use super::debug_vars::DebugVars; /// A Debug Artifact stores, for a given program, the debug info for every function /// along with a map of file Id to the source code so locations in debug info can be mapped to source code they point to. @@ -23,24 +24,19 @@ impl DebugArtifact { pub fn new(debug_symbols: Vec, file_manager: &FileManager) -> Self { let mut file_map = BTreeMap::new(); - let files_with_debug_symbols: BTreeSet = debug_symbols + let file_ids: Vec = debug_symbols .iter() - .flat_map(|function_symbols| { - function_symbols - .locations - .values() - .flat_map(|call_stack| call_stack.iter().map(|location| location.file)) - }) + .flat_map(|debug_info| debug_info.get_file_ids()) .collect(); - for file_id in files_with_debug_symbols { - let file_source = file_manager.fetch_file(file_id).source(); + for file_id in file_ids.iter() { + let file_source = file_manager.fetch_file(*file_id).source(); file_map.insert( - file_id, + *file_id, DebugFile { source: file_source.to_string(), - path: file_manager.path(file_id).to_path_buf(), + path: file_manager.path(*file_id).to_path_buf(), }, ); } @@ -94,6 +90,14 @@ impl DebugArtifact { let source = self.source(location.file)?; self.line_index(location.file, source.len()) } + + pub fn from_program(program: &CompiledProgram) -> Self { + Self { + debug_symbols: vec![program.debug.clone()], + file_map: program.file_map.clone(), + warnings: program.warnings.clone(), + } + } } impl From for DebugArtifact { diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs new file mode 100644 index 00000000000..133ad26e3b5 --- /dev/null +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -0,0 +1,45 @@ +use std::collections::HashMap; +use acvm::acir::brillig::Value; + +#[derive(Debug, Default, Clone)] +pub struct DebugVars { + id_to_name: HashMap, + name_to_id: HashMap, + id_to_value: HashMap, // TODO: something more sophisticated for lexical levels +} + +impl DebugVars { + pub fn new(vars: &HashMap) -> Self { + let mut debug_vars = Self::default(); + debug_vars.id_to_name = vars.iter().map(|(name,id)| (*id, name.clone())).collect(); + debug_vars.name_to_id = vars.clone(); + debug_vars + } + + pub fn get_values<'a>(&'a self) -> HashMap { + self.id_to_value.iter().filter_map(|(var_id,value)| { + self.id_to_name.get(var_id).map(|name| { + (name.clone(),value) + }) + }).collect() + } + + pub fn insert_variables(&mut self, vars: &HashMap) { + vars.iter().for_each(|(var_name,var_id)| { + self.id_to_name.insert(*var_id, var_name.clone()); + self.name_to_id.insert(var_name.clone(), *var_id); + }); + } + + pub fn set_by_id(&mut self, var_id: u32, value: Value) { + self.id_to_value.insert(var_id, value); + } + + pub fn get_by_id<'a>(&'a mut self, var_id: u32) -> Option<&'a Value> { + self.id_to_value.get(&var_id) + } + + pub fn get_by_name<'a>(&'a mut self, var_name: &str) -> Option<&'a Value> { + self.name_to_id.get(var_name).and_then(|var_id| self.id_to_value.get(var_id)) + } +} diff --git a/tooling/nargo/src/artifacts/mod.rs b/tooling/nargo/src/artifacts/mod.rs index 180a900fd81..2d8e1fcef39 100644 --- a/tooling/nargo/src/artifacts/mod.rs +++ b/tooling/nargo/src/artifacts/mod.rs @@ -6,3 +6,4 @@ pub mod contract; pub mod debug; pub mod program; +mod debug_vars; diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index ef014fb436b..b15fd0f53bf 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -49,6 +49,7 @@ pub fn prepare_package(package: &Package, file_reader: Box) -> (Cont let mut context = Context::new(fm, graph); let crate_id = prepare_crate(&mut context, &package.entry_path); + context.root_crate_id = crate_id.clone(); prepare_dependencies(&mut context, crate_id, &package.dependencies); diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 6cc78febab3..66757062550 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -4,6 +4,7 @@ use acvm::{ }; use iter_extended::vecmap; use noirc_printable_type::{decode_string_value, ForeignCallError, PrintableValueDisplay}; +use crate::artifacts::debug::DebugVars; pub trait ForeignCallExecutor { fn execute( @@ -16,6 +17,7 @@ pub trait ForeignCallExecutor { /// After resolution of a foreign call, nargo will restart execution of the ACVM pub(crate) enum ForeignCall { Println, + DebugStateSet, Sequence, ReverseSequence, CreateMock, @@ -35,6 +37,7 @@ impl ForeignCall { pub(crate) fn name(&self) -> &'static str { match self { ForeignCall::Println => "println", + ForeignCall::DebugStateSet => "__debug_state_set", ForeignCall::Sequence => "get_number_sequence", ForeignCall::ReverseSequence => "get_reverse_number_sequence", ForeignCall::CreateMock => "create_mock", @@ -48,6 +51,7 @@ impl ForeignCall { pub(crate) fn lookup(op_name: &str) -> Option { match op_name { "println" => Some(ForeignCall::Println), + "__debug_state_set" => Some(ForeignCall::DebugStateSet), "get_number_sequence" => Some(ForeignCall::Sequence), "get_reverse_number_sequence" => Some(ForeignCall::ReverseSequence), "create_mock" => Some(ForeignCall::CreateMock), @@ -138,6 +142,22 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { fn execute( &mut self, foreign_call: &ForeignCallWaitInfo, + ) -> Result { + self.execute_optional_debug_vars(foreign_call, None) + } + + pub fn execute_with_debug_vars( + &mut self, + foreign_call: &ForeignCallWaitInfo, + debug_vars: &mut DebugVars, + ) -> Result { + self.execute_optional_debug_vars(foreign_call, Some(debug_vars)) + } + + fn execute_optional_debug_vars( + &mut self, + foreign_call: &ForeignCallWaitInfo, + debug_vars: Option<&mut DebugVars>, ) -> Result { let foreign_call_name = foreign_call.function.as_str(); match ForeignCall::lookup(foreign_call_name) { @@ -147,6 +167,20 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { } Ok(ForeignCallResult { values: vec![] }) } + Some(ForeignCall::DebugStateSet) => { + let fcp_var_id = &foreign_call.inputs[0]; + let fcp_value = &foreign_call.inputs[1]; + if let ( + Some(ds), + ForeignCallParam::Single(var_id_value), + ForeignCallParam::Single(value), + ) = (debug_vars, fcp_var_id, fcp_value) { + let var_id = var_id_value.to_u128() as u32; + ds.set_by_id(var_id, value.clone()); + } + // pass-through return value is handled by the oracle wrapper, returning nothing here + Ok(ForeignCallResult { values: vec![] }) + } Some(ForeignCall::Sequence) => { let sequence_length: u128 = foreign_call.inputs[0].unwrap_value().to_field().to_u128(); diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 9ffbc26828e..92bae8803c2 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -258,6 +258,7 @@ fn compile_contract( } fn save_program(program: CompiledProgram, package: &Package, circuit_dir: &Path) { + let debug_artifact = DebugArtifact::from_program(&program); let preprocessed_program = PreprocessedProgram { hash: program.hash, backend: String::from(BACKEND_IDENTIFIER), @@ -265,15 +266,9 @@ fn save_program(program: CompiledProgram, package: &Package, circuit_dir: &Path) noir_version: program.noir_version, bytecode: program.circuit, }; + let circuit_name: String = (&package.name).into(); save_program_to_file(&preprocessed_program, &package.name, circuit_dir); - - let debug_artifact = DebugArtifact { - debug_symbols: vec![program.debug], - file_map: program.file_map, - warnings: program.warnings, - }; - let circuit_name: String = (&package.name).into(); save_debug_artifact_to_file(&debug_artifact, &circuit_name, circuit_dir); } From 705e1be30a22983ec890598467031b51317484f8 Mon Sep 17 00:00:00 2001 From: synthia Date: Thu, 2 Nov 2023 08:36:06 -0700 Subject: [PATCH 02/11] move fild_ids back to a BTreeSet to get deduplication --- tooling/nargo/src/artifacts/debug.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index f6836e95075..e95e721d388 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -4,7 +4,7 @@ use noirc_errors::{debug_info::DebugInfo, Location}; use noirc_evaluator::errors::SsaReport; use serde::{Deserialize, Serialize}; use std::{ - collections::BTreeMap, + collections::{BTreeMap,BTreeSet}, ops::Range, }; @@ -24,7 +24,7 @@ impl DebugArtifact { pub fn new(debug_symbols: Vec, file_manager: &FileManager) -> Self { let mut file_map = BTreeMap::new(); - let file_ids: Vec = debug_symbols + let file_ids: BTreeSet = debug_symbols .iter() .flat_map(|debug_info| debug_info.get_file_ids()) .collect(); From 953a89a433574a19c3c022146f661574e68178e0 Mon Sep 17 00:00:00 2001 From: synthia Date: Thu, 2 Nov 2023 12:04:10 -0700 Subject: [PATCH 03/11] constrained debug function with an extra wrapper --- compiler/noirc_frontend/src/debug/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index fc83d110e14..0362accc6ff 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -61,7 +61,7 @@ impl DebugState { }); } - fn wrap_var_expr(&mut self, var_name: &str, expr: ast::Expression) -> ast::Expression { + fn wrap_var(&mut self, var_name: &str, expr: ast::Expression) -> ast::Expression { let var_id = self.insert_var(var_name); let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { @@ -114,7 +114,7 @@ impl DebugState { block_stmts.extend(vars.iter().map(|id| { let var_name = &id.0.contents; ast::Statement { - kind: ast::StatementKind::Semi(self.wrap_var_expr(var_name, id_expr(id))), + kind: ast::StatementKind::Semi(self.wrap_var(var_name, id_expr(id))), span: Span::single_char(0), } })); @@ -153,9 +153,12 @@ impl DebugState { #[oracle(__debug_state_set)] unconstrained fn __debug_state_set_oracle(_var_id: u32, _input: T) {} - unconstrained pub fn __debug_state_set(var_id: u32, value: T) -> T { + unconstrained fn __debug_state_set_inner(var_id: u32, value: T) { __debug_state_set_oracle(var_id, value); - value + } + + pub fn __debug_state_set(var_id: u32, value: T) { + __debug_state_set_inner(var_id, value); } "#); if !errors.is_empty() { panic!("errors parsing internal oracle definitions: {errors:?}") } From 966bb88a2b6baf353abfcc66a8ef2dae47eb4c6e Mon Sep 17 00:00:00 2001 From: synthia Date: Fri, 3 Nov 2023 12:28:17 -0700 Subject: [PATCH 04/11] pass through let statement span and set the other injected spans to empty --- compiler/noirc_errors/src/position.rs | 4 +++ compiler/noirc_frontend/src/debug/mod.rs | 42 +++++++++++++----------- tooling/debugger/src/repl.rs | 5 ++- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_errors/src/position.rs b/compiler/noirc_errors/src/position.rs index e308eb9a2c7..2a0bccc8cda 100644 --- a/compiler/noirc_errors/src/position.rs +++ b/compiler/noirc_errors/src/position.rs @@ -81,6 +81,10 @@ impl Span { pub fn end(&self) -> u32 { self.0.end().into() } + + pub fn from_str(s: &str) -> Span { + Span(ByteSpan::from_str(s)) + } } impl From for Range { diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 0362accc6ff..6d04dfbd768 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -61,30 +61,30 @@ impl DebugState { }); } - fn wrap_var(&mut self, var_name: &str, expr: ast::Expression) -> ast::Expression { + fn wrap_var(&mut self, var_name: &str, expr: ast::Expression, span: &Span) -> ast::Expression { let var_id = self.insert_var(var_name); let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_state_set")], + segments: vec![ident("__debug_state_set", span)], kind: PathKind::Plain }), - span: Span::single_char(0), + span: none_span(), }), arguments: vec![ ast::Expression { kind: ast::ExpressionKind::Literal(ast::Literal::Integer( (var_id as u128).into() )), - span: Span::single_char(0), + span: none_span(), }, expr ], })); - ast::Expression { kind, span: Span::single_char(0) } + ast::Expression { kind, span: none_span() } } - fn wrap_let_statement(&mut self, let_stmt: &ast::LetStatement) -> ast::Statement { + fn wrap_let_statement(&mut self, let_stmt: &ast::LetStatement, span: &Span) -> ast::Statement { // rewrites let statements written like this: // let (((a,b,c),D { d }),e,f) = x; // @@ -103,46 +103,46 @@ impl DebugState { let vars_pattern: Vec = vars.iter().map(|id| { ast::Pattern::Identifier(id.clone()) }).collect(); - let vars_exprs: Vec = vars.iter().map(|id| id_expr(id)).collect(); + let vars_exprs: Vec = vars.iter().map(|id| id_expr(id, span)).collect(); let mut block_stmts = vec![ ast::Statement { kind: ast::StatementKind::Let(let_stmt.clone()), - span: Span::single_char(0), + span: none_span(), }, ]; block_stmts.extend(vars.iter().map(|id| { let var_name = &id.0.contents; ast::Statement { - kind: ast::StatementKind::Semi(self.wrap_var(var_name, id_expr(id))), - span: Span::single_char(0), + kind: ast::StatementKind::Semi(self.wrap_var(var_name, id_expr(id, span), span)), + span: none_span(), } })); block_stmts.push(ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { kind: ast::ExpressionKind::Tuple(vars_exprs), - span: Span::single_char(0), + span: none_span(), }), - span: Span::single_char(0), + span: none_span(), }); ast::Statement { kind: ast::StatementKind::Let(ast::LetStatement { - pattern: ast::Pattern::Tuple(vars_pattern, Span::single_char(0)), + pattern: ast::Pattern::Tuple(vars_pattern, none_span()), r#type: ast::UnresolvedType::unspecified(), expression: ast::Expression { kind: ast::ExpressionKind::Block(ast::BlockExpression(block_stmts)), - span: Span::single_char(0), + span: none_span(), }, }), - span: Span::single_char(0), + span: span.clone(), } } fn walk_statement(&mut self, stmt: &mut ast::Statement) { match &mut stmt.kind { ast::StatementKind::Let(let_stmt) => { - *stmt = self.wrap_let_statement(&let_stmt); + *stmt = self.wrap_let_statement(&let_stmt, &stmt.span); }, _ => {}, } @@ -190,16 +190,18 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec { vars } -fn ident(s: &str) -> ast::Ident { - ast::Ident(Spanned::from(Span::single_char(0), s.to_string())) +fn ident(s: &str, span: &Span) -> ast::Ident { + ast::Ident(Spanned::from(none_span(), s.to_string())) } -fn id_expr(id: &ast::Ident) -> ast::Expression { +fn id_expr(id: &ast::Ident, span: &Span) -> ast::Expression { ast::Expression { kind: ast::ExpressionKind::Variable(Path { segments: vec![id.clone()], kind: PathKind::Plain, }), - span: Span::single_char(0), + span: none_span(), } } + +fn none_span() -> Span { Span::from_str("") } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index c8ddcf67889..c62261393a6 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -86,6 +86,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { let locations = self.debug_artifact.debug_symbols[0].opcode_location(location); let Some(locations) = locations else { return }; for loc in locations { + if loc.span.start() == loc.span.end() { continue } self.print_location_path(loc); let loc_line_index = self.debug_artifact.location_line_index(loc).unwrap(); @@ -121,8 +122,10 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { if current_line_index == loc_line_index { // Highlight current location - let Range { start: loc_start, end: loc_end } = + let Range { start: loc_start, end: mut loc_end } = self.debug_artifact.location_in_line(loc).unwrap(); + loc_end = loc_end.min(line.len()); + println!["loc_start={loc_start}, loc_end={loc_end}, loc={loc:?}"]; println!( "{:>3} {:2} {}{}{}", current_line_number, From addd08ffafcfdf42fb91a0cd00bebfa1d8f893f7 Mon Sep 17 00:00:00 2001 From: synthia Date: Tue, 7 Nov 2023 11:44:03 -0800 Subject: [PATCH 05/11] assign variables by integer, fn params, and implement drops for fn scope --- compiler/noirc_errors/src/debug_info.rs | 4 +- compiler/noirc_frontend/src/debug/mod.rs | 209 +++++++++++++++++++--- tooling/nargo/src/artifacts/debug_vars.rs | 35 ++-- tooling/nargo/src/ops/foreign_calls.rs | 17 +- 4 files changed, 216 insertions(+), 49 deletions(-) diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 706f6962208..a3e10e0e7de 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -18,7 +18,7 @@ pub struct DebugInfo { /// that they should be serialized to/from strings. #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, - pub variables: HashMap, + pub variables: HashMap, } /// Holds OpCodes Counts for Acir and Brillig Opcodes @@ -32,7 +32,7 @@ pub struct OpCodesCount { impl DebugInfo { pub fn new( locations: BTreeMap>, - variables: HashMap, + variables: HashMap, ) -> Self { Self { locations, variables } } diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 6d04dfbd768..c6872ac3ed8 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap,HashSet}; use crate::parser::{ParsedModule,parse_program}; use crate::{ast, parser::{Item,ItemKind}, ast::{Path,PathKind}}; use noirc_errors::{Span, Spanned}; @@ -7,16 +7,16 @@ use std::collections::VecDeque; #[derive(Debug,Clone)] pub struct DebugState { var_id_to_name: HashMap, - var_name_to_id: HashMap, next_var_id: u32, + active_vars: HashSet, pub enabled: bool, } impl Default for DebugState { fn default() -> Self { Self { - var_id_to_name: HashMap::new(), - var_name_to_id: HashMap::new(), + var_id_to_name: HashMap::default(), + active_vars: HashSet::default(), next_var_id: 0, enabled: true, // TODO } @@ -28,45 +28,120 @@ impl DebugState { let mut debug_state = Self::default(); for (var_name, var_id) in vars.iter() { debug_state.var_id_to_name.insert(*var_id, var_name.clone()); - debug_state.var_name_to_id.insert(var_name.clone(), *var_id); debug_state.next_var_id = debug_state.next_var_id.max(var_id+1); } debug_state } - pub fn get_variables(&self) -> HashMap { - self.var_name_to_id.clone() + pub fn get_variables(&self) -> HashMap { + self.var_id_to_name.clone() } fn insert_var(&mut self, var_name: &str) -> u32 { let var_id = self.next_var_id; self.next_var_id += 1; self.var_id_to_name.insert(var_id, var_name.to_string()); - self.var_name_to_id.insert(var_name.to_string(), var_id); var_id } + fn walk_fn(&mut self, f: &mut ast::FunctionDefinition) { + let pvars: Vec<(u32,ast::Ident)> = f.parameters.iter() + .flat_map(|(pattern, _utype, _vis)| { + pattern_vars(pattern).iter().map(|id| { + (self.insert_var(&id.0.contents), id.clone()) + }).collect::>() + }) + .collect(); + + f.body.0.iter_mut().for_each(|stmt| self.walk_statement(stmt)); + + let (ret_stmt, fn_body) = f.body.0.split_last() + .map(|(e,b)| (e.clone(), b.to_vec())) + .unwrap_or(( + ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Unit), + span: none_span(), + }), + span: none_span(), + }, + vec![] + )); + + f.body.0 = vec![ + // set fn params: + pvars.iter().map(|(var_id, id)| { + self.wrap_set_var(*var_id, id_expr(id)) + }).collect(), + + // copy body minus the return expr: + fn_body, + + // assign return expr to __debug_state_return: + vec![match &ret_stmt.kind { + ast::StatementKind::Expression(ret_expr) => { + ast::Statement { + kind: ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Identifier(ident("__debug_state_return")), + r#type: ast::UnresolvedType::unspecified(), + expression: ret_expr.clone(), + }), + span: none_span(), + } + }, + _ => ret_stmt.clone(), + }], + + // drop fn params: + pvars.iter().map(|(var_id, _id)| { + self.wrap_drop_var(*var_id) + }).collect(), + + // return the __debug_state_return value: + vec![match &ret_stmt.kind { + ast::StatementKind::Expression(ret_expr) => { + ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident("__debug_state_return")], + kind: PathKind::Plain, + }), + span: none_span(), + }), + span: none_span(), + } + }, + _ => ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Unit), + span: none_span(), + }), + span: none_span(), + }, + }], + ].concat(); + } + pub fn insert_symbols(&mut self, module: &mut ParsedModule) { if !self.enabled { return } - self.insert_state_set_oracle(module); - module.items.iter_mut().for_each(|item| { match item { Item { kind: ItemKind::Function(f), .. } => { - // todo: f.def.parameters - f.def.body.0.iter_mut().for_each(|stmt| self.walk_statement(stmt)); + self.walk_fn(&mut f.def); }, _ => {}, } }); + // this part absolutely must happen after ast traversal above + // so that oracle functions don't get wrapped, resulting in infinite recursion: + self.insert_state_set_oracle(module); } - fn wrap_var(&mut self, var_name: &str, expr: ast::Expression, span: &Span) -> ast::Expression { - let var_id = self.insert_var(var_name); + fn wrap_set_var(&mut self, var_id: u32, expr: ast::Expression) -> 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("__debug_state_set", span)], + segments: vec![ident("__debug_state_set")], kind: PathKind::Plain }), span: none_span(), @@ -78,10 +153,37 @@ impl DebugState { )), span: none_span(), }, - expr + expr, ], })); - ast::Expression { kind, span: none_span() } + ast::Statement { + kind: ast::StatementKind::Semi(ast::Expression { kind, span: none_span() }), + span: none_span(), + } + } + + fn wrap_drop_var(&mut self, var_id: u32) -> 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("__debug_state_drop")], + kind: PathKind::Plain + }), + span: none_span(), + }), + arguments: vec![ + ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Integer( + (var_id as u128).into() + )), + span: none_span(), + }, + ], + })); + ast::Statement { + kind: ast::StatementKind::Semi(ast::Expression { kind, span: none_span() }), + span: none_span(), + } } fn wrap_let_statement(&mut self, let_stmt: &ast::LetStatement, span: &Span) -> ast::Statement { @@ -92,10 +194,10 @@ impl DebugState { // // let (a,b,c,d,e,f,g) = { // let (((a,b,c),D { d }),e,f) = x; - // wrap(a); - // wrap(b); + // wrap(1, a); + // wrap(2, b); // ... - // wrap(f); + // wrap(6, f); // (a,b,c,d,e,f,g) // }; @@ -103,7 +205,7 @@ impl DebugState { let vars_pattern: Vec = vars.iter().map(|id| { ast::Pattern::Identifier(id.clone()) }).collect(); - let vars_exprs: Vec = vars.iter().map(|id| id_expr(id, span)).collect(); + let vars_exprs: Vec = vars.iter().map(|id| id_expr(id)).collect(); let mut block_stmts = vec![ ast::Statement { @@ -112,11 +214,8 @@ impl DebugState { }, ]; block_stmts.extend(vars.iter().map(|id| { - let var_name = &id.0.contents; - ast::Statement { - kind: ast::StatementKind::Semi(self.wrap_var(var_name, id_expr(id, span), span)), - span: none_span(), - } + let var_id = self.insert_var(&id.0.contents); + self.wrap_set_var(var_id, id_expr(id)) })); block_stmts.push(ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { @@ -139,12 +238,53 @@ impl DebugState { } } + fn walk_expr(&mut self, expr: &mut ast::Expression) { + match &expr.kind { + ast::ExpressionKind::Block(_block_expr) => { + // TODO: set then drop vars in this scope + }, + _ => {}, + } + } + + fn walk_for(&mut self, for_stmt: &mut ast::ForLoopStatement) { + let var_name = &for_stmt.identifier.0.contents; + let var_id = self.insert_var(var_name); + + let set_stmt = self.wrap_set_var(var_id, id_expr(&for_stmt.identifier)); + let drop_stmt = self.wrap_drop_var(var_id); + + for_stmt.block = ast::Expression { + kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ + set_stmt, + ast::Statement { + kind: ast::StatementKind::Semi(for_stmt.block.clone()), + span: none_span(), + }, + drop_stmt, + ])), + span: none_span(), + }; + } + fn walk_statement(&mut self, stmt: &mut ast::Statement) { match &mut stmt.kind { ast::StatementKind::Let(let_stmt) => { *stmt = self.wrap_let_statement(&let_stmt, &stmt.span); }, - _ => {}, + ast::StatementKind::Expression(expr) => { + self.walk_expr(expr); + }, + ast::StatementKind::Semi(expr) => { + self.walk_expr(expr); + }, + ast::StatementKind::For(ref mut for_stmt) => { + self.walk_for(for_stmt); + }, + ast::StatementKind::Assign(_assign_stmt) => { + // TODO + }, + _ => {}, // Constrain, Error } } @@ -153,13 +293,24 @@ impl DebugState { #[oracle(__debug_state_set)] unconstrained fn __debug_state_set_oracle(_var_id: u32, _input: T) {} + #[oracle(__debug_state_drop)] + unconstrained fn __debug_state_drop_oracle(_var_id: u32) {} + unconstrained fn __debug_state_set_inner(var_id: u32, value: T) { __debug_state_set_oracle(var_id, value); } + unconstrained fn __debug_state_drop_inner(var_id: u32) { + __debug_state_drop_oracle(var_id); + } + pub fn __debug_state_set(var_id: u32, value: T) { __debug_state_set_inner(var_id, value); } + + pub fn __debug_state_drop(var_id: u32) { + __debug_state_drop_inner(var_id); + } "#); if !errors.is_empty() { panic!("errors parsing internal oracle definitions: {errors:?}") } module.items.extend(program.items); @@ -190,11 +341,11 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec { vars } -fn ident(s: &str, span: &Span) -> ast::Ident { +fn ident(s: &str) -> ast::Ident { ast::Ident(Spanned::from(none_span(), s.to_string())) } -fn id_expr(id: &ast::Ident, span: &Span) -> ast::Expression { +fn id_expr(id: &ast::Ident) -> ast::Expression { ast::Expression { kind: ast::ExpressionKind::Variable(Path { segments: vec![id.clone()], diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 133ad26e3b5..c42d38316f0 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -1,45 +1,48 @@ -use std::collections::HashMap; +use std::collections::{HashMap,HashSet}; use acvm::acir::brillig::Value; #[derive(Debug, Default, Clone)] pub struct DebugVars { id_to_name: HashMap, - name_to_id: HashMap, + active: HashSet, id_to_value: HashMap, // TODO: something more sophisticated for lexical levels } impl DebugVars { - pub fn new(vars: &HashMap) -> Self { + pub fn new(vars: &HashMap) -> Self { let mut debug_vars = Self::default(); - debug_vars.id_to_name = vars.iter().map(|(name,id)| (*id, name.clone())).collect(); - debug_vars.name_to_id = vars.clone(); + debug_vars.id_to_name = vars.clone(); debug_vars } - pub fn get_values<'a>(&'a self) -> HashMap { - self.id_to_value.iter().filter_map(|(var_id,value)| { - self.id_to_name.get(var_id).map(|name| { - (name.clone(),value) + pub fn get_values<'a>(&'a self) -> HashMap<&'a str, &'a Value> { + self.active.iter().filter_map(|var_id| { + self.id_to_name.get(var_id).and_then(|name| { + self.id_to_value.get(var_id).map(|value| (name.as_str(), value)) }) }).collect() } - pub fn insert_variables(&mut self, vars: &HashMap) { - vars.iter().for_each(|(var_name,var_id)| { + pub fn insert_variables(&mut self, vars: &HashMap) { + vars.iter().for_each(|(var_id, var_name)| { self.id_to_name.insert(*var_id, var_name.clone()); - self.name_to_id.insert(var_name.clone(), *var_id); }); } - pub fn set_by_id(&mut self, var_id: u32, value: Value) { + pub fn set(&mut self, var_id: u32, value: Value) { + let name = self.id_to_name.get(&var_id).unwrap(); + println!["\n\n######## SET {name}[{var_id}] = {value:?}\n"]; + self.active.insert(var_id); self.id_to_value.insert(var_id, value); } - pub fn get_by_id<'a>(&'a mut self, var_id: u32) -> Option<&'a Value> { + pub fn get<'a>(&'a mut self, var_id: u32) -> Option<&'a Value> { self.id_to_value.get(&var_id) } - pub fn get_by_name<'a>(&'a mut self, var_name: &str) -> Option<&'a Value> { - self.name_to_id.get(var_name).and_then(|var_id| self.id_to_value.get(var_id)) + pub fn drop(&mut self, var_id: u32) { + let name = self.id_to_name.get(&var_id).unwrap(); + println!["\n\n######## DROP {name}[{var_id}]\n"]; + self.active.remove(&var_id); } } diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 66757062550..ae91ba52df1 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -18,6 +18,7 @@ pub trait ForeignCallExecutor { pub(crate) enum ForeignCall { Println, DebugStateSet, + DebugStateDrop, Sequence, ReverseSequence, CreateMock, @@ -38,6 +39,7 @@ impl ForeignCall { match self { ForeignCall::Println => "println", ForeignCall::DebugStateSet => "__debug_state_set", + ForeignCall::DebugStateDrop => "__debug_state_drop", ForeignCall::Sequence => "get_number_sequence", ForeignCall::ReverseSequence => "get_reverse_number_sequence", ForeignCall::CreateMock => "create_mock", @@ -52,6 +54,7 @@ impl ForeignCall { match op_name { "println" => Some(ForeignCall::Println), "__debug_state_set" => Some(ForeignCall::DebugStateSet), + "__debug_state_drop" => Some(ForeignCall::DebugStateDrop), "get_number_sequence" => Some(ForeignCall::Sequence), "get_reverse_number_sequence" => Some(ForeignCall::ReverseSequence), "create_mock" => Some(ForeignCall::CreateMock), @@ -176,9 +179,19 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { ForeignCallParam::Single(value), ) = (debug_vars, fcp_var_id, fcp_value) { let var_id = var_id_value.to_u128() as u32; - ds.set_by_id(var_id, value.clone()); + ds.set(var_id, value.clone()); + } + Ok(ForeignCallResult { values: vec![] }) + } + Some(ForeignCall::DebugStateDrop) => { + let fcp_var_id = &foreign_call.inputs[0]; + if let ( + Some(ds), + ForeignCallParam::Single(var_id_value), + ) = (debug_vars, fcp_var_id) { + let var_id = var_id_value.to_u128() as u32; + ds.drop(var_id); } - // pass-through return value is handled by the oracle wrapper, returning nothing here Ok(ForeignCallResult { values: vec![] }) } Some(ForeignCall::Sequence) => { From f8c0100704946662bae13535b9d75ca76beb53c4 Mon Sep 17 00:00:00 2001 From: synthia Date: Tue, 7 Nov 2023 15:21:33 -0800 Subject: [PATCH 06/11] factor out fn scope handling to also use for blocks --- compiler/noirc_frontend/src/debug/mod.rs | 40 ++++++++++++++++-------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index c6872ac3ed8..7a8158a7938 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -8,7 +8,7 @@ use std::collections::VecDeque; pub struct DebugState { var_id_to_name: HashMap, next_var_id: u32, - active_vars: HashSet, + scope: Vec>, pub enabled: bool, } @@ -16,7 +16,7 @@ impl Default for DebugState { fn default() -> Self { Self { var_id_to_name: HashMap::default(), - active_vars: HashSet::default(), + scope: vec![], next_var_id: 0, enabled: true, // TODO } @@ -41,10 +41,13 @@ impl DebugState { let var_id = self.next_var_id; self.next_var_id += 1; self.var_id_to_name.insert(var_id, var_name.to_string()); + self.scope.last_mut().unwrap().insert(var_id); var_id } fn walk_fn(&mut self, f: &mut ast::FunctionDefinition) { + self.scope.push(HashSet::new()); + let pvars: Vec<(u32,ast::Ident)> = f.parameters.iter() .flat_map(|(pattern, _utype, _vis)| { pattern_vars(pattern).iter().map(|id| { @@ -54,8 +57,23 @@ impl DebugState { .collect(); f.body.0.iter_mut().for_each(|stmt| self.walk_statement(stmt)); + f.body.0 = vec![ + // prapend fn params: + pvars.iter().map(|(var_id, id)| { + self.wrap_set_var(*var_id, id_expr(id)) + }).collect(), + + f.body.0.clone(), + ].concat(); + self.walk_scope(&mut f.body.0); + } + + // Modify a vector of statements in-place, adding instrumentation for sets and drops. + // This function will consume a scope level. + fn walk_scope(&mut self, statements: &mut Vec) { + statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); - let (ret_stmt, fn_body) = f.body.0.split_last() + let (ret_stmt, fn_body) = statements.split_last() .map(|(e,b)| (e.clone(), b.to_vec())) .unwrap_or(( ast::Statement { @@ -68,12 +86,7 @@ impl DebugState { vec![] )); - f.body.0 = vec![ - // set fn params: - pvars.iter().map(|(var_id, id)| { - self.wrap_set_var(*var_id, id_expr(id)) - }).collect(), - + *statements = vec![ // copy body minus the return expr: fn_body, @@ -93,7 +106,7 @@ impl DebugState { }], // drop fn params: - pvars.iter().map(|(var_id, _id)| { + self.scope.pop().unwrap_or(HashSet::default()).iter().map(|var_id| { self.wrap_drop_var(*var_id) }).collect(), @@ -239,9 +252,10 @@ impl DebugState { } fn walk_expr(&mut self, expr: &mut ast::Expression) { - match &expr.kind { - ast::ExpressionKind::Block(_block_expr) => { - // TODO: set then drop vars in this scope + match &mut expr.kind { + ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { + self.scope.push(HashSet::new()); + self.walk_scope(statements); }, _ => {}, } From f59a1e9b0cf460d1234b328295f8a5999d2f1055 Mon Sep 17 00:00:00 2001 From: synthia Date: Thu, 9 Nov 2023 14:45:13 -0800 Subject: [PATCH 07/11] plain case for mutable assignment updates --- compiler/noirc_evaluator/src/ssa.rs | 3 +- compiler/noirc_frontend/src/debug/mod.rs | 196 ++++++++++++++-------- tooling/debugger/src/repl.rs | 10 +- tooling/nargo/src/artifacts/debug_vars.rs | 18 +- tooling/nargo/src/ops/foreign_calls.rs | 72 +++++++- 5 files changed, 213 insertions(+), 86 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index f369121d2bd..fe50d4a6727 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -126,8 +126,7 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let variables = context.debug_state.get_variables(); - let mut debug_info = DebugInfo::new(locations, variables); + let mut debug_info = DebugInfo::new(locations, context.debug_state.variables.clone()); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 7a8158a7938..a439c56e210 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -6,7 +6,7 @@ use std::collections::VecDeque; #[derive(Debug,Clone)] pub struct DebugState { - var_id_to_name: HashMap, + pub variables: HashMap, // id => name next_var_id: u32, scope: Vec>, pub enabled: bool, @@ -15,7 +15,7 @@ pub struct DebugState { impl Default for DebugState { fn default() -> Self { Self { - var_id_to_name: HashMap::default(), + variables: HashMap::default(), scope: vec![], next_var_id: 0, enabled: true, // TODO @@ -24,23 +24,10 @@ impl Default for DebugState { } impl DebugState { - pub fn new(vars: HashMap) -> Self { - let mut debug_state = Self::default(); - for (var_name, var_id) in vars.iter() { - debug_state.var_id_to_name.insert(*var_id, var_name.clone()); - debug_state.next_var_id = debug_state.next_var_id.max(var_id+1); - } - debug_state - } - - pub fn get_variables(&self) -> HashMap { - self.var_id_to_name.clone() - } - fn insert_var(&mut self, var_name: &str) -> u32 { let var_id = self.next_var_id; self.next_var_id += 1; - self.var_id_to_name.insert(var_id, var_name.to_string()); + self.variables.insert(var_id, var_name.to_string()); self.scope.last_mut().unwrap().insert(var_id); var_id } @@ -48,24 +35,22 @@ impl DebugState { fn walk_fn(&mut self, f: &mut ast::FunctionDefinition) { self.scope.push(HashSet::new()); - let pvars: Vec<(u32,ast::Ident)> = f.parameters.iter() + let pvars: Vec<(u32,ast::Ident,bool)> = f.parameters.iter() .flat_map(|(pattern, _utype, _vis)| { - pattern_vars(pattern).iter().map(|id| { - (self.insert_var(&id.0.contents), id.clone()) - }).collect::>() + pattern_vars(pattern).iter().map(|(id,is_mut)| { + (self.insert_var(&id.0.contents), id.clone(), *is_mut) + }).collect::>() }) .collect(); - f.body.0.iter_mut().for_each(|stmt| self.walk_statement(stmt)); - f.body.0 = vec![ - // prapend fn params: - pvars.iter().map(|(var_id, id)| { - self.wrap_set_var(*var_id, id_expr(id)) - }).collect(), + let set_fn_params = pvars.iter().map(|(var_id, id, _is_mut)| { + self.wrap_assign_var(*var_id, id_expr(id)) + }).collect(); - f.body.0.clone(), - ].concat(); self.walk_scope(&mut f.body.0); + + // prapend fn params: + f.body.0 = vec![ set_fn_params, f.body.0.clone() ].concat(); } // Modify a vector of statements in-place, adding instrumentation for sets and drops. @@ -90,12 +75,12 @@ impl DebugState { // copy body minus the return expr: fn_body, - // assign return expr to __debug_state_return: + // assign return expr to __debug_expr: vec![match &ret_stmt.kind { ast::StatementKind::Expression(ret_expr) => { ast::Statement { kind: ast::StatementKind::Let(ast::LetStatement { - pattern: ast::Pattern::Identifier(ident("__debug_state_return")), + pattern: ast::Pattern::Identifier(ident("__debug_expr")), r#type: ast::UnresolvedType::unspecified(), expression: ret_expr.clone(), }), @@ -110,13 +95,13 @@ impl DebugState { self.wrap_drop_var(*var_id) }).collect(), - // return the __debug_state_return value: + // return the __debug_expr value: vec![match &ret_stmt.kind { - ast::StatementKind::Expression(ret_expr) => { + ast::StatementKind::Expression(_ret_expr) => { ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_state_return")], + segments: vec![ident("__debug_expr")], kind: PathKind::Plain, }), span: none_span(), @@ -150,11 +135,11 @@ impl DebugState { self.insert_state_set_oracle(module); } - fn wrap_set_var(&mut self, var_id: u32, expr: ast::Expression) -> ast::Statement { + fn wrap_assign_var(&mut self, var_id: u32, expr: ast::Expression) -> 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("__debug_state_set")], + segments: vec![ident("__debug_var_assign")], kind: PathKind::Plain }), span: none_span(), @@ -179,7 +164,7 @@ impl DebugState { let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_state_drop")], + segments: vec![ident("__debug_var_drop")], kind: PathKind::Plain }), span: none_span(), @@ -215,10 +200,17 @@ impl DebugState { // }; let vars = pattern_vars(&let_stmt.pattern); - let vars_pattern: Vec = vars.iter().map(|id| { - ast::Pattern::Identifier(id.clone()) + let vars_pattern: Vec = vars.iter().map(|(id,is_mut)| { + if *is_mut { + ast::Pattern::Mutable( + Box::new(ast::Pattern::Identifier(id.clone())), + none_span() + ) + } else { + ast::Pattern::Identifier(id.clone()) + } }).collect(); - let vars_exprs: Vec = vars.iter().map(|id| id_expr(id)).collect(); + let vars_exprs: Vec = vars.iter().map(|(id,_)| id_expr(id)).collect(); let mut block_stmts = vec![ ast::Statement { @@ -226,9 +218,9 @@ impl DebugState { span: none_span(), }, ]; - block_stmts.extend(vars.iter().map(|id| { + block_stmts.extend(vars.iter().map(|(id,_)| { let var_id = self.insert_var(&id.0.contents); - self.wrap_set_var(var_id, id_expr(id)) + self.wrap_assign_var(var_id, id_expr(id)) })); block_stmts.push(ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { @@ -251,6 +243,55 @@ impl DebugState { } } + fn wrap_assign_statement(&mut self, assign_stmt: &ast::AssignStatement, span: &Span) -> ast::Statement { + // X = Y becomes: + // X = { + // let __debug_expr = Y; + // wrap(1, __debug_expr); + // __debug_expr + // }; + + let let_kind = ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Identifier(ident("__debug_expr")), + r#type: ast::UnresolvedType::unspecified(), + expression: assign_stmt.expression.clone(), + }); + let new_assign_stmt = match &assign_stmt.lvalue { + ast::LValue::Ident(id) => { + let var_id = self.insert_var(&id.0.contents); + self.wrap_assign_var(var_id, id_expr(&ident("__debug_expr"))) + }, + ast::LValue::MemberAccess { object: _object, field_name: _field_name } => { + // TODO + unimplemented![] + }, + ast::LValue::Index { array: _array, index: _index } => { + // TODO + unimplemented![] + }, + ast::LValue::Dereference(_lv) => { + // TODO + unimplemented![] + }, + }; + let ret_kind = ast::StatementKind::Expression(id_expr(&ident("__debug_expr"))); + + ast::Statement { + kind: ast::StatementKind::Assign(ast::AssignStatement { + lvalue: assign_stmt.lvalue.clone(), + expression: ast::Expression { + kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ + ast::Statement { kind: let_kind, span: none_span() }, + new_assign_stmt, + ast::Statement { kind: ret_kind, span: none_span() }, + ])), + span: none_span(), + }, + }), + span: span.clone(), + } + } + fn walk_expr(&mut self, expr: &mut ast::Expression) { match &mut expr.kind { ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { @@ -265,7 +306,7 @@ impl DebugState { let var_name = &for_stmt.identifier.0.contents; let var_id = self.insert_var(var_name); - let set_stmt = self.wrap_set_var(var_id, id_expr(&for_stmt.identifier)); + let set_stmt = self.wrap_assign_var(var_id, id_expr(&for_stmt.identifier)); let drop_stmt = self.wrap_drop_var(var_id); for_stmt.block = ast::Expression { @@ -286,6 +327,9 @@ impl DebugState { ast::StatementKind::Let(let_stmt) => { *stmt = self.wrap_let_statement(&let_stmt, &stmt.span); }, + ast::StatementKind::Assign(assign_stmt) => { + *stmt = self.wrap_assign_statement(&assign_stmt, &stmt.span); + }, ast::StatementKind::Expression(expr) => { self.walk_expr(expr); }, @@ -295,35 +339,55 @@ impl DebugState { ast::StatementKind::For(ref mut for_stmt) => { self.walk_for(for_stmt); }, - ast::StatementKind::Assign(_assign_stmt) => { - // TODO - }, _ => {}, // Constrain, Error } } fn insert_state_set_oracle(&self, module: &mut ParsedModule) { let (program, errors) = parse_program(r#" - #[oracle(__debug_state_set)] - unconstrained fn __debug_state_set_oracle(_var_id: u32, _input: T) {} - - #[oracle(__debug_state_drop)] - unconstrained fn __debug_state_drop_oracle(_var_id: u32) {} + #[oracle(__debug_var_assign)] + unconstrained fn __debug_var_assign_oracle(_var_id: u32, _input: T) {} + unconstrained fn __debug_var_assign_inner(var_id: u32, value: T) { + __debug_var_assign_oracle(var_id, value); + } + pub fn __debug_var_assign(var_id: u32, value: T) { + __debug_var_assign_inner(var_id, value); + } - unconstrained fn __debug_state_set_inner(var_id: u32, value: T) { - __debug_state_set_oracle(var_id, value); + #[oracle(__debug_var_drop)] + 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) { + __debug_var_drop_inner(var_id); } - unconstrained fn __debug_state_drop_inner(var_id: u32) { - __debug_state_drop_oracle(var_id); + #[oracle(__debug_member_assign)] + unconstrained fn __debug_member_assign_oracle(_var_id: u32, _member_id: u32, _input: T) {} + unconstrained fn __debug_member_assign_inner(var_id: u32, member_id: u32,value: T) { + __debug_member_assign_oracle(var_id, member_id, value); + } + pub fn __debug_member_assign(var_id: u32, member_id: u32, value: T) { + __debug_member_assign_inner(var_id, member_id, value); } - pub fn __debug_state_set(var_id: u32, value: T) { - __debug_state_set_inner(var_id, value); + #[oracle(__debug_index_assign)] + unconstrained fn __debug_index_assign_oracle(_var_id: u32, _index: Field, _input: T) {} + unconstrained fn __debug_index_assign_inner(var_id: u32, index: Field, value: T) { + __debug_index_assign_oracle(var_id, index, value); + } + pub fn __debug_index_assign(var_id: u32, index: Field, value: T) { + __debug_index_assign_inner(var_id, index, value); } - pub fn __debug_state_drop(var_id: u32) { - __debug_state_drop_inner(var_id); + #[oracle(__debug_dereference_assign)] + unconstrained fn __debug_dereference_assign_oracle(_var_id: u32, _input: T) {} + unconstrained fn __debug_dereference_assign_inner(var_id: u32, value: T) { + __debug_dereference_assign_oracle(var_id, value); + } + pub fn __debug_dereference_assign(var_id: u32, value: T) { + __debug_dereference_assign_inner(var_id, value); } "#); if !errors.is_empty() { panic!("errors parsing internal oracle definitions: {errors:?}") } @@ -331,24 +395,24 @@ impl DebugState { } } -fn pattern_vars(pattern: &ast::Pattern) -> Vec { +fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident,bool)> { let mut vars = vec![]; - let mut stack = VecDeque::from([ pattern ]); + let mut stack = VecDeque::from([ (pattern,false) ]); while stack.front().is_some() { - let pattern = stack.pop_front().unwrap(); + let (pattern,is_mut) = stack.pop_front().unwrap(); match pattern { ast::Pattern::Identifier(id) => { - vars.push(id.clone()); + vars.push((id.clone(),is_mut)); }, ast::Pattern::Mutable(pattern, _) => { - stack.push_back(pattern); + stack.push_back((pattern,true)); }, ast::Pattern::Tuple(patterns, _) => { - stack.extend(patterns.iter()); + stack.extend(patterns.iter().map(|pattern| (pattern, false))); }, ast::Pattern::Struct(_, pids, _) => { - stack.extend(pids.iter().map(|(_, pattern)| pattern)); - vars.extend(pids.iter().map(|(id, _)| id.clone())); + stack.extend(pids.iter().map(|(_, pattern)| (pattern,is_mut))); + vars.extend(pids.iter().map(|(id, _)| (id.clone(), false))); }, } } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index c62261393a6..9841e8ca0fb 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -125,7 +125,6 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { let Range { start: loc_start, end: mut loc_end } = self.debug_artifact.location_in_line(loc).unwrap(); loc_end = loc_end.min(line.len()); - println!["loc_start={loc_start}, loc_end={loc_end}, loc={loc:?}"]; println!( "{:>3} {:2} {}{}{}", current_line_number, @@ -560,10 +559,11 @@ pub fn run( "show variable values available at this point in execution", () => || { let mut ctx = ref_context.borrow_mut(); - let vars = ctx.context.debug_vars.get_values(); - println!("{:?}", vars.iter().map(|(var_name,value)| { - (var_name, value.to_field()) - }).collect::>()); + let vars = ctx.context.debug_vars.get_variables(); + println!["{}", vars.iter().map(|(var_name, value)| { + let field = value.to_field(); + format!("{var_name}={field}") + }).collect::>().join(", ")]; Ok(CommandStatus::Done) } }, diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index c42d38316f0..1dc68cf2afd 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -15,7 +15,7 @@ impl DebugVars { debug_vars } - pub fn get_values<'a>(&'a self) -> HashMap<&'a str, &'a Value> { + pub fn get_variables<'a>(&'a self) -> Vec<(&'a str, &'a Value)> { self.active.iter().filter_map(|var_id| { self.id_to_name.get(var_id).and_then(|name| { self.id_to_value.get(var_id).map(|value| (name.as_str(), value)) @@ -29,20 +29,30 @@ impl DebugVars { }); } - pub fn set(&mut self, var_id: u32, value: Value) { + pub fn assign(&mut self, var_id: u32, value: Value) { let name = self.id_to_name.get(&var_id).unwrap(); - println!["\n\n######## SET {name}[{var_id}] = {value:?}\n"]; self.active.insert(var_id); self.id_to_value.insert(var_id, value); } + pub fn assign_member(&mut self, _var_id: u32, _member_id: u32, _value: Value) { + // TODO + } + + pub fn assign_index(&mut self, _var_id: u32, _index: u64, _value: Value) { + // TODO + } + + pub fn assign_deref(&mut self, _var_id: u32, _value: Value) { + // TODO + } + pub fn get<'a>(&'a mut self, var_id: u32) -> Option<&'a Value> { self.id_to_value.get(&var_id) } pub fn drop(&mut self, var_id: u32) { let name = self.id_to_name.get(&var_id).unwrap(); - println!["\n\n######## DROP {name}[{var_id}]\n"]; self.active.remove(&var_id); } } diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index ae91ba52df1..549c67fe676 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -17,8 +17,11 @@ pub trait ForeignCallExecutor { /// After resolution of a foreign call, nargo will restart execution of the ACVM pub(crate) enum ForeignCall { Println, - DebugStateSet, - DebugStateDrop, + DebugVarAssign, + DebugVarDrop, + DebugMemberAssign, + DebugIndexAssign, + DebugDerefAssign, Sequence, ReverseSequence, CreateMock, @@ -38,8 +41,11 @@ impl ForeignCall { pub(crate) fn name(&self) -> &'static str { match self { ForeignCall::Println => "println", - ForeignCall::DebugStateSet => "__debug_state_set", - ForeignCall::DebugStateDrop => "__debug_state_drop", + ForeignCall::DebugVarAssign => "__debug_var_assign", + ForeignCall::DebugVarDrop => "__debug_var_drop", + ForeignCall::DebugMemberAssign => "__debug_member_assign", + ForeignCall::DebugIndexAssign => "__debug_index_assign", + ForeignCall::DebugDerefAssign => "__debug_deref_assign", ForeignCall::Sequence => "get_number_sequence", ForeignCall::ReverseSequence => "get_reverse_number_sequence", ForeignCall::CreateMock => "create_mock", @@ -53,8 +59,11 @@ impl ForeignCall { pub(crate) fn lookup(op_name: &str) -> Option { match op_name { "println" => Some(ForeignCall::Println), - "__debug_state_set" => Some(ForeignCall::DebugStateSet), - "__debug_state_drop" => Some(ForeignCall::DebugStateDrop), + "__debug_var_assign" => Some(ForeignCall::DebugVarAssign), + "__debug_var_drop" => Some(ForeignCall::DebugVarDrop), + "__debug_member_assign" => Some(ForeignCall::DebugMemberAssign), + "__debug_index_assign" => Some(ForeignCall::DebugIndexAssign), + "__debug_deref_assign" => Some(ForeignCall::DebugDerefAssign), "get_number_sequence" => Some(ForeignCall::Sequence), "get_reverse_number_sequence" => Some(ForeignCall::ReverseSequence), "create_mock" => Some(ForeignCall::CreateMock), @@ -170,7 +179,7 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { } Ok(ForeignCallResult { values: vec![] }) } - Some(ForeignCall::DebugStateSet) => { + Some(ForeignCall::DebugVarAssign) => { let fcp_var_id = &foreign_call.inputs[0]; let fcp_value = &foreign_call.inputs[1]; if let ( @@ -179,11 +188,11 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { ForeignCallParam::Single(value), ) = (debug_vars, fcp_var_id, fcp_value) { let var_id = var_id_value.to_u128() as u32; - ds.set(var_id, value.clone()); + ds.assign(var_id, value.clone()); } Ok(ForeignCallResult { values: vec![] }) } - Some(ForeignCall::DebugStateDrop) => { + Some(ForeignCall::DebugVarDrop) => { let fcp_var_id = &foreign_call.inputs[0]; if let ( Some(ds), @@ -194,6 +203,51 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { } Ok(ForeignCallResult { values: vec![] }) } + Some(ForeignCall::DebugMemberAssign) => { + let fcp_var_id = &foreign_call.inputs[0]; + let fcp_member_id = &foreign_call.inputs[1]; + let fcp_value = &foreign_call.inputs[2]; + if let ( + Some(ds), + ForeignCallParam::Single(var_id_value), + ForeignCallParam::Single(member_id_value), + ForeignCallParam::Single(value), + ) = (debug_vars, fcp_var_id, fcp_member_id, fcp_value) { + let var_id = var_id_value.to_u128() as u32; + let member_id = member_id_value.to_u128() as u32; + ds.assign_member(var_id, member_id, value.clone()); + } + Ok(ForeignCallResult { values: vec![] }) + } + Some(ForeignCall::DebugIndexAssign) => { + let fcp_var_id = &foreign_call.inputs[0]; + let fcp_index = &foreign_call.inputs[1]; + let fcp_value = &foreign_call.inputs[2]; + if let ( + Some(ds), + ForeignCallParam::Single(var_id_value), + ForeignCallParam::Single(index_value), + ForeignCallParam::Single(value), + ) = (debug_vars, fcp_var_id, fcp_index, fcp_value) { + let var_id = var_id_value.to_u128() as u32; + let index = index_value.to_u128() as u64; + ds.assign_index(var_id, index, value.clone()); + } + Ok(ForeignCallResult { values: vec![] }) + } + Some(ForeignCall::DebugDerefAssign) => { + let fcp_var_id = &foreign_call.inputs[0]; + let fcp_value = &foreign_call.inputs[1]; + if let ( + Some(ds), + ForeignCallParam::Single(var_id_value), + ForeignCallParam::Single(value), + ) = (debug_vars, fcp_var_id, fcp_value) { + let var_id = var_id_value.to_u128() as u32; + ds.assign_deref(var_id, value.clone()); + } + Ok(ForeignCallResult { values: vec![] }) + } Some(ForeignCall::Sequence) => { let sequence_length: u128 = foreign_call.inputs[0].unwrap_value().to_field().to_u128(); From 8dda4caeded911585554d796720f4948f9c7b8fc Mon Sep 17 00:00:00 2001 From: synthia Date: Mon, 13 Nov 2023 10:14:50 -0800 Subject: [PATCH 08/11] walk all expressions for debug instrumentation --- compiler/noirc_frontend/src/debug/mod.rs | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index a439c56e210..0f93578352c 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -298,6 +298,60 @@ impl DebugState { self.scope.push(HashSet::new()); self.walk_scope(statements); }, + ast::ExpressionKind::Prefix(prefix_expr) => { + self.walk_expr(&mut prefix_expr.rhs); + }, + ast::ExpressionKind::Index(index_expr) => { + self.walk_expr(&mut index_expr.collection); + self.walk_expr(&mut index_expr.index); + }, + ast::ExpressionKind::Call(call_expr) => { + // TODO: push a stack frame or something here? + self.walk_expr(&mut call_expr.func); + call_expr.arguments.iter_mut().for_each(|ref mut expr| { + self.walk_expr(expr); + }); + }, + ast::ExpressionKind::MethodCall(mc_expr) => { + // TODO: also push a stack frame here + self.walk_expr(&mut mc_expr.object); + mc_expr.arguments.iter_mut().for_each(|ref mut expr| { + self.walk_expr(expr); + }); + }, + ast::ExpressionKind::Constructor(c_expr) => { + c_expr.fields.iter_mut().for_each(|(_id, ref mut expr)| { + self.walk_expr(expr); + }); + }, + ast::ExpressionKind::MemberAccess(ma_expr) => { + self.walk_expr(&mut ma_expr.lhs); + }, + ast::ExpressionKind::Cast(cast_expr) => { + self.walk_expr(&mut cast_expr.lhs); + }, + ast::ExpressionKind::Infix(infix_expr) => { + self.walk_expr(&mut infix_expr.lhs); + self.walk_expr(&mut infix_expr.rhs); + }, + ast::ExpressionKind::If(if_expr) => { + self.walk_expr(&mut if_expr.condition); + self.walk_expr(&mut if_expr.consequence); + if let Some(ref mut alt) = if_expr.alternative { + self.walk_expr(alt); + } + }, + ast::ExpressionKind::Tuple(exprs) => { + exprs.iter_mut().for_each(|ref mut expr| { + self.walk_expr(expr); + }); + }, + ast::ExpressionKind::Lambda(lambda) => { + self.walk_expr(&mut lambda.body); + }, + ast::ExpressionKind::Parenthesized(expr) => { + self.walk_expr(expr); + }, _ => {}, } } @@ -309,6 +363,7 @@ impl DebugState { let set_stmt = self.wrap_assign_var(var_id, id_expr(&for_stmt.identifier)); let drop_stmt = self.wrap_drop_var(var_id); + self.walk_expr(&mut for_stmt.block); for_stmt.block = ast::Expression { kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ set_stmt, From 31812fb5c8a1c0b89d829fa91ec5986cb4123224 Mon Sep 17 00:00:00 2001 From: synthia Date: Thu, 16 Nov 2023 22:32:10 -0800 Subject: [PATCH 09/11] collect and display type info for the debugger --- compiler/noirc_errors/Cargo.toml | 1 + compiler/noirc_errors/src/debug_info.rs | 16 +++++++-- compiler/noirc_evaluator/src/ssa.rs | 6 +++- compiler/noirc_frontend/src/debug/mod.rs | 13 ++++---- .../src/monomorphization/ast.rs | 16 +++++++-- .../src/monomorphization/debug_types.rs | 33 +++++++++++++++++++ .../src/monomorphization/mod.rs | 30 ++++++++++++++++- compiler/noirc_printable_type/src/lib.rs | 2 +- tooling/debugger/src/context.rs | 1 + tooling/debugger/src/repl.rs | 5 ++- tooling/nargo/src/artifacts/debug_vars.rs | 31 ++++++++++++----- 11 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 compiler/noirc_frontend/src/monomorphization/debug_types.rs diff --git a/compiler/noirc_errors/Cargo.toml b/compiler/noirc_errors/Cargo.toml index 0cb6afc73bd..8e30c39be35 100644 --- a/compiler/noirc_errors/Cargo.toml +++ b/compiler/noirc_errors/Cargo.toml @@ -13,5 +13,6 @@ codespan-reporting.workspace = true codespan.workspace = true fm.workspace = true chumsky.workspace = true +noirc_printable_type.workspace = true serde.workspace = true serde_with = "3.2.0" diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index a3e10e0e7de..dc60a2408d1 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -9,6 +9,11 @@ use std::mem; use crate::Location; use serde::{Deserialize, Serialize}; +use noirc_printable_type::PrintableType; + +pub type Variables = Vec<(u32, (String, u32))>; +pub type Types = Vec<(u32, PrintableType)>; +pub type VariableTypes = (Variables,Types); #[serde_as] #[derive(Default, Debug, Clone, Deserialize, Serialize)] @@ -18,7 +23,8 @@ pub struct DebugInfo { /// that they should be serialized to/from strings. #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, - pub variables: HashMap, + pub variables: HashMap, // var_id => (name, type_id) + pub types: HashMap, // type_id => printable type } /// Holds OpCodes Counts for Acir and Brillig Opcodes @@ -32,9 +38,13 @@ pub struct OpCodesCount { impl DebugInfo { pub fn new( locations: BTreeMap>, - variables: HashMap, + var_types: VariableTypes, ) -> Self { - Self { locations, variables } + Self { + locations, + variables: var_types.0.into_iter().collect(), + types: var_types.1.into_iter().collect(), + } } /// 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 fe50d4a6727..f7b5a32a19f 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -87,6 +87,7 @@ pub fn create_circuit( enable_ssa_logging: bool, enable_brillig_logging: bool, ) -> Result<(Circuit, DebugInfo, Abi, Vec), RuntimeError> { + let debug_var_types = program.debug_var_types.clone(); let func_sig = program.main_function_signature.clone(); let mut generated_acir = optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?; @@ -126,7 +127,10 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations, context.debug_state.variables.clone()); + let mut debug_info = DebugInfo::new( + locations, + debug_var_types, + ); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 0f93578352c..0eae570ab8a 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -6,7 +6,7 @@ use std::collections::VecDeque; #[derive(Debug,Clone)] pub struct DebugState { - pub variables: HashMap, // id => name + pub variables: HashMap, // var_id => name next_var_id: u32, scope: Vec>, pub enabled: bool, @@ -267,6 +267,7 @@ impl DebugState { }, ast::LValue::Index { array: _array, index: _index } => { // TODO + // TODO: also remember to self.walk(index) unimplemented![] }, ast::LValue::Dereference(_lv) => { @@ -401,7 +402,7 @@ impl DebugState { fn insert_state_set_oracle(&self, module: &mut ParsedModule) { let (program, errors) = parse_program(r#" #[oracle(__debug_var_assign)] - unconstrained fn __debug_var_assign_oracle(_var_id: u32, _input: T) {} + unconstrained fn __debug_var_assign_oracle(_var_id: u32, _value: T) {} unconstrained fn __debug_var_assign_inner(var_id: u32, value: T) { __debug_var_assign_oracle(var_id, value); } @@ -419,8 +420,8 @@ impl DebugState { } #[oracle(__debug_member_assign)] - unconstrained fn __debug_member_assign_oracle(_var_id: u32, _member_id: u32, _input: T) {} - unconstrained fn __debug_member_assign_inner(var_id: u32, member_id: u32,value: T) { + unconstrained fn __debug_member_assign_oracle(_var_id: u32, _member_id: u32, _value: T) {} + unconstrained fn __debug_member_assign_inner(var_id: u32, member_id: u32, value: T) { __debug_member_assign_oracle(var_id, member_id, value); } pub fn __debug_member_assign(var_id: u32, member_id: u32, value: T) { @@ -428,7 +429,7 @@ impl DebugState { } #[oracle(__debug_index_assign)] - unconstrained fn __debug_index_assign_oracle(_var_id: u32, _index: Field, _input: T) {} + unconstrained fn __debug_index_assign_oracle(_var_id: u32, _index: Field, _value: T) {} unconstrained fn __debug_index_assign_inner(var_id: u32, index: Field, value: T) { __debug_index_assign_oracle(var_id, index, value); } @@ -437,7 +438,7 @@ impl DebugState { } #[oracle(__debug_dereference_assign)] - unconstrained fn __debug_dereference_assign_oracle(_var_id: u32, _input: T) {} + unconstrained fn __debug_dereference_assign_oracle(_var_id: u32, _value: T) {} unconstrained fn __debug_dereference_assign_inner(var_id: u32, value: T) { __debug_dereference_assign_oracle(var_id, value); } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 0a005d766fe..73d1d8f5351 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -2,7 +2,11 @@ use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::Location; -use crate::{hir_def::function::FunctionSignature, BinaryOpKind, Distinctness, Signedness}; +use crate::{ + hir_def::function::FunctionSignature, + BinaryOpKind, Distinctness, Signedness, + monomorphization::debug_types, +}; /// The monomorphized AST is expression-based, all statements are also /// folded into this expression enum. Compared to the HIR, the monomorphized @@ -243,6 +247,7 @@ pub struct Program { /// forwarding to the next phase. pub return_distinctness: Distinctness, pub return_location: Option, + pub debug_var_types: debug_types::VariableTypes, } impl Program { @@ -251,8 +256,15 @@ impl Program { main_function_signature: FunctionSignature, return_distinctness: Distinctness, return_location: Option, + debug_var_types: debug_types::VariableTypes, ) -> Program { - Program { functions, main_function_signature, return_distinctness, return_location } + Program { + functions, + main_function_signature, + return_distinctness, + return_location, + debug_var_types + } } pub fn main(&self) -> &Function { diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs new file mode 100644 index 00000000000..02f53ccf6b7 --- /dev/null +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -0,0 +1,33 @@ +use std::collections::HashMap; +use crate::hir_def::types::Type; +use noirc_printable_type::PrintableType; +pub use noirc_errors::debug_info::{Variables, Types, VariableTypes}; + +#[derive(Debug, Clone, Default)] +pub struct DebugTypes { + variables: HashMap, // var_id => (var_name, type_id) + types: HashMap, + next_type_id: u32, +} + +impl DebugTypes { + pub fn insert_var(&mut self, var_id: u32, var_name: &str, var_type: Type) { + let ptype: PrintableType = var_type.into(); + let type_id = self.types.get(&ptype).cloned().unwrap_or_else(|| { + let type_id = self.next_type_id; + self.next_type_id += 1; + self.types.insert(ptype, type_id); + type_id + }); + self.variables.insert(var_id, (var_name.to_string(), type_id)); + } +} + +impl Into for DebugTypes { + fn into(self) -> VariableTypes { + ( + self.variables.into_iter().collect(), + self.types.into_iter().map(|(ptype, type_id)| (type_id, ptype)).collect(), + ) + } +} diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 57e4e6cdeb0..d2e1d301186 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -31,9 +31,11 @@ use crate::{ }; use self::ast::{Definition, FuncId, Function, LocalId, Program}; +use self::debug_types::DebugTypes; pub mod ast; pub mod printer; +pub mod debug_types; struct LambdaContext { env_ident: ast::Ident, @@ -76,6 +78,8 @@ struct Monomorphizer<'interner> { is_range_loop: bool, return_location: Option, + + debug_types: DebugTypes, } type HirType = crate::Type; @@ -106,7 +110,13 @@ pub fn monomorphize(main: node_interner::FuncId, interner: &NodeInterner) -> Pro let functions = vecmap(monomorphizer.finished_functions, |(_, f)| f); let FuncMeta { return_distinctness, .. } = interner.function_meta(&main); - Program::new(functions, function_sig, return_distinctness, monomorphizer.return_location) + Program::new( + functions, + function_sig, + return_distinctness, + monomorphizer.return_location, + monomorphizer.debug_types.into(), + ) } impl<'interner> Monomorphizer<'interner> { @@ -122,6 +132,7 @@ impl<'interner> Monomorphizer<'interner> { lambda_envs_stack: Vec::new(), is_range_loop: false, return_location: None, + debug_types: DebugTypes::default(), } } @@ -875,6 +886,23 @@ impl<'interner> Monomorphizer<'interner> { let original_func = Box::new(self.expr(call.func)); let mut arguments = vecmap(&call.arguments, |id| self.expr(*id)); let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); + if let ( + ast::Expression::Ident(ast::Ident { name, .. }), + 2, + ) = (original_func.as_ref(), arguments.len()) { + if let ( + HirExpression::Literal(HirLiteral::Integer(fe_var_id)), + HirExpression::Ident(HirIdent { id, .. }), + true + ) = (&hir_arguments[0], &hir_arguments[1], name == "__debug_var_assign") { + let var_def = self.interner.definition(*id); + let var_type = self.interner.id_type(call.arguments[1]); + let var_id = fe_var_id.to_u128() as u32; + if var_def.name != "__debug_expr" { + self.debug_types.insert_var(var_id, &var_def.name, var_type); + } + } + } let func: Box; let return_type = self.interner.id_type(id); let return_type = self.convert_type(&return_type); diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 348f5ef3274..8d1c2e3f753 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -6,7 +6,7 @@ use regex::{Captures, Regex}; use serde::{Deserialize, Serialize}; use thiserror::Error; -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "lowercase")] pub enum PrintableType { Field, diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 1b196984b74..2f258843e74 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -41,6 +41,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { let mut debug_vars = DebugVars::default(); debug_artifact.debug_symbols.iter().for_each(|info| { debug_vars.insert_variables(&info.variables); + debug_vars.insert_types(&info.types); }); Self { acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 9841e8ca0fb..0a46c73d664 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -9,7 +9,6 @@ use nargo::NargoError; use easy_repl::{command, CommandStatus, Repl}; use std::cell::RefCell; -use std::collections::HashMap; use codespan_reporting::files::Files; use noirc_errors::Location; @@ -560,9 +559,9 @@ pub fn run( () => || { let mut ctx = ref_context.borrow_mut(); let vars = ctx.context.debug_vars.get_variables(); - println!["{}", vars.iter().map(|(var_name, value)| { + println!["{}", vars.iter().map(|(var_name, value, var_type)| { let field = value.to_field(); - format!("{var_name}={field}") + format!("{var_name}:{var_type:?}={field}") }).collect::>().join(", ")]; Ok(CommandStatus::Done) } diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 1dc68cf2afd..72e5b1acde2 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -1,11 +1,14 @@ use std::collections::{HashMap,HashSet}; use acvm::acir::brillig::Value; +use noirc_printable_type::PrintableType; #[derive(Debug, Default, Clone)] pub struct DebugVars { id_to_name: HashMap, active: HashSet, id_to_value: HashMap, // TODO: something more sophisticated for lexical levels + id_to_type: HashMap, + types: HashMap, } impl DebugVars { @@ -15,22 +18,35 @@ impl DebugVars { debug_vars } - pub fn get_variables<'a>(&'a self) -> Vec<(&'a str, &'a Value)> { + pub fn get_variables<'a>(&'a self) -> Vec<(&'a str, &'a Value, &'a PrintableType)> { self.active.iter().filter_map(|var_id| { - self.id_to_name.get(var_id).and_then(|name| { - self.id_to_value.get(var_id).map(|value| (name.as_str(), value)) - }) + self.id_to_name.get(var_id) + .and_then(|name| { + self.id_to_value.get(var_id).map(|value| (name, value)) + }) + .and_then(|(name, value)| { + self.id_to_type.get(var_id).map(|type_id| (name, value, type_id)) + }) + .and_then(|(name, value, type_id)| { + self.types.get(type_id).map(|ptype| (name.as_str(), value, ptype)) + }) }).collect() } - pub fn insert_variables(&mut self, vars: &HashMap) { - vars.iter().for_each(|(var_id, var_name)| { + pub fn insert_variables(&mut self, vars: &HashMap) { + vars.iter().for_each(|(var_id, (var_name, var_type))| { self.id_to_name.insert(*var_id, var_name.clone()); + self.id_to_type.insert(*var_id, *var_type); + }); + } + + pub fn insert_types(&mut self, types: &HashMap) { + types.iter().for_each(|(type_id, ptype)| { + self.types.insert(*type_id, ptype.clone()); }); } pub fn assign(&mut self, var_id: u32, value: Value) { - let name = self.id_to_name.get(&var_id).unwrap(); self.active.insert(var_id); self.id_to_value.insert(var_id, value); } @@ -52,7 +68,6 @@ impl DebugVars { } pub fn drop(&mut self, var_id: u32) { - let name = self.id_to_name.get(&var_id).unwrap(); self.active.remove(&var_id); } } From cd88fa17888d0d8359e610cff104e42868a6f97f Mon Sep 17 00:00:00 2001 From: synthia Date: Mon, 20 Nov 2023 19:05:15 -0800 Subject: [PATCH 10/11] move default foreign call executor internal debug methods --- compiler/noirc_frontend/src/debug/mod.rs | 4 +- tooling/debugger/src/context.rs | 2 +- tooling/nargo/src/ops/foreign_calls.rs | 68 ++++++++++++------------ 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 0eae570ab8a..3aaea9a6756 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -36,8 +36,8 @@ impl DebugState { self.scope.push(HashSet::new()); let pvars: Vec<(u32,ast::Ident,bool)> = f.parameters.iter() - .flat_map(|(pattern, _utype, _vis)| { - pattern_vars(pattern).iter().map(|(id,is_mut)| { + .flat_map(|param| { + pattern_vars(¶m.pattern).iter().map(|(id,is_mut)| { (self.insert_var(&id.0.contents), id.clone(), *is_mut) }).collect::>() }) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 2f258843e74..0d7e0068064 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -8,7 +8,7 @@ use acvm::{BlackBoxFunctionSolver, FieldElement}; use nargo::artifacts::debug::{DebugArtifact, DebugVars}; use nargo::errors::{ExecutionError, Location}; -use nargo::ops::{DefaultForeignCallExecutor, ForeignCallExecutor}; +use nargo::ops::{DefaultForeignCallExecutor}; use nargo::NargoError; use std::collections::{hash_set::Iter, HashSet}; diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 549c67fe676..85c7a96ca71 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -123,40 +123,6 @@ impl DefaultForeignCallExecutor { pub fn new(show_output: bool) -> Self { DefaultForeignCallExecutor { show_output, ..DefaultForeignCallExecutor::default() } } -} - -impl DefaultForeignCallExecutor { - fn extract_mock_id( - foreign_call_inputs: &[ForeignCallParam], - ) -> Result<(usize, &[ForeignCallParam]), ForeignCallError> { - let (id, params) = - foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - Ok((id.unwrap_value().to_usize(), params)) - } - - fn find_mock_by_id(&mut self, id: usize) -> Option<&mut MockedCall> { - self.mocked_responses.iter_mut().find(|response| response.id == id) - } - - fn parse_string(param: &ForeignCallParam) -> String { - let fields: Vec<_> = param.values().into_iter().map(|value| value.to_field()).collect(); - decode_string_value(&fields) - } - - fn execute_println(foreign_call_inputs: &[ForeignCallParam]) -> Result<(), ForeignCallError> { - let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; - println!("{display_values}"); - Ok(()) - } -} - -impl ForeignCallExecutor for DefaultForeignCallExecutor { - fn execute( - &mut self, - foreign_call: &ForeignCallWaitInfo, - ) -> Result { - self.execute_optional_debug_vars(foreign_call, None) - } pub fn execute_with_debug_vars( &mut self, @@ -341,3 +307,37 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { } } } + +impl DefaultForeignCallExecutor { + fn extract_mock_id( + foreign_call_inputs: &[ForeignCallParam], + ) -> Result<(usize, &[ForeignCallParam]), ForeignCallError> { + let (id, params) = + foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; + Ok((id.unwrap_value().to_usize(), params)) + } + + fn find_mock_by_id(&mut self, id: usize) -> Option<&mut MockedCall> { + self.mocked_responses.iter_mut().find(|response| response.id == id) + } + + fn parse_string(param: &ForeignCallParam) -> String { + let fields: Vec<_> = param.values().into_iter().map(|value| value.to_field()).collect(); + decode_string_value(&fields) + } + + fn execute_println(foreign_call_inputs: &[ForeignCallParam]) -> Result<(), ForeignCallError> { + let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; + println!("{display_values}"); + Ok(()) + } +} + +impl ForeignCallExecutor for DefaultForeignCallExecutor { + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result { + self.execute_optional_debug_vars(foreign_call, None) + } +} From 8b88af93870db10b2727d0e062e2329070b78015 Mon Sep 17 00:00:00 2001 From: synthia Date: Mon, 20 Nov 2023 19:43:50 -0800 Subject: [PATCH 11/11] cargo fmt --- compiler/noirc_errors/src/debug_info.rs | 15 +- compiler/noirc_evaluator/src/ssa.rs | 5 +- compiler/noirc_frontend/src/debug/mod.rs | 247 +++++++++--------- compiler/noirc_frontend/src/hir/mod.rs | 14 +- .../src/hir/resolution/import.rs | 12 +- .../src/monomorphization/ast.rs | 7 +- .../src/monomorphization/debug_types.rs | 8 +- .../src/monomorphization/mod.rs | 14 +- tooling/debugger/src/context.rs | 8 +- tooling/debugger/src/repl.rs | 4 +- tooling/nargo/src/artifacts/debug.rs | 10 +- tooling/nargo/src/artifacts/debug_vars.rs | 30 ++- tooling/nargo/src/artifacts/mod.rs | 2 +- tooling/nargo/src/ops/foreign_calls.rs | 20 +- 14 files changed, 202 insertions(+), 194 deletions(-) diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index dc60a2408d1..30b4a7ce997 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -4,16 +4,16 @@ use fm::FileId; use serde_with::serde_as; use serde_with::DisplayFromStr; -use std::collections::{BTreeMap,HashMap}; +use std::collections::{BTreeMap, HashMap}; use std::mem; use crate::Location; -use serde::{Deserialize, Serialize}; use noirc_printable_type::PrintableType; +use serde::{Deserialize, Serialize}; pub type Variables = Vec<(u32, (String, u32))>; pub type Types = Vec<(u32, PrintableType)>; -pub type VariableTypes = (Variables,Types); +pub type VariableTypes = (Variables, Types); #[serde_as] #[derive(Default, Debug, Clone, Deserialize, Serialize)] @@ -24,7 +24,7 @@ pub struct DebugInfo { #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, pub variables: HashMap, // var_id => (name, type_id) - pub types: HashMap, // type_id => printable type + pub types: HashMap, // type_id => printable type } /// Holds OpCodes Counts for Acir and Brillig Opcodes @@ -101,12 +101,9 @@ impl DebugInfo { } pub fn get_file_ids(&self) -> Vec { - self - .locations + self.locations .values() - .filter_map(|call_stack| { - call_stack.last().map(|location| location.file) - }) + .filter_map(|call_stack| call_stack.last().map(|location| location.file)) .collect() } } diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index f7b5a32a19f..91ddcb95eb1 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -127,10 +127,7 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new( - locations, - debug_var_types, - ); + let mut debug_info = DebugInfo::new(locations, debug_var_types); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 3aaea9a6756..70eb122ada7 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -1,12 +1,16 @@ -use std::collections::{HashMap,HashSet}; -use crate::parser::{ParsedModule,parse_program}; -use crate::{ast, parser::{Item,ItemKind}, ast::{Path,PathKind}}; +use crate::parser::{parse_program, ParsedModule}; +use crate::{ + ast, + ast::{Path, PathKind}, + parser::{Item, ItemKind}, +}; use noirc_errors::{Span, Spanned}; use std::collections::VecDeque; +use std::collections::{HashMap, HashSet}; -#[derive(Debug,Clone)] +#[derive(Debug, Clone)] pub struct DebugState { - pub variables: HashMap, // var_id => name + pub variables: HashMap, // var_id => name next_var_id: u32, scope: Vec>, pub enabled: bool, @@ -35,22 +39,26 @@ impl DebugState { fn walk_fn(&mut self, f: &mut ast::FunctionDefinition) { self.scope.push(HashSet::new()); - let pvars: Vec<(u32,ast::Ident,bool)> = f.parameters.iter() + let pvars: Vec<(u32, ast::Ident, bool)> = f + .parameters + .iter() .flat_map(|param| { - pattern_vars(¶m.pattern).iter().map(|(id,is_mut)| { - (self.insert_var(&id.0.contents), id.clone(), *is_mut) - }).collect::>() + pattern_vars(¶m.pattern) + .iter() + .map(|(id, is_mut)| (self.insert_var(&id.0.contents), id.clone(), *is_mut)) + .collect::>() }) .collect(); - let set_fn_params = pvars.iter().map(|(var_id, id, _is_mut)| { - self.wrap_assign_var(*var_id, id_expr(id)) - }).collect(); + let set_fn_params = pvars + .iter() + .map(|(var_id, id, _is_mut)| self.wrap_assign_var(*var_id, id_expr(id))) + .collect(); self.walk_scope(&mut f.body.0); // prapend fn params: - f.body.0 = vec![ set_fn_params, f.body.0.clone() ].concat(); + f.body.0 = vec![set_fn_params, f.body.0.clone()].concat(); } // Modify a vector of statements in-place, adding instrumentation for sets and drops. @@ -58,9 +66,8 @@ impl DebugState { fn walk_scope(&mut self, statements: &mut Vec) { statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); - let (ret_stmt, fn_body) = statements.split_last() - .map(|(e,b)| (e.clone(), b.to_vec())) - .unwrap_or(( + let (ret_stmt, fn_body) = + statements.split_last().map(|(e, b)| (e.clone(), b.to_vec())).unwrap_or(( ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { kind: ast::ExpressionKind::Literal(ast::Literal::Unit), @@ -68,46 +75,42 @@ impl DebugState { }), span: none_span(), }, - vec![] + vec![], )); *statements = vec![ // copy body minus the return expr: fn_body, - // assign return expr to __debug_expr: vec![match &ret_stmt.kind { - ast::StatementKind::Expression(ret_expr) => { - ast::Statement { - kind: ast::StatementKind::Let(ast::LetStatement { - pattern: ast::Pattern::Identifier(ident("__debug_expr")), - r#type: ast::UnresolvedType::unspecified(), - expression: ret_expr.clone(), - }), - span: none_span(), - } + ast::StatementKind::Expression(ret_expr) => ast::Statement { + kind: ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Identifier(ident("__debug_expr")), + r#type: ast::UnresolvedType::unspecified(), + expression: ret_expr.clone(), + }), + span: none_span(), }, _ => ret_stmt.clone(), }], - // drop fn params: - self.scope.pop().unwrap_or(HashSet::default()).iter().map(|var_id| { - self.wrap_drop_var(*var_id) - }).collect(), - + self.scope + .pop() + .unwrap_or(HashSet::default()) + .iter() + .map(|var_id| self.wrap_drop_var(*var_id)) + .collect(), // return the __debug_expr value: vec![match &ret_stmt.kind { - ast::StatementKind::Expression(_ret_expr) => { - ast::Statement { - kind: ast::StatementKind::Expression(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_expr")], - kind: PathKind::Plain, - }), - span: none_span(), + ast::StatementKind::Expression(_ret_expr) => ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident("__debug_expr")], + kind: PathKind::Plain, }), span: none_span(), - } + }), + span: none_span(), }, _ => ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { @@ -117,18 +120,19 @@ impl DebugState { span: none_span(), }, }], - ].concat(); + ] + .concat(); } pub fn insert_symbols(&mut self, module: &mut ParsedModule) { - if !self.enabled { return } - module.items.iter_mut().for_each(|item| { - match item { - Item { kind: ItemKind::Function(f), .. } => { - self.walk_fn(&mut f.def); - }, - _ => {}, + if !self.enabled { + return; + } + module.items.iter_mut().for_each(|item| match item { + Item { kind: ItemKind::Function(f), .. } => { + self.walk_fn(&mut f.def); } + _ => {} }); // this part absolutely must happen after ast traversal above // so that oracle functions don't get wrapped, resulting in infinite recursion: @@ -140,14 +144,14 @@ impl DebugState { func: Box::new(ast::Expression { kind: ast::ExpressionKind::Variable(ast::Path { segments: vec![ident("__debug_var_assign")], - kind: PathKind::Plain + kind: PathKind::Plain, }), span: none_span(), }), arguments: vec![ ast::Expression { kind: ast::ExpressionKind::Literal(ast::Literal::Integer( - (var_id as u128).into() + (var_id as u128).into(), )), span: none_span(), }, @@ -165,18 +169,14 @@ impl DebugState { func: Box::new(ast::Expression { kind: ast::ExpressionKind::Variable(ast::Path { segments: vec![ident("__debug_var_drop")], - kind: PathKind::Plain + kind: PathKind::Plain, }), span: none_span(), }), - arguments: vec![ - ast::Expression { - kind: ast::ExpressionKind::Literal(ast::Literal::Integer( - (var_id as u128).into() - )), - span: none_span(), - }, - ], + arguments: vec![ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Integer((var_id as u128).into())), + span: none_span(), + }], })); ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span: none_span() }), @@ -200,25 +200,26 @@ impl DebugState { // }; let vars = pattern_vars(&let_stmt.pattern); - let vars_pattern: Vec = vars.iter().map(|(id,is_mut)| { - if *is_mut { - ast::Pattern::Mutable( - Box::new(ast::Pattern::Identifier(id.clone())), - none_span() - ) - } else { - ast::Pattern::Identifier(id.clone()) - } - }).collect(); - let vars_exprs: Vec = vars.iter().map(|(id,_)| id_expr(id)).collect(); - - let mut block_stmts = vec![ - ast::Statement { - kind: ast::StatementKind::Let(let_stmt.clone()), - span: none_span(), - }, - ]; - block_stmts.extend(vars.iter().map(|(id,_)| { + let vars_pattern: Vec = vars + .iter() + .map(|(id, is_mut)| { + if *is_mut { + ast::Pattern::Mutable( + Box::new(ast::Pattern::Identifier(id.clone())), + none_span(), + ) + } else { + ast::Pattern::Identifier(id.clone()) + } + }) + .collect(); + let vars_exprs: Vec = vars.iter().map(|(id, _)| id_expr(id)).collect(); + + let mut block_stmts = vec![ast::Statement { + kind: ast::StatementKind::Let(let_stmt.clone()), + span: none_span(), + }]; + block_stmts.extend(vars.iter().map(|(id, _)| { let var_id = self.insert_var(&id.0.contents); self.wrap_assign_var(var_id, id_expr(id)) })); @@ -243,7 +244,11 @@ impl DebugState { } } - fn wrap_assign_statement(&mut self, assign_stmt: &ast::AssignStatement, span: &Span) -> ast::Statement { + fn wrap_assign_statement( + &mut self, + assign_stmt: &ast::AssignStatement, + span: &Span, + ) -> ast::Statement { // X = Y becomes: // X = { // let __debug_expr = Y; @@ -260,20 +265,20 @@ impl DebugState { ast::LValue::Ident(id) => { let var_id = self.insert_var(&id.0.contents); self.wrap_assign_var(var_id, id_expr(&ident("__debug_expr"))) - }, + } ast::LValue::MemberAccess { object: _object, field_name: _field_name } => { // TODO unimplemented![] - }, + } ast::LValue::Index { array: _array, index: _index } => { // TODO // TODO: also remember to self.walk(index) unimplemented![] - }, + } ast::LValue::Dereference(_lv) => { // TODO unimplemented![] - }, + } }; let ret_kind = ast::StatementKind::Expression(id_expr(&ident("__debug_expr"))); @@ -298,62 +303,62 @@ impl DebugState { ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { self.scope.push(HashSet::new()); self.walk_scope(statements); - }, + } ast::ExpressionKind::Prefix(prefix_expr) => { self.walk_expr(&mut prefix_expr.rhs); - }, + } ast::ExpressionKind::Index(index_expr) => { self.walk_expr(&mut index_expr.collection); self.walk_expr(&mut index_expr.index); - }, + } ast::ExpressionKind::Call(call_expr) => { // TODO: push a stack frame or something here? self.walk_expr(&mut call_expr.func); call_expr.arguments.iter_mut().for_each(|ref mut expr| { self.walk_expr(expr); }); - }, + } ast::ExpressionKind::MethodCall(mc_expr) => { // TODO: also push a stack frame here self.walk_expr(&mut mc_expr.object); mc_expr.arguments.iter_mut().for_each(|ref mut expr| { self.walk_expr(expr); }); - }, + } ast::ExpressionKind::Constructor(c_expr) => { c_expr.fields.iter_mut().for_each(|(_id, ref mut expr)| { self.walk_expr(expr); }); - }, + } ast::ExpressionKind::MemberAccess(ma_expr) => { self.walk_expr(&mut ma_expr.lhs); - }, + } ast::ExpressionKind::Cast(cast_expr) => { self.walk_expr(&mut cast_expr.lhs); - }, + } ast::ExpressionKind::Infix(infix_expr) => { self.walk_expr(&mut infix_expr.lhs); self.walk_expr(&mut infix_expr.rhs); - }, + } ast::ExpressionKind::If(if_expr) => { self.walk_expr(&mut if_expr.condition); self.walk_expr(&mut if_expr.consequence); if let Some(ref mut alt) = if_expr.alternative { self.walk_expr(alt); } - }, + } ast::ExpressionKind::Tuple(exprs) => { exprs.iter_mut().for_each(|ref mut expr| { self.walk_expr(expr); }); - }, + } ast::ExpressionKind::Lambda(lambda) => { self.walk_expr(&mut lambda.body); - }, + } ast::ExpressionKind::Parenthesized(expr) => { self.walk_expr(expr); - }, - _ => {}, + } + _ => {} } } @@ -382,25 +387,26 @@ impl DebugState { match &mut stmt.kind { ast::StatementKind::Let(let_stmt) => { *stmt = self.wrap_let_statement(&let_stmt, &stmt.span); - }, + } ast::StatementKind::Assign(assign_stmt) => { *stmt = self.wrap_assign_statement(&assign_stmt, &stmt.span); - }, + } ast::StatementKind::Expression(expr) => { self.walk_expr(expr); - }, + } ast::StatementKind::Semi(expr) => { self.walk_expr(expr); - }, + } ast::StatementKind::For(ref mut for_stmt) => { self.walk_for(for_stmt); - }, - _ => {}, // Constrain, Error + } + _ => {} // Constrain, Error } } fn insert_state_set_oracle(&self, module: &mut ParsedModule) { - let (program, errors) = parse_program(r#" + let (program, errors) = parse_program( + r#" #[oracle(__debug_var_assign)] unconstrained fn __debug_var_assign_oracle(_var_id: u32, _value: T) {} unconstrained fn __debug_var_assign_inner(var_id: u32, value: T) { @@ -445,31 +451,34 @@ impl DebugState { pub fn __debug_dereference_assign(var_id: u32, value: T) { __debug_dereference_assign_inner(var_id, value); } - "#); - if !errors.is_empty() { panic!("errors parsing internal oracle definitions: {errors:?}") } + "#, + ); + if !errors.is_empty() { + panic!("errors parsing internal oracle definitions: {errors:?}") + } module.items.extend(program.items); } } -fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident,bool)> { +fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { let mut vars = vec![]; - let mut stack = VecDeque::from([ (pattern,false) ]); + let mut stack = VecDeque::from([(pattern, false)]); while stack.front().is_some() { - let (pattern,is_mut) = stack.pop_front().unwrap(); + let (pattern, is_mut) = stack.pop_front().unwrap(); match pattern { ast::Pattern::Identifier(id) => { - vars.push((id.clone(),is_mut)); - }, + vars.push((id.clone(), is_mut)); + } ast::Pattern::Mutable(pattern, _) => { - stack.push_back((pattern,true)); - }, + stack.push_back((pattern, true)); + } ast::Pattern::Tuple(patterns, _) => { stack.extend(patterns.iter().map(|pattern| (pattern, false))); - }, + } ast::Pattern::Struct(_, pids, _) => { - stack.extend(pids.iter().map(|(_, pattern)| (pattern,is_mut))); + stack.extend(pids.iter().map(|(_, pattern)| (pattern, is_mut))); vars.extend(pids.iter().map(|(id, _)| (id.clone(), false))); - }, + } } } vars @@ -489,4 +498,6 @@ fn id_expr(id: &ast::Ident) -> ast::Expression { } } -fn none_span() -> Span { Span::from_str("") } +fn none_span() -> Span { + Span::from_str("") +} diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index a32c3b9bf58..1e9b23da6b9 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -7,13 +7,13 @@ pub mod type_check; #[cfg(feature = "aztec")] pub(crate) mod aztec_library; +use crate::debug::DebugState; use crate::graph::{CrateGraph, CrateId}; use crate::hir_def::function::FuncMeta; -use crate::debug::DebugState; use crate::node_interner::{FuncId, NodeInterner, StructId}; -use crate::parser::{SortedModule, ParserError}; -use def_map::{Contract, CrateDefMap, parse_file}; -use fm::{FileManager, FileId}; +use crate::parser::{ParserError, SortedModule}; +use def_map::{parse_file, Contract, CrateDefMap}; +use fm::{FileId, FileManager}; use noirc_errors::Location; use std::collections::BTreeMap; @@ -221,7 +221,11 @@ impl Context { /// Given a FileId, fetch the File, from the FileManager and parse its content, /// applying sorting and debug transforms if debug mode is enabled. - pub fn parse_file(&mut self, file_id: FileId, crate_id: CrateId) -> (SortedModule, Vec) { + pub fn parse_file( + &mut self, + file_id: FileId, + crate_id: CrateId, + ) -> (SortedModule, Vec) { let (mut ast, parsing_errors) = parse_file(&self.file_manager, file_id); if crate_id == self.root_crate_id { diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index d296ebf3b6a..6f3140a65d4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -35,13 +35,11 @@ pub struct ResolvedImport { impl From for CustomDiagnostic { fn from(error: PathResolutionError) -> Self { match error { - PathResolutionError::Unresolved(ident) => { - CustomDiagnostic::simple_error( - format!("Could not resolve '{ident}' in path"), - String::new(), - ident.span(), - ) - } + PathResolutionError::Unresolved(ident) => CustomDiagnostic::simple_error( + format!("Could not resolve '{ident}' in path"), + String::new(), + ident.span(), + ), PathResolutionError::ExternalContractUsed(ident) => CustomDiagnostic::simple_error( format!("Contract variable '{ident}' referenced from outside the contract"), "Contracts may only be referenced from within a contract".to_string(), diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 73d1d8f5351..0048b9d7a43 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -3,9 +3,8 @@ use iter_extended::vecmap; use noirc_errors::Location; use crate::{ - hir_def::function::FunctionSignature, - BinaryOpKind, Distinctness, Signedness, - monomorphization::debug_types, + hir_def::function::FunctionSignature, monomorphization::debug_types, BinaryOpKind, + Distinctness, Signedness, }; /// The monomorphized AST is expression-based, all statements are also @@ -263,7 +262,7 @@ impl Program { main_function_signature, return_distinctness, return_location, - debug_var_types + debug_var_types, } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs index 02f53ccf6b7..07bdefbc6a7 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug_types.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -1,12 +1,12 @@ -use std::collections::HashMap; use crate::hir_def::types::Type; +pub use noirc_errors::debug_info::{Types, VariableTypes, Variables}; use noirc_printable_type::PrintableType; -pub use noirc_errors::debug_info::{Variables, Types, VariableTypes}; +use std::collections::HashMap; #[derive(Debug, Clone, Default)] pub struct DebugTypes { - variables: HashMap, // var_id => (var_name, type_id) - types: HashMap, + variables: HashMap, // var_id => (var_name, type_id) + types: HashMap, next_type_id: u32, } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index d2e1d301186..f8d6466151c 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -34,8 +34,8 @@ use self::ast::{Definition, FuncId, Function, LocalId, Program}; use self::debug_types::DebugTypes; pub mod ast; -pub mod printer; pub mod debug_types; +pub mod printer; struct LambdaContext { env_ident: ast::Ident, @@ -886,15 +886,15 @@ impl<'interner> Monomorphizer<'interner> { let original_func = Box::new(self.expr(call.func)); let mut arguments = vecmap(&call.arguments, |id| self.expr(*id)); let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); - if let ( - ast::Expression::Ident(ast::Ident { name, .. }), - 2, - ) = (original_func.as_ref(), arguments.len()) { + if let (ast::Expression::Ident(ast::Ident { name, .. }), 2) = + (original_func.as_ref(), arguments.len()) + { if let ( HirExpression::Literal(HirLiteral::Integer(fe_var_id)), HirExpression::Ident(HirIdent { id, .. }), - true - ) = (&hir_arguments[0], &hir_arguments[1], name == "__debug_var_assign") { + true, + ) = (&hir_arguments[0], &hir_arguments[1], name == "__debug_var_assign") + { let var_def = self.interner.definition(*id); let var_type = self.interner.id_type(call.arguments[1]); let var_id = fe_var_id.to_u128() as u32; diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 0d7e0068064..6a240f98515 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -8,7 +8,7 @@ use acvm::{BlackBoxFunctionSolver, FieldElement}; use nargo::artifacts::debug::{DebugArtifact, DebugVars}; use nargo::errors::{ExecutionError, Location}; -use nargo::ops::{DefaultForeignCallExecutor}; +use nargo::ops::DefaultForeignCallExecutor; use nargo::NargoError; use std::collections::{hash_set::Iter, HashSet}; @@ -125,10 +125,8 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } fn handle_foreign_call(&mut self, foreign_call: ForeignCallWaitInfo) -> DebugCommandResult { - let foreign_call_result = self.foreign_call_executor.execute_with_debug_vars( - &foreign_call, - &mut self.debug_vars, - ); + let foreign_call_result = + self.foreign_call_executor.execute_with_debug_vars(&foreign_call, &mut self.debug_vars); match foreign_call_result { Ok(foreign_call_result) => { self.acvm.resolve_pending_foreign_call(foreign_call_result); diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 0a46c73d664..f99171bd5c3 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -85,7 +85,9 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { let locations = self.debug_artifact.debug_symbols[0].opcode_location(location); let Some(locations) = locations else { return }; for loc in locations { - if loc.span.start() == loc.span.end() { continue } + if loc.span.start() == loc.span.end() { + continue; + } self.print_location_path(loc); let loc_line_index = self.debug_artifact.location_line_index(loc).unwrap(); diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index e95e721d388..afa62df51ae 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -4,12 +4,12 @@ use noirc_errors::{debug_info::DebugInfo, Location}; use noirc_evaluator::errors::SsaReport; use serde::{Deserialize, Serialize}; use std::{ - collections::{BTreeMap,BTreeSet}, + collections::{BTreeMap, BTreeSet}, ops::Range, }; -use fm::{FileId, FileManager, PathString}; pub use super::debug_vars::DebugVars; +use fm::{FileId, FileManager, PathString}; /// A Debug Artifact stores, for a given program, the debug info for every function /// along with a map of file Id to the source code so locations in debug info can be mapped to source code they point to. @@ -24,10 +24,8 @@ impl DebugArtifact { pub fn new(debug_symbols: Vec, file_manager: &FileManager) -> Self { let mut file_map = BTreeMap::new(); - let file_ids: BTreeSet = debug_symbols - .iter() - .flat_map(|debug_info| debug_info.get_file_ids()) - .collect(); + let file_ids: BTreeSet = + debug_symbols.iter().flat_map(|debug_info| debug_info.get_file_ids()).collect(); for file_id in file_ids.iter() { let file_source = file_manager.fetch_file(*file_id).source(); diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 72e5b1acde2..b357a696c4b 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -1,10 +1,10 @@ -use std::collections::{HashMap,HashSet}; use acvm::acir::brillig::Value; use noirc_printable_type::PrintableType; +use std::collections::{HashMap, HashSet}; #[derive(Debug, Default, Clone)] pub struct DebugVars { - id_to_name: HashMap, + id_to_name: HashMap, active: HashSet, id_to_value: HashMap, // TODO: something more sophisticated for lexical levels id_to_type: HashMap, @@ -19,18 +19,20 @@ impl DebugVars { } pub fn get_variables<'a>(&'a self) -> Vec<(&'a str, &'a Value, &'a PrintableType)> { - self.active.iter().filter_map(|var_id| { - self.id_to_name.get(var_id) - .and_then(|name| { - self.id_to_value.get(var_id).map(|value| (name, value)) - }) - .and_then(|(name, value)| { - self.id_to_type.get(var_id).map(|type_id| (name, value, type_id)) - }) - .and_then(|(name, value, type_id)| { - self.types.get(type_id).map(|ptype| (name.as_str(), value, ptype)) - }) - }).collect() + self.active + .iter() + .filter_map(|var_id| { + self.id_to_name + .get(var_id) + .and_then(|name| self.id_to_value.get(var_id).map(|value| (name, value))) + .and_then(|(name, value)| { + self.id_to_type.get(var_id).map(|type_id| (name, value, type_id)) + }) + .and_then(|(name, value, type_id)| { + self.types.get(type_id).map(|ptype| (name.as_str(), value, ptype)) + }) + }) + .collect() } pub fn insert_variables(&mut self, vars: &HashMap) { diff --git a/tooling/nargo/src/artifacts/mod.rs b/tooling/nargo/src/artifacts/mod.rs index 2d8e1fcef39..c7b3736f90b 100644 --- a/tooling/nargo/src/artifacts/mod.rs +++ b/tooling/nargo/src/artifacts/mod.rs @@ -5,5 +5,5 @@ //! to generate them using these artifacts as a starting point. pub mod contract; pub mod debug; -pub mod program; mod debug_vars; +pub mod program; diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 85c7a96ca71..6e2a50753f0 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -1,10 +1,10 @@ +use crate::artifacts::debug::DebugVars; use acvm::{ acir::brillig::{ForeignCallParam, ForeignCallResult, Value}, pwg::ForeignCallWaitInfo, }; use iter_extended::vecmap; use noirc_printable_type::{decode_string_value, ForeignCallError, PrintableValueDisplay}; -use crate::artifacts::debug::DebugVars; pub trait ForeignCallExecutor { fn execute( @@ -152,7 +152,8 @@ impl DefaultForeignCallExecutor { Some(ds), ForeignCallParam::Single(var_id_value), ForeignCallParam::Single(value), - ) = (debug_vars, fcp_var_id, fcp_value) { + ) = (debug_vars, fcp_var_id, fcp_value) + { let var_id = var_id_value.to_u128() as u32; ds.assign(var_id, value.clone()); } @@ -160,10 +161,8 @@ impl DefaultForeignCallExecutor { } Some(ForeignCall::DebugVarDrop) => { let fcp_var_id = &foreign_call.inputs[0]; - if let ( - Some(ds), - ForeignCallParam::Single(var_id_value), - ) = (debug_vars, fcp_var_id) { + if let (Some(ds), ForeignCallParam::Single(var_id_value)) = (debug_vars, fcp_var_id) + { let var_id = var_id_value.to_u128() as u32; ds.drop(var_id); } @@ -178,7 +177,8 @@ impl DefaultForeignCallExecutor { ForeignCallParam::Single(var_id_value), ForeignCallParam::Single(member_id_value), ForeignCallParam::Single(value), - ) = (debug_vars, fcp_var_id, fcp_member_id, fcp_value) { + ) = (debug_vars, fcp_var_id, fcp_member_id, fcp_value) + { let var_id = var_id_value.to_u128() as u32; let member_id = member_id_value.to_u128() as u32; ds.assign_member(var_id, member_id, value.clone()); @@ -194,7 +194,8 @@ impl DefaultForeignCallExecutor { ForeignCallParam::Single(var_id_value), ForeignCallParam::Single(index_value), ForeignCallParam::Single(value), - ) = (debug_vars, fcp_var_id, fcp_index, fcp_value) { + ) = (debug_vars, fcp_var_id, fcp_index, fcp_value) + { let var_id = var_id_value.to_u128() as u32; let index = index_value.to_u128() as u64; ds.assign_index(var_id, index, value.clone()); @@ -208,7 +209,8 @@ impl DefaultForeignCallExecutor { Some(ds), ForeignCallParam::Single(var_id_value), ForeignCallParam::Single(value), - ) = (debug_vars, fcp_var_id, fcp_value) { + ) = (debug_vars, fcp_var_id, fcp_value) + { let var_id = var_id_value.to_u128() as u32; ds.assign_deref(var_id, value.clone()); }