From 87355b88445bd2f477ad5b5aed7b97670d789383 Mon Sep 17 00:00:00 2001 From: synthia Date: Mon, 22 Jan 2024 14:48:04 -0800 Subject: [PATCH 01/21] feat: Include function stack info in debug vars command --- compiler/noirc_frontend/src/debug/mod.rs | 76 +++++++++++++++++-- compiler/noirc_frontend/src/hir_def/types.rs | 8 +- .../src/monomorphization/debug.rs | 26 +++++++ .../src/monomorphization/debug_types.rs | 6 +- .../src/monomorphization/mod.rs | 27 +++++++ compiler/noirc_printable_type/src/lib.rs | 11 ++- tooling/debugger/src/context.rs | 2 +- tooling/debugger/src/foreign_calls.rs | 20 ++++- tooling/debugger/src/repl.rs | 18 +++-- tooling/debugger/src/source_code_printer.rs | 9 ++- tooling/nargo/src/artifacts/debug_vars.rs | 52 +++++++++---- 11 files changed, 214 insertions(+), 41 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 9f182d2baa2..9de3ba7d2e7 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -77,7 +77,14 @@ impl DebugInstrumenter { } fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) { + let func_name = &func.name.0.contents; + if is_instrumentation_method(func_name) { + return; + } self.scope.push(HashMap::default()); + let func_name_span = func.name.span(); + let fn_id = self.insert_var(func_name); + let enter_fn = Self::call_fn("enter", vec![uint_expr(fn_id.0 as u128, func_name_span)], func_name_span); let set_fn_params = func .parameters @@ -93,15 +100,19 @@ impl DebugInstrumenter { }) .collect(); - self.walk_scope(&mut func.body.0, func.span); + self.walk_scope(&mut func.body.0, func.span, Some(fn_id)); // prepend fn params: - func.body.0 = vec![set_fn_params, func.body.0.clone()].concat(); + func.body.0 = vec![ + vec![enter_fn], + set_fn_params, + func.body.0.clone(), + ].concat(); } // 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, span: Span) { + fn walk_scope(&mut self, statements: &mut Vec, span: Span, opt_fn_id: Option) { statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); // extract and save the return value from the scope if there is one @@ -134,6 +145,11 @@ impl DebugInstrumenter { let drop_vars_stmts = scope_vars.values().map(|var_id| build_drop_var_stmt(*var_id, span)); statements.extend(drop_vars_stmts); + // exit fn for fn scopes + if let Some(fn_id) = opt_fn_id { + statements.push(Self::call_fn("exit", vec![uint_expr(fn_id.0 as u128, span)], span)); + } + // return the saved value in __debug_expr, or unit otherwise let last_stmt = if has_ret_expr { ast::Statement { @@ -159,6 +175,21 @@ impl DebugInstrumenter { statements.push(last_stmt); } + fn call_fn(fname: &str, arguments: Vec, span: Span) -> ast::Statement { + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident(&format!["__debug_fn_{fname}"], span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments, + })); + ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } + } + fn walk_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; @@ -310,7 +341,7 @@ impl DebugInstrumenter { match &mut expr.kind { ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { self.scope.push(HashMap::default()); - self.walk_scope(statements, expr.span); + self.walk_scope(statements, expr.span, None); } ast::ExpressionKind::Prefix(prefix_expr) => { self.walk_expr(&mut prefix_expr.rhs); @@ -422,6 +453,8 @@ impl DebugInstrumenter { use dep::__debug::{{ __debug_var_assign, __debug_var_drop, + __debug_fn_enter, + __debug_fn_exit, __debug_dereference_assign, {member_assigns}, }};"# @@ -446,14 +479,32 @@ pub fn build_debug_crate_file() -> String { } #[oracle(__debug_var_drop)] - unconstrained fn __debug_var_drop_oracle(_var_id: u32) {} - unconstrained fn __debug_var_drop_inner(var_id: u32) { + unconstrained fn __debug_var_drop_oracle(_var_id: u32) {} + unconstrained fn __debug_var_drop_inner(var_id: u32) { __debug_var_drop_oracle(var_id); } - pub fn __debug_var_drop(var_id: u32) { + pub fn __debug_var_drop(var_id: u32) { __debug_var_drop_inner(var_id); } + #[oracle(__debug_fn_enter)] + unconstrained fn __debug_fn_enter_oracle(_fn_id: u32) {} + unconstrained fn __debug_fn_enter_inner(var_id: u32) { + __debug_fn_enter_oracle(var_id); + } + pub fn __debug_fn_enter(var_id: u32) { + __debug_fn_enter_inner(var_id); + } + + #[oracle(__debug_fn_exit)] + unconstrained fn __debug_fn_exit_oracle(_fn_id: u32) {} + unconstrained fn __debug_fn_exit_inner(var_id: u32) { + __debug_fn_exit_oracle(var_id); + } + pub fn __debug_fn_exit(var_id: u32) { + __debug_fn_exit_inner(var_id); + } + #[oracle(__debug_dereference_assign)] unconstrained fn __debug_dereference_assign_oracle(_var_id: u32, _value: T) {} unconstrained fn __debug_dereference_assign_inner(var_id: u32, value: T) { @@ -605,3 +656,14 @@ fn sint_expr(x: i128, span: Span) -> ast::Expression { span, } } + +fn is_instrumentation_method(fname: &str) -> bool { + match fname { + "__debug_var_assign" + | "__debug_var_drop" + | "__debug_dereference_assign" + | "__debug_fn_enter" + | "__debug_fn_exit" => true, + _ => fname.starts_with("__debug_member_assign_") + } +} diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 00e24de279b..9b037b51a8f 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1651,8 +1651,12 @@ impl From<&Type> for PrintableType { Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(_, _, env) => { - PrintableType::Function { env: Box::new(env.as_ref().into()) } + Type::Function(args, _, env) => { + PrintableType::Function { + name: "?".to_string(), + arguments: args.iter().map(|arg| ("?".to_string(), arg.into())).collect(), + env: Box::new(env.as_ref().into()), + } } Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index d36816e3d37..b4c9573ba4a 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -46,6 +46,8 @@ impl<'interner> Monomorphizer<'interner> { self.patch_debug_var_assign(call, arguments); } else if name == "__debug_var_drop" { self.patch_debug_var_drop(call, arguments); + } else if name == "__debug_fn_enter" { + self.patch_debug_fn_enter(call, arguments); } else if let Some(arity) = name.strip_prefix(DEBUG_MEMBER_ASSIGN_PREFIX) { let arity = arity.parse::().expect("failed to parse member assign arity"); self.patch_debug_member_assign(call, arguments, arity); @@ -168,6 +170,30 @@ impl<'interner> Monomorphizer<'interner> { arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); } + /// Update instrumentation code to track function enter and exit. + fn patch_debug_fn_enter(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) { + let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); + let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT); + let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else { + unreachable!("Missing source_var_id in __debug_fn_enter call"); + }; + let source_var_id = source_var_id.to_u128().into(); + let func_id = self.current_function_id.expect("current function not set"); + let fn_meta = self.interner.function_meta(&func_id); + let fn_name = self.interner.definition(fn_meta.name.id).name.clone(); + let ptype = PrintableType::Function { + name: fn_name.clone(), + arguments: fn_meta.parameters.iter().map(|(arg_pattern, arg_type, _arg_vis)| { + let arg_str = self.pattern_to_string(arg_pattern); + (arg_str, arg_type.follow_bindings().into()) + }).collect(), + env: Box::new(PrintableType::Tuple { types: vec![] }), + }; + let fn_id = self.debug_type_tracker.insert_var_printable(source_var_id, ptype); + let interned_var_id = self.intern_var_id(fn_id, &call.location); + arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); + } + fn intern_var_id(&mut self, var_id: DebugVarId, location: &Location) -> ExprId { let var_id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false); let expr_id = self.interner.push_expr(HirExpression::Literal(var_id_literal)); diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs index fea073d394f..62b91a1a68e 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug_types.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -84,11 +84,15 @@ impl DebugTypeTracker { } pub fn insert_var(&mut self, source_var_id: SourceVarId, var_type: Type) -> DebugVarId { + let ptype: PrintableType = var_type.follow_bindings().into(); + self.insert_var_printable(source_var_id, ptype) + } + + pub fn insert_var_printable(&mut self, source_var_id: SourceVarId, ptype: PrintableType) -> DebugVarId { if !self.source_variables.contains_key(&source_var_id) { unreachable!("cannot find source debug variable {source_var_id:?}"); } - let ptype: PrintableType = var_type.follow_bindings().into(); let type_id = self.types_reverse.get(&ptype).copied().unwrap_or_else(|| { let type_id = DebugTypeId(self.next_type_id); self.next_type_id += 1; diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 21c095eb877..322112abe75 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -83,6 +83,8 @@ struct Monomorphizer<'interner> { return_location: Option, debug_type_tracker: DebugTypeTracker, + + current_function_id: Option, } type HirType = crate::Type; @@ -154,6 +156,7 @@ impl<'interner> Monomorphizer<'interner> { is_range_loop: false, return_location: None, debug_type_tracker, + current_function_id: None, } } @@ -249,6 +252,8 @@ impl<'interner> Monomorphizer<'interner> { } fn function(&mut self, f: node_interner::FuncId, id: FuncId) { + self.current_function_id = Some(f); + if let Some((self_type, trait_id)) = self.interner.get_function_trait(&f) { let the_trait = self.interner.get_trait(trait_id); the_trait.self_type_typevar.force_bind(self_type); @@ -1590,6 +1595,28 @@ impl<'interner> Monomorphizer<'interner> { bindings } + + fn pattern_to_string(&self, pat: &HirPattern) -> String { + match pat { + HirPattern::Identifier(hir_id) => self.interner.definition(hir_id.id).name.clone(), + HirPattern::Mutable(mpat, _) => format!("mut {}", self.pattern_to_string(mpat)), + HirPattern::Tuple(pats, _) => format!("({})", pats.iter() + .map(|tpat| self.pattern_to_string(tpat)).collect::>().join(",")), + HirPattern::Struct(Type::Struct(sh_stype, _field_types), fields, _) => { + let stype = sh_stype.borrow(); + format!( + "{} {{ {} }}", + &stype.name.0.contents, + fields.iter().map(|(id,pat)| { + format!("{}: {}", &id.0.contents, self.pattern_to_string(pat)) + }).collect::>().join(", "), + ) + }, + HirPattern::Struct(typ, _, _) => { + panic!("unexpected type of struct: {typ:?}"); + }, + } + } } fn unwrap_tuple_type(typ: &HirType) -> Vec { diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 24f4f275a14..ace05811e99 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -33,6 +33,8 @@ pub enum PrintableType { length: u64, }, Function { + name: String, + arguments: Vec<(String, PrintableType)>, env: Box, }, MutableReference { @@ -176,8 +178,11 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str("false"); } } - (PrintableValue::Field(_), PrintableType::Function { .. }) => { - output.push_str("<>"); + (PrintableValue::Field(_), PrintableType::Function { name, arguments, .. }) => { + output.push_str(&format!( + "<>", + arguments.iter().map(|(var_name,_)| { var_name }) + )); } (_, PrintableType::MutableReference { .. }) => { output.push_str("<>"); @@ -350,7 +355,7 @@ pub fn decode_value( PrintableValue::Struct(struct_map) } - PrintableType::Function { env } => { + PrintableType::Function { env, .. } => { let field_element = field_iterator.next().unwrap(); let func_ref = PrintableValue::Field(field_element); // we want to consume the fields from the environment, but for now they are not actually printed diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 5ab2c63c365..74e7cb17092 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -467,7 +467,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } - pub(super) fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + pub(super) fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { return self.foreign_call_executor.get_variables(); } diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 01676adfef3..e24dad791f1 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -14,6 +14,8 @@ pub(crate) enum DebugForeignCall { VarDrop, MemberAssign(u32), DerefAssign, + FnEnter, + FnExit, } impl DebugForeignCall { @@ -28,13 +30,15 @@ impl DebugForeignCall { "__debug_var_assign" => Some(DebugForeignCall::VarAssign), "__debug_var_drop" => Some(DebugForeignCall::VarDrop), "__debug_deref_assign" => Some(DebugForeignCall::DerefAssign), + "__debug_fn_enter" => Some(DebugForeignCall::FnEnter), + "__debug_fn_exit" => Some(DebugForeignCall::FnExit), _ => None, } } } pub trait DebugForeignCallExecutor: ForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)>; + fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)>; } pub struct DefaultDebugForeignCallExecutor { @@ -65,7 +69,7 @@ impl DefaultDebugForeignCallExecutor { } impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { self.debug_vars.get_variables() } } @@ -132,6 +136,18 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { } Ok(ForeignCallResult::default().into()) } + Some(DebugForeignCall::FnEnter) => { + let fcp_fn_id = &foreign_call.inputs[0]; + let ForeignCallParam::Single(fn_id_value) = fcp_fn_id + else { panic!("unexpected foreign call parameter in fn enter: {fcp_fn_id:?}") }; + let fn_id = debug_var_id(fn_id_value); + self.debug_vars.push_fn(fn_id); + Ok(ForeignCallResult::default().into()) + } + Some(DebugForeignCall::FnExit) => { + self.debug_vars.pop_fn(); + Ok(ForeignCallResult::default().into()) + } None => self.executor.execute(foreign_call), } } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 8441dbde9be..d3ca572ce85 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -77,7 +77,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } let locations = self.context.get_source_location_for_opcode_location(&location); - print_source_code_location(self.debug_artifact, &locations); + print_source_code_location(&self.context, self.debug_artifact, &locations); } } } @@ -102,7 +102,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } let locations = self.context.get_source_location_for_opcode_location(location); - print_source_code_location(self.debug_artifact, &locations); + print_source_code_location(&self.context, self.debug_artifact, &locations); } pub fn show_current_call_stack(&self) { @@ -337,11 +337,13 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } pub fn show_vars(&self) { - let vars = self.context.get_variables(); - for (var_name, value, var_type) in vars.iter() { - let printable_value = - PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); - println!("{var_name}:{var_type:?} = {}", printable_value); + for (fname, params, vars) in self.context.get_variables() { + println!("{fname}({})", params.join(", ")); + for (var_name, value, var_type) in vars.iter() { + let printable_value = + PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); + println!(" {var_name}:{var_type:?} = {}", printable_value); + } } } @@ -530,7 +532,7 @@ pub fn run( .add( "vars", command! { - "show variable values available at this point in execution", + "show variables for each function scope available at this point in execution", () => || { ref_context.borrow_mut().show_vars(); Ok(CommandStatus::Done) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index b5ffdb12d01..d107751f94d 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -1,8 +1,10 @@ +use acvm::BlackBoxFunctionSolver; use codespan_reporting::files::Files; use nargo::artifacts::debug::DebugArtifact; use noirc_errors::Location; use owo_colors::OwoColorize; use std::ops::Range; +use crate::context::DebugContext; #[derive(Debug, PartialEq)] enum PrintedLine<'a> { @@ -30,10 +32,15 @@ struct LocationPrintContext { // Given a DebugArtifact and an OpcodeLocation, prints all the source code // locations the OpcodeLocation maps to, with some surrounding context and // visual aids to highlight the location itself. -pub(crate) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { +pub(crate) fn print_source_code_location<'a, B: BlackBoxFunctionSolver>( + context: &DebugContext<'a, B>, + debug_artifact: &DebugArtifact, + locations: &[Location], +) { let locations = locations.iter(); for loc in locations { + if context.is_source_location_in_debug_module(loc) { continue } print_location_path(debug_artifact, *loc); let lines = render_location(debug_artifact, loc); diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index b5559ca53c8..c0499933e25 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -9,24 +9,36 @@ use std::collections::{HashMap, HashSet}; pub struct DebugVars { variables: HashMap, types: HashMap, - active: HashSet, values: HashMap, + frames: Vec<(DebugVarId, HashSet)>, } impl DebugVars { - pub fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { - self.active - .iter() - .filter_map(|var_id| { - self.variables - .get(var_id) - .and_then(|debug_var| { - let Some(value) = self.values.get(var_id) else { return None; }; - let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; - Some((debug_var.name.as_str(), value, ptype)) - }) + pub fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + self.frames.iter().map(|(fn_id, frame)| { + let fn_type_id = &self.variables.get(fn_id) + .expect("failed to find type for fn_id={fn_id:?}") + .debug_type_id; + let fn_type = self.types.get(fn_type_id) + .expect(&format!("failed to get function type for fn_type_id={fn_type_id:?}")); + let PrintableType::Function { name, arguments, .. } = fn_type + else { panic!("unexpected function type {fn_type:?}") }; + let params: Vec<&str> = arguments.iter().map(|(var_name,_)| var_name.as_str()).collect(); + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame.iter() + .filter_map(|var_id| { self.lookup_var(*var_id) }) + .collect(); + (name.as_str(), params, vars) + }).collect() + } + + fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableValue, &PrintableType)> { + self.variables + .get(&var_id) + .and_then(|debug_var| { + let Some(value) = self.values.get(&var_id) else { return None; }; + let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; + Some((debug_var.name.as_str(), value, ptype)) }) - .collect() } pub fn insert_variables(&mut self, vars: &DebugVariables) { @@ -38,7 +50,7 @@ impl DebugVars { } pub fn assign_var(&mut self, var_id: DebugVarId, values: &[Value]) { - self.active.insert(var_id); + self.frames.last_mut().expect("unexpected empty stack frames").1.insert(var_id); let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; let ptype = self.types.get(type_id).unwrap(); self.values.insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); @@ -100,7 +112,7 @@ impl DebugVars { }; } *cursor = decode_value(&mut values.iter().map(|v| v.to_field()), cursor_type); - self.active.insert(var_id); + self.frames.last_mut().expect("unexpected empty stack frames").1.insert(var_id); } pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[Value]) { @@ -112,6 +124,14 @@ impl DebugVars { } pub fn drop_var(&mut self, var_id: DebugVarId) { - self.active.remove(&var_id); + self.frames.last_mut().expect("unexpected empty stack frames").1.remove(&var_id); + } + + pub fn push_fn(&mut self, fn_id: DebugVarId) { + self.frames.push((fn_id, HashSet::default())); + } + + pub fn pop_fn(&mut self) { + self.frames.pop(); } } From 9a057ccfebe68896d3b86daff1d14aabd0329071 Mon Sep 17 00:00:00 2001 From: synthia Date: Wed, 24 Jan 2024 11:24:23 -0800 Subject: [PATCH 02/21] fix: use new get_variables() signature in dap debugger --- tooling/debugger/src/dap.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index dd9a30d50da..8f60cf6bbe8 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -601,6 +601,9 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { let mut variables: Vec<_> = self .context .get_variables() + .last() + .expect("no stack frame available") + .2 .iter() .map(|(name, value, _var_type)| Variable { name: String::from(*name), From 22be9a3306fac5aad7af20e641abe367061b10dd Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 25 Jan 2024 16:11:21 +0100 Subject: [PATCH 03/21] Use frame function name in stacktrace --- tooling/debugger/src/dap.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index 8f60cf6bbe8..02f5e927c0a 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -229,6 +229,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_stack_trace(&self) -> Vec { + let stack_frames = self.context.get_variables(); + self.context .get_source_call_stack() .iter() @@ -238,9 +240,15 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { self.debug_artifact.location_line_number(*source_location).unwrap(); let column_number = self.debug_artifact.location_column_number(*source_location).unwrap(); + + let name = match stack_frames.get(index) { + Some(frame) => format!("{} {}", frame.0, index), + None => format!("frame #{index}"), + }; + StackFrame { id: index as i64, - name: format!("frame #{index}"), + name, source: Some(Source { path: self.debug_artifact.file_map[&source_location.file] .path From 434fc9449c54aea14e12e82aefce1e3cc978c0da Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 25 Jan 2024 16:11:34 +0100 Subject: [PATCH 04/21] cargo fmt --- compiler/noirc_frontend/src/debug/mod.rs | 21 +++++--- compiler/noirc_frontend/src/hir_def/types.rs | 12 ++--- .../src/monomorphization/debug.rs | 12 +++-- .../src/monomorphization/debug_types.rs | 6 ++- .../src/monomorphization/mod.rs | 23 ++++++--- compiler/noirc_printable_type/src/lib.rs | 2 +- tooling/debugger/src/context.rs | 4 +- tooling/debugger/src/foreign_calls.rs | 7 ++- tooling/debugger/src/source_code_printer.rs | 6 ++- tooling/nargo/src/artifacts/debug_vars.rs | 49 +++++++++++-------- 10 files changed, 88 insertions(+), 54 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 9de3ba7d2e7..23d3e5a26ec 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -84,7 +84,11 @@ impl DebugInstrumenter { self.scope.push(HashMap::default()); let func_name_span = func.name.span(); let fn_id = self.insert_var(func_name); - let enter_fn = Self::call_fn("enter", vec![uint_expr(fn_id.0 as u128, func_name_span)], func_name_span); + let enter_fn = Self::call_fn( + "enter", + vec![uint_expr(fn_id.0 as u128, func_name_span)], + func_name_span, + ); let set_fn_params = func .parameters @@ -103,16 +107,17 @@ impl DebugInstrumenter { self.walk_scope(&mut func.body.0, func.span, Some(fn_id)); // prepend fn params: - func.body.0 = vec![ - vec![enter_fn], - set_fn_params, - func.body.0.clone(), - ].concat(); + func.body.0 = vec![vec![enter_fn], set_fn_params, func.body.0.clone()].concat(); } // 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, span: Span, opt_fn_id: Option) { + fn walk_scope( + &mut self, + statements: &mut Vec, + span: Span, + opt_fn_id: Option, + ) { statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); // extract and save the return value from the scope if there is one @@ -664,6 +669,6 @@ fn is_instrumentation_method(fname: &str) -> bool { | "__debug_dereference_assign" | "__debug_fn_enter" | "__debug_fn_exit" => true, - _ => fname.starts_with("__debug_member_assign_") + _ => fname.starts_with("__debug_member_assign_"), } } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 9b037b51a8f..063969f0fa2 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1651,13 +1651,11 @@ impl From<&Type> for PrintableType { Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(args, _, env) => { - PrintableType::Function { - name: "?".to_string(), - arguments: args.iter().map(|arg| ("?".to_string(), arg.into())).collect(), - env: Box::new(env.as_ref().into()), - } - } + Type::Function(args, _, env) => PrintableType::Function { + name: "?".to_string(), + arguments: args.iter().map(|arg| ("?".to_string(), arg.into())).collect(), + env: Box::new(env.as_ref().into()), + }, Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index b4c9573ba4a..b93ef6a9181 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -183,10 +183,14 @@ impl<'interner> Monomorphizer<'interner> { let fn_name = self.interner.definition(fn_meta.name.id).name.clone(); let ptype = PrintableType::Function { name: fn_name.clone(), - arguments: fn_meta.parameters.iter().map(|(arg_pattern, arg_type, _arg_vis)| { - let arg_str = self.pattern_to_string(arg_pattern); - (arg_str, arg_type.follow_bindings().into()) - }).collect(), + arguments: fn_meta + .parameters + .iter() + .map(|(arg_pattern, arg_type, _arg_vis)| { + let arg_str = self.pattern_to_string(arg_pattern); + (arg_str, arg_type.follow_bindings().into()) + }) + .collect(), env: Box::new(PrintableType::Tuple { types: vec![] }), }; let fn_id = self.debug_type_tracker.insert_var_printable(source_var_id, ptype); diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs index 62b91a1a68e..f54e97bd33e 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug_types.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -88,7 +88,11 @@ impl DebugTypeTracker { self.insert_var_printable(source_var_id, ptype) } - pub fn insert_var_printable(&mut self, source_var_id: SourceVarId, ptype: PrintableType) -> DebugVarId { + pub fn insert_var_printable( + &mut self, + source_var_id: SourceVarId, + ptype: PrintableType, + ) -> DebugVarId { if !self.source_variables.contains_key(&source_var_id) { unreachable!("cannot find source debug variable {source_var_id:?}"); } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 322112abe75..79a2ced7a30 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1600,21 +1600,30 @@ impl<'interner> Monomorphizer<'interner> { match pat { HirPattern::Identifier(hir_id) => self.interner.definition(hir_id.id).name.clone(), HirPattern::Mutable(mpat, _) => format!("mut {}", self.pattern_to_string(mpat)), - HirPattern::Tuple(pats, _) => format!("({})", pats.iter() - .map(|tpat| self.pattern_to_string(tpat)).collect::>().join(",")), + HirPattern::Tuple(pats, _) => format!( + "({})", + pats.iter() + .map(|tpat| self.pattern_to_string(tpat)) + .collect::>() + .join(",") + ), HirPattern::Struct(Type::Struct(sh_stype, _field_types), fields, _) => { let stype = sh_stype.borrow(); format!( "{} {{ {} }}", &stype.name.0.contents, - fields.iter().map(|(id,pat)| { - format!("{}: {}", &id.0.contents, self.pattern_to_string(pat)) - }).collect::>().join(", "), + fields + .iter() + .map(|(id, pat)| { + format!("{}: {}", &id.0.contents, self.pattern_to_string(pat)) + }) + .collect::>() + .join(", "), ) - }, + } HirPattern::Struct(typ, _, _) => { panic!("unexpected type of struct: {typ:?}"); - }, + } } } } diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index ace05811e99..c78722501db 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -181,7 +181,7 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { (PrintableValue::Field(_), PrintableType::Function { name, arguments, .. }) => { output.push_str(&format!( "<>", - arguments.iter().map(|(var_name,_)| { var_name }) + arguments.iter().map(|(var_name, _)| { var_name }) )); } (_, PrintableType::MutableReference { .. }) => { diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 74e7cb17092..9a5b48e5fe5 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -467,7 +467,9 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } - pub(super) fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + pub(super) fn get_variables( + &self, + ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { return self.foreign_call_executor.get_variables(); } diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index e24dad791f1..b937d87929e 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -38,7 +38,8 @@ impl DebugForeignCall { } pub trait DebugForeignCallExecutor: ForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)>; + fn get_variables(&self) + -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)>; } pub struct DefaultDebugForeignCallExecutor { @@ -69,7 +70,9 @@ impl DefaultDebugForeignCallExecutor { } impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + fn get_variables( + &self, + ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { self.debug_vars.get_variables() } } diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index d107751f94d..84ccf460c26 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -1,10 +1,10 @@ +use crate::context::DebugContext; use acvm::BlackBoxFunctionSolver; use codespan_reporting::files::Files; use nargo::artifacts::debug::DebugArtifact; use noirc_errors::Location; use owo_colors::OwoColorize; use std::ops::Range; -use crate::context::DebugContext; #[derive(Debug, PartialEq)] enum PrintedLine<'a> { @@ -40,7 +40,9 @@ pub(crate) fn print_source_code_location<'a, B: BlackBoxFunctionSolver>( let locations = locations.iter(); for loc in locations { - if context.is_source_location_in_debug_module(loc) { continue } + if context.is_source_location_in_debug_module(loc) { + continue; + } print_location_path(debug_artifact, *loc); let lines = render_location(debug_artifact, loc); diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index c0499933e25..6c59596b86b 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -14,31 +14,38 @@ pub struct DebugVars { } impl DebugVars { - pub fn get_variables(&self) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { - self.frames.iter().map(|(fn_id, frame)| { - let fn_type_id = &self.variables.get(fn_id) - .expect("failed to find type for fn_id={fn_id:?}") - .debug_type_id; - let fn_type = self.types.get(fn_type_id) - .expect(&format!("failed to get function type for fn_type_id={fn_type_id:?}")); - let PrintableType::Function { name, arguments, .. } = fn_type + pub fn get_variables( + &self, + ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + self.frames + .iter() + .map(|(fn_id, frame)| { + let fn_type_id = &self + .variables + .get(fn_id) + .expect("failed to find type for fn_id={fn_id:?}") + .debug_type_id; + let fn_type = self + .types + .get(fn_type_id) + .expect(&format!("failed to get function type for fn_type_id={fn_type_id:?}")); + let PrintableType::Function { name, arguments, .. } = fn_type else { panic!("unexpected function type {fn_type:?}") }; - let params: Vec<&str> = arguments.iter().map(|(var_name,_)| var_name.as_str()).collect(); - let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame.iter() - .filter_map(|var_id| { self.lookup_var(*var_id) }) - .collect(); - (name.as_str(), params, vars) - }).collect() + let params: Vec<&str> = + arguments.iter().map(|(var_name, _)| var_name.as_str()).collect(); + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = + frame.iter().filter_map(|var_id| self.lookup_var(*var_id)).collect(); + (name.as_str(), params, vars) + }) + .collect() } fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableValue, &PrintableType)> { - self.variables - .get(&var_id) - .and_then(|debug_var| { - let Some(value) = self.values.get(&var_id) else { return None; }; - let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; - Some((debug_var.name.as_str(), value, ptype)) - }) + self.variables.get(&var_id).and_then(|debug_var| { + let Some(value) = self.values.get(&var_id) else { return None; }; + let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; + Some((debug_var.name.as_str(), value, ptype)) + }) } pub fn insert_variables(&mut self, vars: &DebugVariables) { From 95c4a684bb1b5d0b3c5c9cd85ce6371470fa10bb Mon Sep 17 00:00:00 2001 From: synthia Date: Thu, 25 Jan 2024 11:25:52 -0800 Subject: [PATCH 05/21] fix: pass span through to enter/exit --- compiler/noirc_frontend/src/debug/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 23d3e5a26ec..014d495f045 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -86,8 +86,8 @@ impl DebugInstrumenter { let fn_id = self.insert_var(func_name); let enter_fn = Self::call_fn( "enter", - vec![uint_expr(fn_id.0 as u128, func_name_span)], - func_name_span, + vec![uint_expr(fn_id.0 as u128, func.span)], + func.span, ); let set_fn_params = func From fdb8e7999fb4ce3b790805b85a3d587a05104ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Thu, 25 Jan 2024 17:35:39 -0500 Subject: [PATCH 06/21] chore: cargo fmt --- compiler/noirc_frontend/src/debug/mod.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 014d495f045..2a44d9f72fd 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -84,11 +84,8 @@ impl DebugInstrumenter { self.scope.push(HashMap::default()); let func_name_span = func.name.span(); let fn_id = self.insert_var(func_name); - let enter_fn = Self::call_fn( - "enter", - vec![uint_expr(fn_id.0 as u128, func.span)], - func.span, - ); + let enter_fn = + Self::call_fn("enter", vec![uint_expr(fn_id.0 as u128, func.span)], func.span); let set_fn_params = func .parameters From 3e15b97a7a876436b0e374350b147ccfab0b2587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Fri, 26 Jan 2024 16:52:37 -0500 Subject: [PATCH 07/21] chore: Remove unnecessary parameter to print_source_code_location Debug locations are filtered before invoking this function --- tooling/debugger/src/repl.rs | 4 ++-- tooling/debugger/src/source_code_printer.rs | 11 +---------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index d3ca572ce85..4f3f4959511 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -77,7 +77,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } let locations = self.context.get_source_location_for_opcode_location(&location); - print_source_code_location(&self.context, self.debug_artifact, &locations); + print_source_code_location(self.debug_artifact, &locations); } } } @@ -102,7 +102,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } let locations = self.context.get_source_location_for_opcode_location(location); - print_source_code_location(&self.context, self.debug_artifact, &locations); + print_source_code_location(self.debug_artifact, &locations); } pub fn show_current_call_stack(&self) { diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 84ccf460c26..4333506c433 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -1,5 +1,3 @@ -use crate::context::DebugContext; -use acvm::BlackBoxFunctionSolver; use codespan_reporting::files::Files; use nargo::artifacts::debug::DebugArtifact; use noirc_errors::Location; @@ -32,17 +30,10 @@ struct LocationPrintContext { // Given a DebugArtifact and an OpcodeLocation, prints all the source code // locations the OpcodeLocation maps to, with some surrounding context and // visual aids to highlight the location itself. -pub(crate) fn print_source_code_location<'a, B: BlackBoxFunctionSolver>( - context: &DebugContext<'a, B>, - debug_artifact: &DebugArtifact, - locations: &[Location], -) { +pub(super) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { let locations = locations.iter(); for loc in locations { - if context.is_source_location_in_debug_module(loc) { - continue; - } print_location_path(debug_artifact, *loc); let lines = render_location(debug_artifact, loc); From d90b17a548c37f9314873cdbb89dcf222a06f54e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Fri, 26 Jan 2024 16:53:30 -0500 Subject: [PATCH 08/21] feat: Improve opcode lookup heuristic when setting source breakpoints Also move code mapping source to opcodes into debugging context. --- tooling/debugger/src/context.rs | 90 +++++++++++++++++++++++++++++++- tooling/debugger/src/dap.rs | 92 +++++---------------------------- 2 files changed, 101 insertions(+), 81 deletions(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 9a5b48e5fe5..44090578f5c 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -8,11 +8,15 @@ use acvm::pwg::{ }; use acvm::{BlackBoxFunctionSolver, FieldElement}; +use codespan_reporting::files::{Files, SimpleFile}; +use fm::FileId; use nargo::artifacts::debug::DebugArtifact; use nargo::errors::{ExecutionError, Location}; use nargo::NargoError; +use noirc_driver::DebugFile; use noirc_printable_type::{PrintableType, PrintableValue}; +use std::collections::BTreeMap; use std::collections::{hash_set::Iter, HashSet}; #[derive(Debug)] @@ -29,6 +33,7 @@ pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> { foreign_call_executor: Box, debug_artifact: &'a DebugArtifact, breakpoints: HashSet, + source_to_opcodes: BTreeMap>, } impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { @@ -39,12 +44,14 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness: WitnessMap, foreign_call_executor: Box, ) -> Self { + let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact); Self { acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), brillig_solver: None, foreign_call_executor, debug_artifact, breakpoints: HashSet::new(), + source_to_opcodes, } } @@ -100,10 +107,45 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { self.debug_artifact .file_map .get(&location.file) - .map(|file| file.path.starts_with("__debug/")) + .map(is_debug_file_in_debug_crate) .unwrap_or(false) } + /// Find an opcode location matching a source code location + // We apply some heuristics here, and there are four possibilities for the + // return value of this function: + // 1. the source location is not found -> None + // 2. an exact unique location is found (very rare) -> Some(opcode_location) + // 3. an exact but not unique location is found, ie. a source location may + // be mapped to multiple opcodes, and those may be disjoint, for example for + // functions called multiple times throughout the program + // -> return the first opcode in program order that matches the source location + // 4. exact location is not found, so an opcode for a nearby source location + // is returned (this again could actually be more than one opcodes) + // -> return the opcode for the next source line that is mapped + pub(super) fn find_opcode_for_source_location( + &self, + file_id: &FileId, + line: i64, + ) -> Option { + let line = line as usize; + let Some(line_to_opcodes) = self.source_to_opcodes.get(file_id) else { + return None; + }; + let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) { + Ok(index) => { + // move backwards to find the first opcode which matches the line + let mut index = index; + while index > 0 && line_to_opcodes[index - 1].0 == line { + index -= 1; + } + line_to_opcodes[index].1 + } + Err(index) => line_to_opcodes[index].1, + }; + Some(found_index) + } + /// Returns the callstack in source code locations for the currently /// executing opcode. This can be `None` if the execution finished (and /// `get_current_opcode_location()` returns `None`) or if the opcode is not @@ -528,6 +570,52 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } +fn is_debug_file_in_debug_crate(debug_file: &DebugFile) -> bool { + debug_file.path.starts_with("__debug/") +} + +/// Builds a map from FileId to an ordered vector of tuples with line +/// numbers and opcode locations corresponding to those line numbers +fn build_source_to_opcode_debug_mappings( + debug_artifact: &DebugArtifact, +) -> BTreeMap> { + if debug_artifact.debug_symbols.is_empty() { + return BTreeMap::new(); + } + let locations = &debug_artifact.debug_symbols[0].locations; + let simple_files: BTreeMap<_, _> = debug_artifact + .file_map + .iter() + .filter(|(_, debug_file)| !is_debug_file_in_debug_crate(debug_file)) + .map(|(file_id, debug_file)| { + ( + file_id, + SimpleFile::new(debug_file.path.to_str().unwrap(), debug_file.source.as_str()), + ) + }) + .collect(); + + let mut result: BTreeMap> = BTreeMap::new(); + locations.iter().for_each(|(opcode_location, source_locations)| { + source_locations.iter().for_each(|source_location| { + let span = source_location.span; + let file_id = source_location.file; + let Some(file) = simple_files.get(&file_id) else { + return; + }; + let Ok(line_index) = file.line_index((), span.start() as usize) else { + return; + }; + let line_number = line_index + 1; + + result.entry(file_id).or_default().push((line_number, *opcode_location)); + }); + }); + result.iter_mut().for_each(|(_, file_locations)| file_locations.sort_by_key(|x| (x.0, x.1))); + + result +} + #[cfg(test)] mod tests { use super::*; diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index 02f5e927c0a..c4a2d206c3b 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -5,7 +5,6 @@ use std::str::FromStr; use acvm::acir::circuit::{Circuit, OpcodeLocation}; use acvm::acir::native_types::WitnessMap; use acvm::BlackBoxFunctionSolver; -use codespan_reporting::files::{Files, SimpleFile}; use crate::context::DebugCommandResult; use crate::context::DebugContext; @@ -30,15 +29,16 @@ use nargo::artifacts::debug::DebugArtifact; use fm::FileId; use noirc_driver::CompiledProgram; +type BreakpointId = i64; + pub struct DapSession<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> { server: Server, context: DebugContext<'a, B>, debug_artifact: &'a DebugArtifact, running: bool, - source_to_opcodes: BTreeMap>, - next_breakpoint_id: i64, - instruction_breakpoints: Vec<(OpcodeLocation, i64)>, - source_breakpoints: BTreeMap>, + next_breakpoint_id: BreakpointId, + instruction_breakpoints: Vec<(OpcodeLocation, BreakpointId)>, + source_breakpoints: BTreeMap>, } enum ScopeReferences { @@ -57,8 +57,6 @@ impl From for ScopeReferences { } } -// BTreeMap - impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { pub fn new( server: Server, @@ -67,7 +65,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, ) -> Self { - let source_to_opcodes = Self::build_source_to_opcode_debug_mappings(debug_artifact); let context = DebugContext::new( solver, circuit, @@ -79,7 +76,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { server, context, debug_artifact, - source_to_opcodes, running: false, next_breakpoint_id: 1, instruction_breakpoints: vec![], @@ -87,45 +83,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } } - /// Builds a map from FileId to an ordered vector of tuples with line - /// numbers and opcode locations corresponding to those line numbers - fn build_source_to_opcode_debug_mappings( - debug_artifact: &'a DebugArtifact, - ) -> BTreeMap> { - if debug_artifact.debug_symbols.is_empty() { - return BTreeMap::new(); - } - let locations = &debug_artifact.debug_symbols[0].locations; - let simple_files: BTreeMap<_, _> = debug_artifact - .file_map - .iter() - .map(|(file_id, debug_file)| { - ( - file_id, - SimpleFile::new(debug_file.path.to_str().unwrap(), debug_file.source.as_str()), - ) - }) - .collect(); - - let mut result: BTreeMap> = BTreeMap::new(); - locations.iter().for_each(|(opcode_location, source_locations)| { - if source_locations.is_empty() { - return; - } - let source_location = source_locations[0]; - let span = source_location.span; - let file_id = source_location.file; - let Ok(line_index) = &simple_files[&file_id].line_index((), span.start() as usize) else { - return; - }; - let line_number = line_index + 1; - - result.entry(file_id).or_default().push((line_number, *opcode_location)); - }); - result.iter_mut().for_each(|(_, file_locations)| file_locations.sort_by_key(|x| x.0)); - result - } - fn send_stopped_event(&mut self, reason: StoppedEventReason) -> Result<(), ServerError> { let description = format!("{:?}", &reason); self.server.send_event(Event::Stopped(StoppedEventBody { @@ -429,7 +386,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { Ok(()) } - fn get_next_breakpoint_id(&mut self) -> i64 { + fn get_next_breakpoint_id(&mut self) -> BreakpointId { let id = self.next_breakpoint_id; self.next_breakpoint_id += 1; id @@ -498,31 +455,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { found.map(|iter| *iter.0) } - // TODO: there are four possibilities for the return value of this function: - // 1. the source location is not found -> None - // 2. an exact unique location is found -> Some(opcode_location) - // 3. an exact but not unique location is found (ie. a source location may - // be mapped to multiple opcodes, and those may be disjoint, for example for - // functions called multiple times throughout the program) - // 4. exact location is not found, so an opcode for a nearby source location - // is returned (this again could actually be more than one opcodes) - // Case 3 is not supported yet, and 4 is not correctly handled. - fn find_opcode_for_source_location( - &self, - file_id: &FileId, - line: i64, - ) -> Option { - let line = line as usize; - let Some(line_to_opcodes) = self.source_to_opcodes.get(file_id) else { - return None; - }; - let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) { - Ok(index) => line_to_opcodes[index].1, - Err(index) => line_to_opcodes[index].1, - }; - Some(found_index) - } - fn map_source_breakpoints(&mut self, args: &SetBreakpointsArguments) -> Vec { let Some(ref source) = &args.source.path else { return vec![]; @@ -539,7 +471,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { .iter() .map(|breakpoint| { let line = breakpoint.line; - let Some(location) = self.find_opcode_for_source_location(&file_id, line) else { + let Some(location) = self.context.find_opcode_for_source_location(&file_id, line) else { return Breakpoint { verified: false, message: Some(String::from("Source location cannot be matched to opcode location")), @@ -606,11 +538,11 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_local_variables(&self) -> Vec { - let mut variables: Vec<_> = self - .context - .get_variables() - .last() - .expect("no stack frame available") + let all_frames = self.context.get_variables(); + let Some(current_frame) = all_frames.last() else { + return vec![]; + }; + let mut variables: Vec<_> = current_frame .2 .iter() .map(|(name, value, _var_type)| Variable { From 68acf85f15b1020ab5eea8765c35316eabd808dd Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 26 Jan 2024 15:00:46 +0100 Subject: [PATCH 09/21] support frames in debug mappings --- tooling/nargo/src/artifacts/debug_vars.rs | 43 ++++++++++++++++------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 6c59596b86b..4c862ed40f0 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -10,31 +10,43 @@ pub struct DebugVars { variables: HashMap, types: HashMap, values: HashMap, - frames: Vec<(DebugVarId, HashSet)>, + frames: Vec<(DebugVarId, HashSet)>, } impl DebugVars { pub fn get_variables( &self, ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + println!("get_variables"); + println!("id_to_name: {:#?}", self.id_to_name); + println!("frames: {:#?}", self.frames); + println!("id_to_type: {:#?}", self.id_to_type); + println!("types: {:#?}", self.types); + self.frames .iter() .map(|(fn_id, frame)| { - let fn_type_id = &self - .variables - .get(fn_id) - .expect("failed to find type for fn_id={fn_id:?}") - .debug_type_id; let fn_type = self .types .get(fn_type_id) .expect(&format!("failed to get function type for fn_type_id={fn_type_id:?}")); let PrintableType::Function { name, arguments, .. } = fn_type else { panic!("unexpected function type {fn_type:?}") }; + let params: Vec<&str> = arguments.iter().map(|(var_name, _)| var_name.as_str()).collect(); let vars: Vec<(&str, &PrintableValue, &PrintableType)> = - frame.iter().filter_map(|var_id| self.lookup_var(*var_id)).collect(); + frame.iter().filter_map(|(var_id, var_value)| { + self.lookup_var(*var_id).map(|(name, typ)| { + (name, var_value, typ) + }) + }) + .collect(); + + // println!("name: {:#?}", name); + // println!("params: {:#?}", params); + // println!("vars: {:#?}", vars); + (name.as_str(), params, vars) }) .collect() @@ -57,17 +69,21 @@ impl DebugVars { } pub fn assign_var(&mut self, var_id: DebugVarId, values: &[Value]) { - self.frames.last_mut().expect("unexpected empty stack frames").1.insert(var_id); let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; let ptype = self.types.get(type_id).unwrap(); self.values.insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); + self.frames.last_mut() + .expect("unexpected empty stack frames").1 + .insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); } pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[Value]) { - let mut cursor: &mut PrintableValue = self - .values + let mut current_frame = &mut self + .frames.last_mut() + .expect("unexpected empty stack frames").1; + let mut cursor: &mut PrintableValue = current_frame .get_mut(&var_id) - .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); + .unwrap_or_else(|| panic!("value unavailable for var_id {var_id}")); let cursor_type_id = &self .variables .get(&var_id) @@ -119,7 +135,10 @@ impl DebugVars { }; } *cursor = decode_value(&mut values.iter().map(|v| v.to_field()), cursor_type); - self.frames.last_mut().expect("unexpected empty stack frames").1.insert(var_id); + + //TODO: I think this is not necessary because current_frame and + // cursor are already mutably borrowed + //current_frame.insert(var_id, *cursor); } pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[Value]) { From 4484bee7c272b6f87714b3176e974fd1bb7a2d77 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 26 Jan 2024 23:45:28 +0100 Subject: [PATCH 10/21] Fix stackframes --- compiler/noirc_frontend/src/debug/mod.rs | 1 - tooling/debugger/src/context.rs | 11 ++- tooling/debugger/src/dap.rs | 13 +-- tooling/debugger/src/foreign_calls.rs | 16 ++-- tooling/debugger/src/repl.rs | 6 +- tooling/nargo/src/artifacts/debug.rs | 2 +- tooling/nargo/src/artifacts/debug_vars.rs | 106 +++++++++++----------- 7 files changed, 79 insertions(+), 76 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 2a44d9f72fd..40ddc121930 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -82,7 +82,6 @@ impl DebugInstrumenter { return; } self.scope.push(HashMap::default()); - let func_name_span = func.name.span(); let fn_id = self.insert_var(func_name); let enter_fn = Self::call_fn("enter", vec![uint_expr(fn_id.0 as u128, func.span)], func.span); diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 44090578f5c..367b72d100d 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -10,11 +10,10 @@ use acvm::{BlackBoxFunctionSolver, FieldElement}; use codespan_reporting::files::{Files, SimpleFile}; use fm::FileId; -use nargo::artifacts::debug::DebugArtifact; +use nargo::artifacts::debug::{DebugArtifact, StackFrame}; use nargo::errors::{ExecutionError, Location}; use nargo::NargoError; use noirc_driver::DebugFile; -use noirc_printable_type::{PrintableType, PrintableValue}; use std::collections::BTreeMap; use std::collections::{hash_set::Iter, HashSet}; @@ -509,12 +508,14 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } - pub(super) fn get_variables( - &self, - ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + pub(super) fn get_variables(&self) -> Vec { return self.foreign_call_executor.get_variables(); } + pub(super) fn current_stack_frame(&self) -> Option { + return self.foreign_call_executor.current_stack_frame(); + } + fn breakpoint_reached(&self) -> bool { if let Some(location) = self.get_current_opcode_location() { self.breakpoints.contains(&location) diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index c4a2d206c3b..5b9f6a1eb86 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -199,7 +199,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { self.debug_artifact.location_column_number(*source_location).unwrap(); let name = match stack_frames.get(index) { - Some(frame) => format!("{} {}", frame.0, index), + Some(frame) => format!("{} {}", frame.function_name, index), None => format!("frame #{index}"), }; @@ -538,19 +538,20 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_local_variables(&self) -> Vec { - let all_frames = self.context.get_variables(); - let Some(current_frame) = all_frames.last() else { + let Some(current_stack_frame) = self.context.current_stack_frame() else { return vec![]; }; - let mut variables: Vec<_> = current_frame - .2 + + let mut variables = current_stack_frame + .variables .iter() .map(|(name, value, _var_type)| Variable { name: String::from(*name), value: format!("{:?}", *value), ..Variable::default() }) - .collect(); + .collect::>(); + variables.sort_by(|a, b| a.name.partial_cmp(&b.name).unwrap()); variables } diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index b937d87929e..9d01d33ec7c 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -3,11 +3,11 @@ use acvm::{ pwg::ForeignCallWaitInfo, }; use nargo::{ - artifacts::debug::{DebugArtifact, DebugVars}, + artifacts::debug::{DebugArtifact, DebugVars, StackFrame}, ops::{DefaultForeignCallExecutor, ForeignCallExecutor, NargoForeignCallResult}, }; use noirc_errors::debug_info::DebugVarId; -use noirc_printable_type::{ForeignCallError, PrintableType, PrintableValue}; +use noirc_printable_type::ForeignCallError; pub(crate) enum DebugForeignCall { VarAssign, @@ -38,8 +38,8 @@ impl DebugForeignCall { } pub trait DebugForeignCallExecutor: ForeignCallExecutor { - fn get_variables(&self) - -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)>; + fn get_variables(&self) -> Vec; + fn current_stack_frame(&self) -> Option; } pub struct DefaultDebugForeignCallExecutor { @@ -70,11 +70,13 @@ impl DefaultDebugForeignCallExecutor { } impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor { - fn get_variables( - &self, - ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { + fn get_variables(&self) -> Vec { self.debug_vars.get_variables() } + + fn current_stack_frame(&self) -> Option { + self.debug_vars.current_stack_frame() + } } fn debug_var_id(value: &Value) -> DebugVarId { diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 4f3f4959511..41dbf604f99 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -337,9 +337,9 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } pub fn show_vars(&self) { - for (fname, params, vars) in self.context.get_variables() { - println!("{fname}({})", params.join(", ")); - for (var_name, value, var_type) in vars.iter() { + for frame in self.context.get_variables() { + println!("{}({})", frame.function_name, frame.function_params.join(", ")); + for (var_name, value, var_type) in frame.variables.iter() { let printable_value = PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); println!(" {var_name}:{var_type:?} = {}", printable_value); diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index a249ecb03ad..7058df120e3 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -8,7 +8,7 @@ use std::{ ops::Range, }; -pub use super::debug_vars::DebugVars; +pub use super::debug_vars::{DebugVars, StackFrame}; use fm::{FileId, FileManager, PathString}; /// A Debug Artifact stores, for a given program, the debug info for every function diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 4c862ed40f0..452e31a628e 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -3,63 +3,67 @@ use noirc_errors::debug_info::{ DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, }; use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; #[derive(Debug, Default, Clone)] pub struct DebugVars { variables: HashMap, types: HashMap, - values: HashMap, - frames: Vec<(DebugVarId, HashSet)>, + frames: Vec<(DebugVarId, HashMap)>, +} + +pub struct StackFrame<'a> { + pub function_name: &'a str, + pub function_params: Vec<&'a str>, + pub variables: Vec<(&'a str, &'a PrintableValue, &'a PrintableType)>, } impl DebugVars { - pub fn get_variables( - &self, - ) -> Vec<(&str, Vec<&str>, Vec<(&str, &PrintableValue, &PrintableType)>)> { - println!("get_variables"); - println!("id_to_name: {:#?}", self.id_to_name); - println!("frames: {:#?}", self.frames); - println!("id_to_type: {:#?}", self.id_to_type); - println!("types: {:#?}", self.types); + pub fn get_variables(&self) -> Vec { + self.frames.iter().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)).collect() + } - self.frames - .iter() - .map(|(fn_id, frame)| { - let fn_type = self - .types - .get(fn_type_id) - .expect(&format!("failed to get function type for fn_type_id={fn_type_id:?}")); - let PrintableType::Function { name, arguments, .. } = fn_type - else { panic!("unexpected function type {fn_type:?}") }; - - let params: Vec<&str> = - arguments.iter().map(|(var_name, _)| var_name.as_str()).collect(); - let vars: Vec<(&str, &PrintableValue, &PrintableType)> = - frame.iter().filter_map(|(var_id, var_value)| { - self.lookup_var(*var_id).map(|(name, typ)| { - (name, var_value, typ) - }) - }) - .collect(); - - // println!("name: {:#?}", name); - // println!("params: {:#?}", params); - // println!("vars: {:#?}", vars); - - (name.as_str(), params, vars) - }) - .collect() + pub fn current_stack_frame(&self) -> Option { + self.frames.last().map(|(function_id, frame)| self.build_stack_frame(function_id, frame)) } - fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableValue, &PrintableType)> { + fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableType)> { self.variables.get(&var_id).and_then(|debug_var| { - let Some(value) = self.values.get(&var_id) else { return None; }; let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; - Some((debug_var.name.as_str(), value, ptype)) + Some((debug_var.name.as_str(), ptype)) }) } + fn build_stack_frame<'a>( + &'a self, + function_id: &DebugVarId, + frame: &'a HashMap, + ) -> StackFrame { + let fn_type_id = &self + .variables + .get(function_id) + .expect("failed to find type for fn_id={function_id:?}") + .debug_type_id; + + let fn_type = self + .types + .get(fn_type_id) + .unwrap_or_else(|| panic!("failed to get function type for fn_type_id={fn_type_id:?}")); + + let PrintableType::Function { name, arguments, .. } = fn_type + else { panic!("unexpected function type {fn_type:?}") }; + + let params: Vec<&str> = arguments.iter().map(|(var_name, _)| var_name.as_str()).collect(); + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame + .iter() + .filter_map(|(var_id, var_value)| { + self.lookup_var(*var_id).map(|(name, typ)| (name, var_value, typ)) + }) + .collect(); + + StackFrame { function_name: name.as_str(), function_params: params, variables: vars } + } + pub fn insert_variables(&mut self, vars: &DebugVariables) { self.variables.extend(vars.clone().into_iter()); } @@ -71,19 +75,19 @@ impl DebugVars { pub fn assign_var(&mut self, var_id: DebugVarId, values: &[Value]) { let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; let ptype = self.types.get(type_id).unwrap(); - self.values.insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); - self.frames.last_mut() - .expect("unexpected empty stack frames").1 + + self.frames + .last_mut() + .expect("unexpected empty stack frames") + .1 .insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); } pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[Value]) { - let mut current_frame = &mut self - .frames.last_mut() - .expect("unexpected empty stack frames").1; + let current_frame = &mut self.frames.last_mut().expect("unexpected empty stack frames").1; let mut cursor: &mut PrintableValue = current_frame .get_mut(&var_id) - .unwrap_or_else(|| panic!("value unavailable for var_id {var_id}")); + .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); let cursor_type_id = &self .variables .get(&var_id) @@ -135,10 +139,6 @@ impl DebugVars { }; } *cursor = decode_value(&mut values.iter().map(|v| v.to_field()), cursor_type); - - //TODO: I think this is not necessary because current_frame and - // cursor are already mutably borrowed - //current_frame.insert(var_id, *cursor); } pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[Value]) { @@ -154,7 +154,7 @@ impl DebugVars { } pub fn push_fn(&mut self, fn_id: DebugVarId) { - self.frames.push((fn_id, HashSet::default())); + self.frames.push((fn_id, HashMap::default())); } pub fn pop_fn(&mut self) { From fc4e19694c19d3856fb2026a67505b84773a1ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Fri, 9 Feb 2024 17:16:17 -0500 Subject: [PATCH 11/21] chore: Re-enable some debugger tests and increase timeout for eddsa --- tooling/debugger/ignored-tests.txt | 10 +--------- tooling/debugger/tests/debug.rs | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index 94bf0f91b57..0a07f435e0a 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -1,19 +1,11 @@ -array_sort -assign_ex -bit_shifts_comptime +bigint brillig_cow -brillig_nested_arrays brillig_references brillig_to_bytes_integration debug_logs double_verify_proof modulus -nested_array_dynamic -nested_array_in_slice -nested_arrays_from_brillig references scalar_mul signed_comparison -simple_2d_array to_bytes_integration -bigint \ No newline at end of file diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs index 4cb678192b8..143ee7987f8 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -12,7 +12,7 @@ mod tests { let nargo_bin = cargo_bin("nargo").into_os_string().into_string().expect("Cannot parse nargo path"); - let mut dbg_session = spawn_bash(Some(10000)).expect("Could not start bash session"); + let mut dbg_session = spawn_bash(Some(15000)).expect("Could not start bash session"); // Set backend to `/dev/null` to force an error if nargo tries to speak to a backend. dbg_session From c772979f89eb652576f5839f5861e8e4cedc12de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Sat, 10 Feb 2024 12:15:14 -0500 Subject: [PATCH 12/21] chore: Some refactoring for code organization --- compiler/noirc_frontend/src/debug/mod.rs | 35 +++++++-------- .../src/monomorphization/debug.rs | 44 +++++++++++++++++-- .../src/monomorphization/mod.rs | 31 ------------- 3 files changed, 58 insertions(+), 52 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 40ddc121930..4a7faefd69d 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -83,8 +83,7 @@ impl DebugInstrumenter { } self.scope.push(HashMap::default()); let fn_id = self.insert_var(func_name); - let enter_fn = - Self::call_fn("enter", vec![uint_expr(fn_id.0 as u128, func.span)], func.span); + let enter_fn = build_debug_call_stmt("enter", fn_id, func.span); let set_fn_params = func .parameters @@ -148,7 +147,7 @@ impl DebugInstrumenter { // exit fn for fn scopes if let Some(fn_id) = opt_fn_id { - statements.push(Self::call_fn("exit", vec![uint_expr(fn_id.0 as u128, span)], span)); + statements.push(build_debug_call_stmt("exit", fn_id, span)); } // return the saved value in __debug_expr, or unit otherwise @@ -176,21 +175,6 @@ impl DebugInstrumenter { statements.push(last_stmt); } - fn call_fn(fname: &str, arguments: Vec, span: Span) -> ast::Statement { - let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { - func: Box::new(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident(&format!["__debug_fn_{fname}"], span)], - kind: PathKind::Plain, - span, - }), - span, - }), - arguments, - })); - ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } - } - fn walk_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; @@ -605,6 +589,21 @@ fn build_assign_member_stmt( ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } } +fn build_debug_call_stmt(fname: &str, fn_id: SourceVarId, span: Span) -> ast::Statement { + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident(&format!["__debug_fn_{fname}"], span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments: vec![uint_expr(fn_id.0 as u128, span)], + })); + ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } +} + fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { let mut vars = vec![]; let mut stack = VecDeque::from([(pattern, false)]); diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index b93ef6a9181..d1fb7b8cdaa 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -5,7 +5,9 @@ use noirc_printable_type::PrintableType; use crate::debug::{SourceFieldId, SourceVarId}; use crate::hir_def::expr::*; +use crate::hir_def::stmt::HirPattern; use crate::node_interner::ExprId; +use crate::Type; use super::ast::{Expression, Ident}; use super::Monomorphizer; @@ -186,9 +188,9 @@ impl<'interner> Monomorphizer<'interner> { arguments: fn_meta .parameters .iter() - .map(|(arg_pattern, arg_type, _arg_vis)| { - let arg_str = self.pattern_to_string(arg_pattern); - (arg_str, arg_type.follow_bindings().into()) + .map(|(arg_pattern, arg_type, _)| { + let arg_display = self.pattern_to_string(arg_pattern); + (arg_display, arg_type.follow_bindings().into()) }) .collect(), env: Box::new(PrintableType::Tuple { types: vec![] }), @@ -205,6 +207,42 @@ impl<'interner> Monomorphizer<'interner> { self.interner.push_expr_location(expr_id, location.span, location.file); expr_id } + + fn pattern_to_string(&self, pat: &HirPattern) -> String { + match pat { + HirPattern::Identifier(hir_id) => self.interner.definition(hir_id.id).name.clone(), + HirPattern::Mutable(mpat, _) => format!("mut {}", self.pattern_to_string(mpat)), + HirPattern::Tuple(elements, _) => format!( + "({})", + elements + .iter() + .map(|element| self.pattern_to_string(element)) + .collect::>() + .join(", ") + ), + HirPattern::Struct(Type::Struct(struct_type, _), fields, _) => { + let struct_type = struct_type.borrow(); + format!( + "{} {{ {} }}", + &struct_type.name.0.contents, + fields + .iter() + .map(|(field_ident, field_pattern)| { + format!( + "{}: {}", + &field_ident.0.contents, + self.pattern_to_string(field_pattern) + ) + }) + .collect::>() + .join(", "), + ) + } + HirPattern::Struct(typ, _, _) => { + panic!("unexpected type of struct: {typ:?}"); + } + } + } } fn element_type_at_index(ptype: &PrintableType, i: usize) -> &PrintableType { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 79a2ced7a30..554bd03409b 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1595,37 +1595,6 @@ impl<'interner> Monomorphizer<'interner> { bindings } - - fn pattern_to_string(&self, pat: &HirPattern) -> String { - match pat { - HirPattern::Identifier(hir_id) => self.interner.definition(hir_id.id).name.clone(), - HirPattern::Mutable(mpat, _) => format!("mut {}", self.pattern_to_string(mpat)), - HirPattern::Tuple(pats, _) => format!( - "({})", - pats.iter() - .map(|tpat| self.pattern_to_string(tpat)) - .collect::>() - .join(",") - ), - HirPattern::Struct(Type::Struct(sh_stype, _field_types), fields, _) => { - let stype = sh_stype.borrow(); - format!( - "{} {{ {} }}", - &stype.name.0.contents, - fields - .iter() - .map(|(id, pat)| { - format!("{}: {}", &id.0.contents, self.pattern_to_string(pat)) - }) - .collect::>() - .join(", "), - ) - } - HirPattern::Struct(typ, _, _) => { - panic!("unexpected type of struct: {typ:?}"); - } - } - } } fn unwrap_tuple_type(typ: &HirType) -> Vec { From a4591226f3d1dbeaafb051942b36908c710f0b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 10:48:44 -0500 Subject: [PATCH 13/21] chore: Remove unneeded function --- compiler/noirc_frontend/src/debug/mod.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 4a7faefd69d..3c403cd92b1 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -78,9 +78,6 @@ impl DebugInstrumenter { fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) { let func_name = &func.name.0.contents; - if is_instrumentation_method(func_name) { - return; - } self.scope.push(HashMap::default()); let fn_id = self.insert_var(func_name); let enter_fn = build_debug_call_stmt("enter", fn_id, func.span); @@ -656,14 +653,3 @@ fn sint_expr(x: i128, span: Span) -> ast::Expression { span, } } - -fn is_instrumentation_method(fname: &str) -> bool { - match fname { - "__debug_var_assign" - | "__debug_var_drop" - | "__debug_dereference_assign" - | "__debug_fn_enter" - | "__debug_fn_exit" => true, - _ => fname.starts_with("__debug_member_assign_"), - } -} From 3b39a3973191fa451212f3fb19552b6342389509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 11:21:34 -0500 Subject: [PATCH 14/21] chore: Load only first DebugInfo when building DebugVars And add documentation re: handling multiple DebugInfo when debugging contracts --- tooling/debugger/src/context.rs | 3 +++ tooling/debugger/src/foreign_calls.rs | 11 +++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 367b72d100d..0b242dac887 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -169,6 +169,9 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { &self, opcode_location: &OpcodeLocation, ) -> Vec { + // TODO: this assumes we're debugging a program (ie. the DebugArtifact + // will contain a single DebugInfo), but this assumption doesn't hold + // for contracts self.debug_artifact.debug_symbols[0] .opcode_location(opcode_location) .map(|source_locations| { diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 9d01d33ec7c..41c81d8079d 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -62,10 +62,13 @@ impl DefaultDebugForeignCallExecutor { } pub fn load_artifact(&mut self, artifact: &DebugArtifact) { - artifact.debug_symbols.iter().for_each(|info| { - self.debug_vars.insert_variables(&info.variables); - self.debug_vars.insert_types(&info.types); - }); + // TODO: handle loading from the correct DebugInfo when we support + // debugging contracts + let Some(info) = artifact.debug_symbols.get(0) else { + return; + }; + self.debug_vars.insert_variables(&info.variables); + self.debug_vars.insert_types(&info.types); } } From 3fae7dc303f7a7c0325936636fe69bd7da47f6dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 14:30:40 -0500 Subject: [PATCH 15/21] feat: Put debug info for functions separate from variables --- compiler/noirc_errors/src/debug_info.rs | 14 +++- compiler/noirc_evaluator/src/ssa.rs | 3 +- compiler/noirc_frontend/src/debug/mod.rs | 50 +++++++++++-- .../src/monomorphization/ast.rs | 5 +- .../src/monomorphization/debug.rs | 74 +------------------ .../src/monomorphization/debug_types.rs | 60 +++++++-------- .../src/monomorphization/mod.rs | 9 +-- tooling/debugger/src/foreign_calls.rs | 11 ++- tooling/nargo/src/artifacts/debug_vars.rs | 48 +++++------- 9 files changed, 125 insertions(+), 149 deletions(-) diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 25722aac57f..0197d55abbc 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -24,6 +24,9 @@ use serde::{ #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] pub struct DebugVarId(pub u32); +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] +pub struct DebugFnId(pub u32); + #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] pub struct DebugTypeId(pub u32); @@ -33,7 +36,14 @@ pub struct DebugVariable { pub debug_type_id: DebugTypeId, } +#[derive(Debug, Clone, Hash, Deserialize, Serialize)] +pub struct DebugFunction { + pub name: String, + pub arg_names: Vec, +} + pub type DebugVariables = BTreeMap; +pub type DebugFunctions = BTreeMap; pub type DebugTypes = BTreeMap; #[serde_as] @@ -45,6 +55,7 @@ pub struct DebugInfo { #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, pub variables: DebugVariables, + pub functions: DebugFunctions, pub types: DebugTypes, } @@ -60,9 +71,10 @@ impl DebugInfo { pub fn new( locations: BTreeMap>, variables: DebugVariables, + functions: DebugFunctions, types: DebugTypes, ) -> Self { - Self { locations, variables, types } + Self { locations, variables, functions, types } } /// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified. diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index d19c4467235..2ca4618064a 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -93,6 +93,7 @@ pub fn create_circuit( ) -> Result<(Circuit, DebugInfo, Vec, Vec, Vec), RuntimeError> { let debug_variables = program.debug_variables.clone(); let debug_types = program.debug_types.clone(); + let debug_functions = program.debug_functions.clone(); let func_sig = program.main_function_signature.clone(); let recursive = program.recursive; let mut generated_acir = optimize_into_acir( @@ -135,7 +136,7 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations, debug_variables, debug_types); + let mut debug_info = DebugInfo::new(locations, debug_variables, debug_functions, debug_types); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 3c403cd92b1..156a2e2d6c6 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -4,6 +4,7 @@ use crate::{ ast::{Path, PathKind}, parser::{Item, ItemKind}, }; +use noirc_errors::debug_info::{DebugFnId, DebugFunction}; use noirc_errors::{Span, Spanned}; use std::collections::HashMap; use std::collections::VecDeque; @@ -26,8 +27,12 @@ pub struct DebugInstrumenter { // all field names referenced when assigning to a member of a variable pub field_names: HashMap, + // all collected function metadata (name + argument names) + pub functions: HashMap, + next_var_id: u32, next_field_name_id: u32, + next_fn_id: u32, // last seen variable names and their IDs grouped by scope scope: Vec>, @@ -38,9 +43,11 @@ impl Default for DebugInstrumenter { Self { variables: HashMap::default(), field_names: HashMap::default(), + functions: HashMap::default(), scope: vec![], next_var_id: 0, next_field_name_id: 1, + next_fn_id: 0, } } } @@ -76,11 +83,20 @@ impl DebugInstrumenter { field_name_id } + fn insert_function(&mut self, fn_name: String, arguments: Vec) -> DebugFnId { + let fn_id = DebugFnId(self.next_fn_id); + self.next_fn_id += 1; + self.functions.insert(fn_id, DebugFunction { name: fn_name, arg_names: arguments }); + fn_id + } + fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) { - let func_name = &func.name.0.contents; - self.scope.push(HashMap::default()); - let fn_id = self.insert_var(func_name); + let func_name = func.name.0.contents.clone(); + let func_args = + func.parameters.iter().map(|param| pattern_to_string(¶m.pattern)).collect(); + let fn_id = self.insert_function(func_name, func_args); let enter_fn = build_debug_call_stmt("enter", fn_id, func.span); + self.scope.push(HashMap::default()); let set_fn_params = func .parameters @@ -108,7 +124,7 @@ impl DebugInstrumenter { &mut self, statements: &mut Vec, span: Span, - opt_fn_id: Option, + opt_fn_id: Option, ) { statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); @@ -586,7 +602,7 @@ fn build_assign_member_stmt( ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } } -fn build_debug_call_stmt(fname: &str, fn_id: SourceVarId, span: Span) -> ast::Statement { +fn build_debug_call_stmt(fname: &str, fn_id: DebugFnId, span: Span) -> ast::Statement { let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { kind: ast::ExpressionKind::Variable(ast::Path { @@ -625,6 +641,30 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { vars } +fn pattern_to_string(pattern: &ast::Pattern) -> String { + match pattern { + ast::Pattern::Identifier(id) => id.0.contents.clone(), + ast::Pattern::Mutable(mpat, _) => format!("mut {}", pattern_to_string(mpat.as_ref())), + ast::Pattern::Tuple(elements, _) => format!( + "({})", + elements.iter().map(pattern_to_string).collect::>().join(", ") + ), + ast::Pattern::Struct(name, fields, _) => { + format!( + "{} {{ {} }}", + name, + fields + .iter() + .map(|(field_ident, field_pattern)| { + format!("{}: {}", &field_ident.0.contents, pattern_to_string(field_pattern)) + }) + .collect::>() + .join(", "), + ) + } + } +} + fn ident(s: &str, span: Span) -> ast::Ident { ast::Ident(Spanned::from(span, s.to_string())) } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 73e7ef372ab..67ce17f4982 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -1,7 +1,7 @@ use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{ - debug_info::{DebugTypes, DebugVariables}, + debug_info::{DebugFunctions, DebugTypes, DebugVariables}, Location, }; @@ -252,6 +252,7 @@ pub struct Program { /// Indicates to a backend whether a SNARK-friendly prover should be used. pub recursive: bool, pub debug_variables: DebugVariables, + pub debug_functions: DebugFunctions, pub debug_types: DebugTypes, } @@ -265,6 +266,7 @@ impl Program { return_visibility: Visibility, recursive: bool, debug_variables: DebugVariables, + debug_functions: DebugFunctions, debug_types: DebugTypes, ) -> Program { Program { @@ -275,6 +277,7 @@ impl Program { return_visibility, recursive, debug_variables, + debug_functions, debug_types, } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index d1fb7b8cdaa..10bc247c220 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -5,9 +5,7 @@ use noirc_printable_type::PrintableType; use crate::debug::{SourceFieldId, SourceVarId}; use crate::hir_def::expr::*; -use crate::hir_def::stmt::HirPattern; use crate::node_interner::ExprId; -use crate::Type; use super::ast::{Expression, Ident}; use super::Monomorphizer; @@ -48,8 +46,6 @@ impl<'interner> Monomorphizer<'interner> { self.patch_debug_var_assign(call, arguments); } else if name == "__debug_var_drop" { self.patch_debug_var_drop(call, arguments); - } else if name == "__debug_fn_enter" { - self.patch_debug_fn_enter(call, arguments); } else if let Some(arity) = name.strip_prefix(DEBUG_MEMBER_ASSIGN_PREFIX) { let arity = arity.parse::().expect("failed to parse member assign arity"); self.patch_debug_member_assign(call, arguments, arity); @@ -75,7 +71,7 @@ impl<'interner> Monomorphizer<'interner> { let var_type = self.interner.id_type(call.arguments[DEBUG_VALUE_ARG_SLOT]); let source_var_id = source_var_id.to_u128().into(); // then update the ID used for tracking at runtime - let var_id = self.debug_type_tracker.insert_var(source_var_id, var_type); + let var_id = self.debug_type_tracker.insert_var(source_var_id, &var_type); let interned_var_id = self.intern_var_id(var_id, &call.location); arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); } @@ -172,77 +168,13 @@ impl<'interner> Monomorphizer<'interner> { arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); } - /// Update instrumentation code to track function enter and exit. - fn patch_debug_fn_enter(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) { - let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); - let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT); - let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else { - unreachable!("Missing source_var_id in __debug_fn_enter call"); - }; - let source_var_id = source_var_id.to_u128().into(); - let func_id = self.current_function_id.expect("current function not set"); - let fn_meta = self.interner.function_meta(&func_id); - let fn_name = self.interner.definition(fn_meta.name.id).name.clone(); - let ptype = PrintableType::Function { - name: fn_name.clone(), - arguments: fn_meta - .parameters - .iter() - .map(|(arg_pattern, arg_type, _)| { - let arg_display = self.pattern_to_string(arg_pattern); - (arg_display, arg_type.follow_bindings().into()) - }) - .collect(), - env: Box::new(PrintableType::Tuple { types: vec![] }), - }; - let fn_id = self.debug_type_tracker.insert_var_printable(source_var_id, ptype); - let interned_var_id = self.intern_var_id(fn_id, &call.location); - arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); - } - fn intern_var_id(&mut self, var_id: DebugVarId, location: &Location) -> ExprId { - let var_id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false); - let expr_id = self.interner.push_expr(HirExpression::Literal(var_id_literal)); + let id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false); + let expr_id = self.interner.push_expr(HirExpression::Literal(id_literal)); self.interner.push_expr_type(&expr_id, crate::Type::FieldElement); self.interner.push_expr_location(expr_id, location.span, location.file); expr_id } - - fn pattern_to_string(&self, pat: &HirPattern) -> String { - match pat { - HirPattern::Identifier(hir_id) => self.interner.definition(hir_id.id).name.clone(), - HirPattern::Mutable(mpat, _) => format!("mut {}", self.pattern_to_string(mpat)), - HirPattern::Tuple(elements, _) => format!( - "({})", - elements - .iter() - .map(|element| self.pattern_to_string(element)) - .collect::>() - .join(", ") - ), - HirPattern::Struct(Type::Struct(struct_type, _), fields, _) => { - let struct_type = struct_type.borrow(); - format!( - "{} {{ {} }}", - &struct_type.name.0.contents, - fields - .iter() - .map(|(field_ident, field_pattern)| { - format!( - "{}: {}", - &field_ident.0.contents, - self.pattern_to_string(field_pattern) - ) - }) - .collect::>() - .join(", "), - ) - } - HirPattern::Struct(typ, _, _) => { - panic!("unexpected type of struct: {typ:?}"); - } - } - } } fn element_type_at_index(ptype: &PrintableType, i: usize) -> &PrintableType { diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs index f54e97bd33e..16b82d1e7b9 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug_types.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -3,7 +3,8 @@ use crate::{ hir_def::types::Type, }; use noirc_errors::debug_info::{ - DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, + DebugFnId, DebugFunction, DebugFunctions, DebugTypeId, DebugTypes, DebugVarId, DebugVariable, + DebugVariables, }; use noirc_printable_type::PrintableType; use std::collections::HashMap; @@ -30,7 +31,10 @@ pub struct DebugTypeTracker { // All instances of tracked variables variables: HashMap, - // Types of tracked variables + // Function metadata collected during instrumentation injection + functions: HashMap, + + // Types of tracked variables and functions types: HashMap, types_reverse: HashMap, @@ -43,34 +47,29 @@ impl DebugTypeTracker { DebugTypeTracker { source_variables: instrumenter.variables.clone(), source_field_names: instrumenter.field_names.clone(), + functions: instrumenter.functions.clone(), ..DebugTypeTracker::default() } } - pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugTypes) { + pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugFunctions, DebugTypes) { let debug_variables = self .variables .clone() .into_iter() .map(|(var_id, (source_var_id, type_id))| { - ( - var_id, - DebugVariable { - name: self.source_variables.get(&source_var_id).cloned().unwrap_or_else( - || { - unreachable!( - "failed to retrieve variable name for {source_var_id:?}" - ); - }, - ), - debug_type_id: type_id, - }, - ) + let var_name = + self.source_variables.get(&source_var_id).cloned().unwrap_or_else(|| { + unreachable!("failed to retrieve variable name for {source_var_id:?}"); + }); + (var_id, DebugVariable { name: var_name, debug_type_id: type_id }) }) .collect(); + + let debug_functions = self.functions.clone().into_iter().collect(); let debug_types = self.types.clone().into_iter().collect(); - (debug_variables, debug_types) + (debug_variables, debug_functions, debug_types) } pub fn resolve_field_index( @@ -83,27 +82,24 @@ impl DebugTypeTracker { .and_then(|field_name| get_field(cursor_type, field_name)) } - pub fn insert_var(&mut self, source_var_id: SourceVarId, var_type: Type) -> DebugVarId { - let ptype: PrintableType = var_type.follow_bindings().into(); - self.insert_var_printable(source_var_id, ptype) + fn insert_type(&mut self, the_type: &Type) -> DebugTypeId { + let ptype: PrintableType = the_type.follow_bindings().into(); + self.types_reverse.get(&ptype).copied().unwrap_or_else(|| { + let type_id = DebugTypeId(self.next_type_id); + self.next_type_id += 1; + self.types_reverse.insert(ptype.clone(), type_id); + self.types.insert(type_id, ptype); + type_id + }) } - pub fn insert_var_printable( - &mut self, - source_var_id: SourceVarId, - ptype: PrintableType, - ) -> DebugVarId { + pub fn insert_var(&mut self, source_var_id: SourceVarId, var_type: &Type) -> DebugVarId { if !self.source_variables.contains_key(&source_var_id) { unreachable!("cannot find source debug variable {source_var_id:?}"); } - let type_id = self.types_reverse.get(&ptype).copied().unwrap_or_else(|| { - let type_id = DebugTypeId(self.next_type_id); - self.next_type_id += 1; - self.types_reverse.insert(ptype.clone(), type_id); - self.types.insert(type_id, ptype); - type_id - }); + let type_id = self.insert_type(var_type); + // check if we need to instantiate the var with a new type let existing_var_id = self.source_to_debug_vars.get(&source_var_id).and_then(|var_id| { let (_, existing_type_id) = self.variables.get(var_id).unwrap(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 554bd03409b..829e628a8c0 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -83,8 +83,6 @@ struct Monomorphizer<'interner> { return_location: Option, debug_type_tracker: DebugTypeTracker, - - current_function_id: Option, } type HirType = crate::Type; @@ -129,7 +127,8 @@ pub fn monomorphize_debug( let FuncMeta { return_distinctness, return_visibility, kind, .. } = monomorphizer.interner.function_meta(&main); - let (debug_variables, debug_types) = monomorphizer.debug_type_tracker.extract_vars_and_types(); + let (debug_variables, debug_functions, debug_types) = + monomorphizer.debug_type_tracker.extract_vars_and_types(); Program::new( functions, function_sig, @@ -138,6 +137,7 @@ pub fn monomorphize_debug( *return_visibility, *kind == FunctionKind::Recursive, debug_variables, + debug_functions, debug_types, ) } @@ -156,7 +156,6 @@ impl<'interner> Monomorphizer<'interner> { is_range_loop: false, return_location: None, debug_type_tracker, - current_function_id: None, } } @@ -252,8 +251,6 @@ impl<'interner> Monomorphizer<'interner> { } fn function(&mut self, f: node_interner::FuncId, id: FuncId) { - self.current_function_id = Some(f); - if let Some((self_type, trait_id)) = self.interner.get_function_trait(&f) { let the_trait = self.interner.get_trait(trait_id); the_trait.self_type_typevar.force_bind(self_type); diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 41c81d8079d..9110bc62d10 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -6,7 +6,7 @@ use nargo::{ artifacts::debug::{DebugArtifact, DebugVars, StackFrame}, ops::{DefaultForeignCallExecutor, ForeignCallExecutor, NargoForeignCallResult}, }; -use noirc_errors::debug_info::DebugVarId; +use noirc_errors::debug_info::{DebugFnId, DebugVarId}; use noirc_printable_type::ForeignCallError; pub(crate) enum DebugForeignCall { @@ -67,8 +67,7 @@ impl DefaultDebugForeignCallExecutor { let Some(info) = artifact.debug_symbols.get(0) else { return; }; - self.debug_vars.insert_variables(&info.variables); - self.debug_vars.insert_types(&info.types); + self.debug_vars.insert_debug_info(info); } } @@ -86,6 +85,10 @@ fn debug_var_id(value: &Value) -> DebugVarId { DebugVarId(value.to_u128() as u32) } +fn debug_fn_id(value: &Value) -> DebugFnId { + DebugFnId(value.to_u128() as u32) +} + impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { fn execute( &mut self, @@ -148,7 +151,7 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { let fcp_fn_id = &foreign_call.inputs[0]; let ForeignCallParam::Single(fn_id_value) = fcp_fn_id else { panic!("unexpected foreign call parameter in fn enter: {fcp_fn_id:?}") }; - let fn_id = debug_var_id(fn_id_value); + let fn_id = debug_fn_id(fn_id_value); self.debug_vars.push_fn(fn_id); Ok(ForeignCallResult::default().into()) } diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 452e31a628e..4b10ab9f839 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -1,6 +1,6 @@ use acvm::brillig_vm::brillig::Value; use noirc_errors::debug_info::{ - DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, + DebugFnId, DebugFunction, DebugInfo, DebugTypeId, DebugVarId, DebugVariable, }; use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; use std::collections::HashMap; @@ -8,8 +8,9 @@ use std::collections::HashMap; #[derive(Debug, Default, Clone)] pub struct DebugVars { variables: HashMap, + functions: HashMap, types: HashMap, - frames: Vec<(DebugVarId, HashMap)>, + frames: Vec<(DebugFnId, HashMap)>, } pub struct StackFrame<'a> { @@ -19,12 +20,18 @@ pub struct StackFrame<'a> { } impl DebugVars { + pub fn insert_debug_info(&mut self, info: &DebugInfo) { + self.variables.extend(info.variables.clone().into_iter()); + self.types.extend(info.types.clone().into_iter()); + self.functions.extend(info.functions.clone().into_iter()); + } + pub fn get_variables(&self) -> Vec { self.frames.iter().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)).collect() } pub fn current_stack_frame(&self) -> Option { - self.frames.last().map(|(function_id, frame)| self.build_stack_frame(function_id, frame)) + self.frames.last().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)) } fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableType)> { @@ -36,24 +43,13 @@ impl DebugVars { fn build_stack_frame<'a>( &'a self, - function_id: &DebugVarId, + fn_id: &DebugFnId, frame: &'a HashMap, ) -> StackFrame { - let fn_type_id = &self - .variables - .get(function_id) - .expect("failed to find type for fn_id={function_id:?}") - .debug_type_id; - - let fn_type = self - .types - .get(fn_type_id) - .unwrap_or_else(|| panic!("failed to get function type for fn_type_id={fn_type_id:?}")); + let debug_fn = &self.functions.get(fn_id).expect("failed to find function metadata"); - let PrintableType::Function { name, arguments, .. } = fn_type - else { panic!("unexpected function type {fn_type:?}") }; - - let params: Vec<&str> = arguments.iter().map(|(var_name, _)| var_name.as_str()).collect(); + let params: Vec<&str> = + debug_fn.arg_names.iter().map(|arg_name| arg_name.as_str()).collect(); let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame .iter() .filter_map(|(var_id, var_value)| { @@ -61,15 +57,11 @@ impl DebugVars { }) .collect(); - StackFrame { function_name: name.as_str(), function_params: params, variables: vars } - } - - pub fn insert_variables(&mut self, vars: &DebugVariables) { - self.variables.extend(vars.clone().into_iter()); - } - - pub fn insert_types(&mut self, types: &DebugTypes) { - self.types.extend(types.clone().into_iter()); + StackFrame { + function_name: debug_fn.name.as_str(), + function_params: params, + variables: vars, + } } pub fn assign_var(&mut self, var_id: DebugVarId, values: &[Value]) { @@ -153,7 +145,7 @@ impl DebugVars { self.frames.last_mut().expect("unexpected empty stack frames").1.remove(&var_id); } - pub fn push_fn(&mut self, fn_id: DebugVarId) { + pub fn push_fn(&mut self, fn_id: DebugFnId) { self.frames.push((fn_id, HashMap::default())); } From e7411416694edb74ef182838a1ef17897144ccff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 14:38:17 -0500 Subject: [PATCH 16/21] fix: Don't put names in a function's PrintableType --- compiler/noirc_frontend/src/hir_def/types.rs | 6 +++--- compiler/noirc_printable_type/src/lib.rs | 11 ++++------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 063969f0fa2..35e3ef7fa90 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1651,9 +1651,9 @@ impl From<&Type> for PrintableType { Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(args, _, env) => PrintableType::Function { - name: "?".to_string(), - arguments: args.iter().map(|arg| ("?".to_string(), arg.into())).collect(), + Type::Function(arguments, return_type, env) => PrintableType::Function { + arguments: arguments.iter().map(|arg| arg.into()).collect(), + return_type: Box::new(return_type.as_ref().into()), env: Box::new(env.as_ref().into()), }, Type::MutableReference(typ) => { diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index c78722501db..60f233cd86d 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -33,8 +33,8 @@ pub enum PrintableType { length: u64, }, Function { - name: String, - arguments: Vec<(String, PrintableType)>, + arguments: Vec, + return_type: Box, env: Box, }, MutableReference { @@ -178,11 +178,8 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str("false"); } } - (PrintableValue::Field(_), PrintableType::Function { name, arguments, .. }) => { - output.push_str(&format!( - "<>", - arguments.iter().map(|(var_name, _)| { var_name }) - )); + (PrintableValue::Field(_), PrintableType::Function { arguments, return_type, .. }) => { + output.push_str(&format!("< {:?}>>", arguments, return_type,)); } (_, PrintableType::MutableReference { .. }) => { output.push_str("<>"); From 3d392d27f3da488447214e81d48c1c91b9e02f46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 15:04:52 -0500 Subject: [PATCH 17/21] feat: Insert enter/exit instrumentation calls in walk_fn To improve readability, instead of passing an optional to walk_scope. --- compiler/noirc_frontend/src/debug/mod.rs | 35 ++++++++++++------------ 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 156a2e2d6c6..f46f01acd46 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -95,10 +95,10 @@ impl DebugInstrumenter { let func_args = func.parameters.iter().map(|param| pattern_to_string(¶m.pattern)).collect(); let fn_id = self.insert_function(func_name, func_args); - let enter_fn = build_debug_call_stmt("enter", fn_id, func.span); + let enter_stmt = build_debug_call_stmt("enter", fn_id, func.span); self.scope.push(HashMap::default()); - let set_fn_params = func + let set_fn_params: Vec<_> = func .parameters .iter() .flat_map(|param| { @@ -112,20 +112,26 @@ impl DebugInstrumenter { }) .collect(); - self.walk_scope(&mut func.body.0, func.span, Some(fn_id)); + let func_body = &mut func.body.0; + let mut statements = func_body.drain(..).collect(); - // prepend fn params: - func.body.0 = vec![vec![enter_fn], set_fn_params, func.body.0.clone()].concat(); + self.walk_scope(&mut statements, func.span); + + // walk_scope ensures that the last statement is the return value of the function + let last_stmt = statements.pop().expect("at least one statement after walk_scope"); + let exit_stmt = build_debug_call_stmt("exit", fn_id, last_stmt.span); + + // rebuild function body + func_body.push(enter_stmt); + func_body.extend(set_fn_params); + func_body.extend(statements); + func_body.push(exit_stmt); + func_body.push(last_stmt); } // Modify a vector of statements in-place, adding instrumentation for sets and drops. // This function will consume a scope level. - fn walk_scope( - &mut self, - statements: &mut Vec, - span: Span, - opt_fn_id: Option, - ) { + fn walk_scope(&mut self, statements: &mut Vec, span: Span) { statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); // extract and save the return value from the scope if there is one @@ -158,11 +164,6 @@ impl DebugInstrumenter { let drop_vars_stmts = scope_vars.values().map(|var_id| build_drop_var_stmt(*var_id, span)); statements.extend(drop_vars_stmts); - // exit fn for fn scopes - if let Some(fn_id) = opt_fn_id { - statements.push(build_debug_call_stmt("exit", fn_id, span)); - } - // return the saved value in __debug_expr, or unit otherwise let last_stmt = if has_ret_expr { ast::Statement { @@ -339,7 +340,7 @@ impl DebugInstrumenter { match &mut expr.kind { ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { self.scope.push(HashMap::default()); - self.walk_scope(statements, expr.span, None); + self.walk_scope(statements, expr.span); } ast::ExpressionKind::Prefix(prefix_expr) => { self.walk_expr(&mut prefix_expr.rhs); From 8a4937ef66ce7f6f4e88afbab1f244d583e1b9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 15:16:26 -0500 Subject: [PATCH 18/21] chore: Rename parameter in debug instrumentation oracle functions --- compiler/noirc_frontend/src/debug/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index f46f01acd46..47e6e88c25b 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -488,20 +488,20 @@ pub fn build_debug_crate_file() -> String { #[oracle(__debug_fn_enter)] unconstrained fn __debug_fn_enter_oracle(_fn_id: u32) {} - unconstrained fn __debug_fn_enter_inner(var_id: u32) { - __debug_fn_enter_oracle(var_id); + unconstrained fn __debug_fn_enter_inner(fn_id: u32) { + __debug_fn_enter_oracle(fn_id); } - pub fn __debug_fn_enter(var_id: u32) { - __debug_fn_enter_inner(var_id); + pub fn __debug_fn_enter(fn_id: u32) { + __debug_fn_enter_inner(fn_id); } #[oracle(__debug_fn_exit)] unconstrained fn __debug_fn_exit_oracle(_fn_id: u32) {} - unconstrained fn __debug_fn_exit_inner(var_id: u32) { - __debug_fn_exit_oracle(var_id); + unconstrained fn __debug_fn_exit_inner(fn_id: u32) { + __debug_fn_exit_oracle(fn_id); } - pub fn __debug_fn_exit(var_id: u32) { - __debug_fn_exit_inner(var_id); + pub fn __debug_fn_exit(fn_id: u32) { + __debug_fn_exit_inner(fn_id); } #[oracle(__debug_dereference_assign)] From 47a5a55b3a7f4ab92d899e47538c60fa0baa039c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 15:16:44 -0500 Subject: [PATCH 19/21] fix: Handle edge case when mapping source lines into opcodes --- tooling/debugger/src/context.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 0b242dac887..e2dd83a2483 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -140,7 +140,12 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } line_to_opcodes[index].1 } - Err(index) => line_to_opcodes[index].1, + Err(index) => { + if index >= line_to_opcodes.len() { + return None; + } + line_to_opcodes[index].1 + } }; Some(found_index) } From 100ad57c0166f35995cfbec4c632f784e2448d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 12 Feb 2024 15:31:34 -0500 Subject: [PATCH 20/21] fix: Compilation errors --- compiler/noirc_frontend/src/debug/mod.rs | 2 +- tooling/debugger/src/source_code_printer.rs | 8 ++++++-- tooling/nargo/src/artifacts/debug.rs | 8 ++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 048749fee01..e751be4591c 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -649,7 +649,7 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { fn pattern_to_string(pattern: &ast::Pattern) -> String { match pattern { ast::Pattern::Identifier(id) => id.0.contents.clone(), - ast::Pattern::Mutable(mpat, _) => format!("mut {}", pattern_to_string(mpat.as_ref())), + ast::Pattern::Mutable(mpat, _, _) => format!("mut {}", pattern_to_string(mpat.as_ref())), ast::Pattern::Tuple(elements, _) => format!( "({})", elements.iter().map(pattern_to_string).collect::>().join(", ") diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 4333506c433..e298eb8aadd 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -269,8 +269,12 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = - vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; + let debug_symbols = vec![DebugInfo::new( + opcode_locations, + BTreeMap::default(), + BTreeMap::default(), + BTreeMap::default(), + )]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_rendered: Vec<_> = render_location(&debug_artifact, &loc).collect(); diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 7058df120e3..fbdf59805c9 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -231,8 +231,12 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = - vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; + let debug_symbols = vec![DebugInfo::new( + opcode_locations, + BTreeMap::default(), + BTreeMap::default(), + BTreeMap::default(), + )]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_in_line = debug_artifact.location_in_line(loc).expect("Expected a range"); From ccd3ec7ff395cde5a6797e466d061943aae97946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Tue, 20 Feb 2024 10:45:05 -0500 Subject: [PATCH 21/21] chore: cargo clippy --- compiler/noirc_frontend/src/debug/mod.rs | 3 ++- tooling/debugger/src/dap.rs | 3 ++- tooling/debugger/src/foreign_calls.rs | 5 +++-- tooling/nargo/src/artifacts/debug_vars.rs | 10 ++++++---- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 6fc3569f4d0..05916502d73 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -8,6 +8,7 @@ use noirc_errors::debug_info::{DebugFnId, DebugFunction}; use noirc_errors::{Span, Spanned}; use std::collections::HashMap; use std::collections::VecDeque; +use std::mem::take; const MAX_MEMBER_ASSIGN_DEPTH: usize = 8; @@ -113,7 +114,7 @@ impl DebugInstrumenter { .collect(); let func_body = &mut func.body.0; - let mut statements = func_body.drain(..).collect(); + let mut statements = take(func_body); self.walk_scope(&mut statements, func.span); diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index e5aa186f1d5..7c722ed0a61 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -473,7 +473,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { .iter() .map(|breakpoint| { let line = breakpoint.line; - let Some(location) = self.context.find_opcode_for_source_location(&file_id, line) else { + let Some(location) = self.context.find_opcode_for_source_location(&file_id, line) + else { return Breakpoint { verified: false, message: Some(String::from( diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 9f2b447ea14..25f126ff490 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -153,8 +153,9 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { } Some(DebugForeignCall::FnEnter) => { let fcp_fn_id = &foreign_call.inputs[0]; - let ForeignCallParam::Single(fn_id_value) = fcp_fn_id - else { panic!("unexpected foreign call parameter in fn enter: {fcp_fn_id:?}") }; + let ForeignCallParam::Single(fn_id_value) = fcp_fn_id else { + panic!("unexpected foreign call parameter in fn enter: {fcp_fn_id:?}") + }; let fn_id = debug_fn_id(fn_id_value); self.debug_vars.push_fn(fn_id); Ok(ForeignCallResult::default().into()) diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs index 4b10ab9f839..66568bec833 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -21,9 +21,9 @@ pub struct StackFrame<'a> { impl DebugVars { pub fn insert_debug_info(&mut self, info: &DebugInfo) { - self.variables.extend(info.variables.clone().into_iter()); - self.types.extend(info.types.clone().into_iter()); - self.functions.extend(info.functions.clone().into_iter()); + self.variables.extend(info.variables.clone()); + self.types.extend(info.types.clone()); + self.functions.extend(info.functions.clone()); } pub fn get_variables(&self) -> Vec { @@ -36,7 +36,9 @@ impl DebugVars { fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableType)> { self.variables.get(&var_id).and_then(|debug_var| { - let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; + let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { + return None; + }; Some((debug_var.name.as_str(), ptype)) }) }