From 12bf4b64f4bcdb4f03b155dc216265e574ceda7d Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 19:15:44 +0100 Subject: [PATCH 01/15] Debugger integration tests --- Cargo.lock | 38 +++++++++++++++++ tooling/debugger/Cargo.toml | 11 ++++- tooling/debugger/build.rs | 74 +++++++++++++++++++++++++++++++++ tooling/debugger/tests/debug.rs | 55 ++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 tooling/debugger/build.rs create mode 100644 tooling/debugger/tests/debug.rs diff --git a/Cargo.lock b/Cargo.lock index 1750f5d284e..704e458cc44 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -910,6 +910,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" +[[package]] +name = "comma" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55b672471b4e9f9e95499ea597ff64941a309b2cdbffcc46f2cc5e2d971fd335" + [[package]] name = "console" version = "0.15.7" @@ -2762,6 +2768,20 @@ dependencies = [ "memoffset 0.6.5", ] +[[package]] +name = "nix" +version = "0.25.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f346ff70e7dbfd675fe90590b92d59ef2de15a8779ae305ebcbfd3f0caf59be4" +dependencies = [ + "autocfg", + "bitflags 1.3.2", + "cfg-if 1.0.0", + "libc", + "memoffset 0.6.5", + "pin-utils", +] + [[package]] name = "nix" version = "0.26.2" @@ -2779,6 +2799,8 @@ name = "noir_debugger" version = "0.22.0" dependencies = [ "acvm", + "assert_cmd", + "build-data", "codespan-reporting", "dap", "easy-repl", @@ -2788,7 +2810,10 @@ dependencies = [ "noirc_errors", "noirc_printable_type", "owo-colors", + "rexpect", + "rustc_version", "serde_json", + "test-binary", "thiserror", ] @@ -3701,6 +3726,19 @@ dependencies = [ "winreg", ] +[[package]] +name = "rexpect" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "01ff60778f96fb5a48adbe421d21bf6578ed58c0872d712e7e08593c195adff8" +dependencies = [ + "comma", + "nix 0.25.1", + "regex", + "tempfile", + "thiserror", +] + [[package]] name = "rfc6979" version = "0.3.1" diff --git a/tooling/debugger/Cargo.toml b/tooling/debugger/Cargo.toml index fba4d028d05..71a3b7bb279 100644 --- a/tooling/debugger/Cargo.toml +++ b/tooling/debugger/Cargo.toml @@ -6,7 +6,9 @@ authors.workspace = true edition.workspace = true license.workspace = true -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[build-dependencies] +rustc_version = "0.4.0" +build-data.workspace = true [dependencies] acvm.workspace = true @@ -20,4 +22,9 @@ codespan-reporting.workspace = true dap.workspace = true easy-repl = "0.2.1" owo-colors = "3" -serde_json.workspace = true \ No newline at end of file +serde_json.workspace = true + +[dev-dependencies] +assert_cmd = "2.0.12" +rexpect = "0.5.0" +test-binary = "3.0.1" \ No newline at end of file diff --git a/tooling/debugger/build.rs b/tooling/debugger/build.rs new file mode 100644 index 00000000000..5d14ec2bae2 --- /dev/null +++ b/tooling/debugger/build.rs @@ -0,0 +1,74 @@ +use rustc_version::{version, Version}; +use std::fs::File; +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::{env, fs}; + +fn check_rustc_version() { + assert!( + version().unwrap() >= Version::parse("1.71.1").unwrap(), + "The minimal supported rustc version is 1.71.1." + ); +} + +const GIT_COMMIT: &&str = &"GIT_COMMIT"; + +fn main() { + // Rebuild if the tests have changed + println!("cargo:rerun-if-changed=tests"); + + check_rustc_version(); + + // Only use build_data if the environment variable isn't set + // The environment variable is always set when working via Nix + if std::env::var(GIT_COMMIT).is_err() { + build_data::set_GIT_COMMIT(); + build_data::set_GIT_DIRTY(); + build_data::no_debug_rebuilds(); + } + + let out_dir = env::var("OUT_DIR").unwrap(); + let destination = Path::new(&out_dir).join("debug.rs"); + let mut test_file = File::create(destination).unwrap(); + + // Try to find the directory that Cargo sets when it is running; otherwise fallback to assuming the CWD + // is the root of the repository and append the crate path + let root_dir = match std::env::var("CARGO_MANIFEST_DIR") { + Ok(dir) => PathBuf::from(dir).parent().unwrap().parent().unwrap().to_path_buf(), + Err(_) => std::env::current_dir().unwrap(), + }; + let test_dir = root_dir.join("test_programs"); + + generate_debugger_tests(&mut test_file, &test_dir); +} + +fn generate_debugger_tests(test_file: &mut File, test_data_dir: &Path) { + let test_sub_dir = "execution_success"; + let test_data_dir = test_data_dir.join(test_sub_dir); + + let test_case_dirs = + fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir()); + + for test_dir in test_case_dirs { + let test_name = + test_dir.file_name().into_string().expect("Directory can't be converted to string"); + if test_name.contains('-') { + panic!( + "Invalid test directory: {test_name}. Cannot include `-`, please convert to `_`" + ); + }; + let test_dir = &test_dir.path(); + + write!( + test_file, + r#" +#[test] +fn debug_{test_name}() {{ + debugger_execution_success("{test_dir}"); +}} + "#, + test_dir = test_dir.display(), + ) + .expect("Could not write templated test file."); + } +} diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs new file mode 100644 index 00000000000..b2f441f5606 --- /dev/null +++ b/tooling/debugger/tests/debug.rs @@ -0,0 +1,55 @@ +#[cfg(test)] +mod tests { + // Some of these imports are consumed by the injected tests + use assert_cmd::cargo::cargo_bin; + + use rexpect::spawn_bash; + + test_binary::build_test_binary_once!(mock_backend, "../backend_interface/test-binaries"); + + // include tests generated by `build.rs` + include!(concat!(env!("OUT_DIR"), "/debug.rs")); + + pub fn debugger_execution_success(test_program_dir: &str) { + let nargo_bin = + cargo_bin("nargo").into_os_string().into_string().expect("Cannot parse nargo path"); + + let mock_backend_path = + path_to_mock_backend().into_string().expect("Cannot parse mock_backend path"); + + let mut dbg_session = spawn_bash(Some(10000)).expect("Could not start bash session"); + + dbg_session + .send_line(&format!("export NARGO_BACKEND_PATH={}", mock_backend_path)) + .expect("Could not export NARGO_BACKEND_PATH."); + dbg_session.wait_for_prompt().expect("Could not export NARGO_BACKEND_PATH."); + + // Start debugger and test that it loads for the given program. + dbg_session + .execute( + &format!("{} debug --program-dir {}", nargo_bin, test_program_dir), + &format!(".*\\Starting debugger.*"), + ) + .expect("Could not start debugger"); + + // While running the debugger, issue a "continue" cmd, + // which should run to the program to end given + // we haven't set any breakpoints. + // ">" is the debugger's prompt, so finding one + // after running "continue" indicates that the + // debugger has not panicked until the end of the program. + dbg_session + .send_line("c") + .expect("Debugger panicked while attempting to step through program."); + dbg_session + .exp_string(">") + .expect("Failed while waiting for debugger to step through program."); + + // Run the "quit" command, then check that the debugger confirms + // having successfully solved the circuit witness. + dbg_session.send_line("quit").expect("Failed to quit debugger"); + dbg_session + .exp_regex(&format!(".*Circuit witness successfully solved.*")) + .expect("Expected circuit witness to be successfully solved."); + } +} From a76d97a2fc1099dd78026773e7e6fcdcb50e4759 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 19:16:21 +0100 Subject: [PATCH 02/15] Fix debugger repl bug --- tooling/debugger/src/repl.rs | 22 ++++++++++++++++++++-- tooling/nargo/src/artifacts/debug.rs | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 16dc206e303..f95cc8aa223 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -92,6 +92,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { self.print_location_path(loc); let loc_line_index = self.debug_artifact.location_line_index(loc).unwrap(); + let loc_end_line_index = self.debug_artifact.location_end_line_index(loc).unwrap(); // How many lines before or after the location's line we // print @@ -101,9 +102,11 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { if loc_line_index < context_lines { 0 } else { loc_line_index - context_lines }; let last_line_index = self.debug_artifact.last_line_index(loc).unwrap(); - let last_line_to_print = std::cmp::min(loc_line_index + context_lines, last_line_index); + let last_line_to_print = + std::cmp::min(loc_end_line_index + context_lines, last_line_index); let source = self.debug_artifact.location_source_code(loc).unwrap(); + for (current_line_index, line) in source.lines().enumerate() { let current_line_number = current_line_index + 1; @@ -126,6 +129,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { // Highlight current location let Range { start: loc_start, end: loc_end } = self.debug_artifact.location_in_line(loc).unwrap(); + println!( "{:>3} {:2} {}{}{}", current_line_number, @@ -134,6 +138,20 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { &line[loc_start..loc_end], &line[loc_end..].to_string().dimmed() ); + } else if current_line_index < loc_end_line_index { + println!("{:>3} {:2} {}", current_line_number, "", line); + } else if current_line_index == loc_end_line_index { + // Highlight current location + let Range { start: loc_start, end: loc_end } = + self.debug_artifact.location_in_end_line(loc).unwrap(); + + println!( + "{:>3} {:2} {}{}", + current_line_number, + "", + &line[loc_start..loc_end], + &line[loc_end..].to_string().dimmed() + ); } else { print_dimmed_line(current_line_number, line); } @@ -181,7 +199,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { println!(" | outputs={:?}", brillig.outputs); for (brillig_index, brillig_opcode) in brillig.bytecode.iter().enumerate() { println!( - "{:>3}.{:<2} |{:2} {:?}", + "{:>3}{:<2} |{:2} {:?}", acir_index, brillig_index, brillig_marker(acir_index, brillig_index), diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 324c476d13d..eacc31f9236 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -59,6 +59,12 @@ impl DebugArtifact { self.line_index(location.file, location_start) } + /// Given a location, returns the index of the line it starts at + pub fn location_end_line_index(&self, location: Location) -> Result { + let location_end = location.span.end() as usize; + self.line_index(location.file, location_end) + } + /// Given a location, returns the line number it starts at pub fn location_line_number(&self, location: Location) -> Result { let location_start = location.span.start() as usize; @@ -82,12 +88,25 @@ impl DebugArtifact { let line_index = self.line_index(location.file, location_start)?; let line_span = self.line_range(location.file, line_index)?; + let line_length = line_span.end - (line_span.start + 1); let start_in_line = location_start - line_span.start; let end_in_line = location_end - line_span.start; + let end_in_line = std::cmp::min(end_in_line, line_length); Ok(Range { start: start_in_line, end: end_in_line }) } + /// Given a location, returns a Span relative to its last line's + /// position in the file. This is useful when processing a file's + /// contents on a per-line-basis. + pub fn location_in_end_line(&self, location: Location) -> Result, Error> { + let end_line_index = self.location_end_line_index(location)?; + let line_span = self.line_range(location.file, end_line_index)?; + let location_end = location.span.end() as usize; + let end_in_line = location_end - line_span.start; + Ok(Range { start: 0, end: end_in_line }) + } + /// Given a location, returns the last line index /// of its file pub fn last_line_index(&self, location: Location) -> Result { From 4bf4676e09d8b15bfc674ebd5c06283676aa54bb Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 19:16:21 +0100 Subject: [PATCH 03/15] Fix debugger repl bug --- tooling/debugger/src/repl.rs | 22 ++++++++++++++++++++-- tooling/nargo/src/artifacts/debug.rs | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 16dc206e303..f95cc8aa223 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -92,6 +92,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { self.print_location_path(loc); let loc_line_index = self.debug_artifact.location_line_index(loc).unwrap(); + let loc_end_line_index = self.debug_artifact.location_end_line_index(loc).unwrap(); // How many lines before or after the location's line we // print @@ -101,9 +102,11 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { if loc_line_index < context_lines { 0 } else { loc_line_index - context_lines }; let last_line_index = self.debug_artifact.last_line_index(loc).unwrap(); - let last_line_to_print = std::cmp::min(loc_line_index + context_lines, last_line_index); + let last_line_to_print = + std::cmp::min(loc_end_line_index + context_lines, last_line_index); let source = self.debug_artifact.location_source_code(loc).unwrap(); + for (current_line_index, line) in source.lines().enumerate() { let current_line_number = current_line_index + 1; @@ -126,6 +129,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { // Highlight current location let Range { start: loc_start, end: loc_end } = self.debug_artifact.location_in_line(loc).unwrap(); + println!( "{:>3} {:2} {}{}{}", current_line_number, @@ -134,6 +138,20 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { &line[loc_start..loc_end], &line[loc_end..].to_string().dimmed() ); + } else if current_line_index < loc_end_line_index { + println!("{:>3} {:2} {}", current_line_number, "", line); + } else if current_line_index == loc_end_line_index { + // Highlight current location + let Range { start: loc_start, end: loc_end } = + self.debug_artifact.location_in_end_line(loc).unwrap(); + + println!( + "{:>3} {:2} {}{}", + current_line_number, + "", + &line[loc_start..loc_end], + &line[loc_end..].to_string().dimmed() + ); } else { print_dimmed_line(current_line_number, line); } @@ -181,7 +199,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { println!(" | outputs={:?}", brillig.outputs); for (brillig_index, brillig_opcode) in brillig.bytecode.iter().enumerate() { println!( - "{:>3}.{:<2} |{:2} {:?}", + "{:>3}{:<2} |{:2} {:?}", acir_index, brillig_index, brillig_marker(acir_index, brillig_index), diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 324c476d13d..eacc31f9236 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -59,6 +59,12 @@ impl DebugArtifact { self.line_index(location.file, location_start) } + /// Given a location, returns the index of the line it starts at + pub fn location_end_line_index(&self, location: Location) -> Result { + let location_end = location.span.end() as usize; + self.line_index(location.file, location_end) + } + /// Given a location, returns the line number it starts at pub fn location_line_number(&self, location: Location) -> Result { let location_start = location.span.start() as usize; @@ -82,12 +88,25 @@ impl DebugArtifact { let line_index = self.line_index(location.file, location_start)?; let line_span = self.line_range(location.file, line_index)?; + let line_length = line_span.end - (line_span.start + 1); let start_in_line = location_start - line_span.start; let end_in_line = location_end - line_span.start; + let end_in_line = std::cmp::min(end_in_line, line_length); Ok(Range { start: start_in_line, end: end_in_line }) } + /// Given a location, returns a Span relative to its last line's + /// position in the file. This is useful when processing a file's + /// contents on a per-line-basis. + pub fn location_in_end_line(&self, location: Location) -> Result, Error> { + let end_line_index = self.location_end_line_index(location)?; + let line_span = self.line_range(location.file, end_line_index)?; + let location_end = location.span.end() as usize; + let end_in_line = location_end - line_span.start; + Ok(Range { start: 0, end: end_in_line }) + } + /// Given a location, returns the last line index /// of its file pub fn last_line_index(&self, location: Location) -> Result { From 415c01108c5f0197c9e8b450c2835e1b77c86e53 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 19:31:05 +0100 Subject: [PATCH 04/15] Add comments --- tooling/debugger/src/repl.rs | 11 ++++++++--- tooling/nargo/src/artifacts/debug.rs | 5 ++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index f95cc8aa223..09b6e687fea 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -102,6 +102,8 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { if loc_line_index < context_lines { 0 } else { loc_line_index - context_lines }; let last_line_index = self.debug_artifact.last_line_index(loc).unwrap(); + + // Print all lines that the current location spans let last_line_to_print = std::cmp::min(loc_end_line_index + context_lines, last_line_index); @@ -126,10 +128,11 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } if current_line_index == loc_line_index { - // Highlight current location let Range { start: loc_start, end: loc_end } = self.debug_artifact.location_in_line(loc).unwrap(); + // Highlight current location from where it starts + // to the end of the current line println!( "{:>3} {:2} {}{}{}", current_line_number, @@ -139,12 +142,14 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { &line[loc_end..].to_string().dimmed() ); } else if current_line_index < loc_end_line_index { + // Highlight current line if it's contained by the current location println!("{:>3} {:2} {}", current_line_number, "", line); } else if current_line_index == loc_end_line_index { - // Highlight current location let Range { start: loc_start, end: loc_end } = self.debug_artifact.location_in_end_line(loc).unwrap(); + // Highlight current location from the beginning + // of the line until the location's own end println!( "{:>3} {:2} {}{}", current_line_number, @@ -199,7 +204,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { println!(" | outputs={:?}", brillig.outputs); for (brillig_index, brillig_opcode) in brillig.bytecode.iter().enumerate() { println!( - "{:>3}{:<2} |{:2} {:?}", + "{:>3}.{:<2} |{:2} {:?}", acir_index, brillig_index, brillig_marker(acir_index, brillig_index), diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index eacc31f9236..43316bef9ef 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -59,7 +59,7 @@ impl DebugArtifact { self.line_index(location.file, location_start) } - /// Given a location, returns the index of the line it starts at + /// Given a location, returns the index of the line it ends at pub fn location_end_line_index(&self, location: Location) -> Result { let location_end = location.span.end() as usize; self.line_index(location.file, location_end) @@ -90,6 +90,9 @@ impl DebugArtifact { let line_length = line_span.end - (line_span.start + 1); let start_in_line = location_start - line_span.start; + + // The location might continue beyond the line, + // so we need a bounds check let end_in_line = location_end - line_span.start; let end_in_line = std::cmp::min(end_in_line, line_length); From 05efccd57f28f8f20c8b7ab069aa8bb610ea3fd0 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 21:37:31 +0100 Subject: [PATCH 05/15] Refactor source code printing --- tooling/debugger/src/lib.rs | 1 + tooling/debugger/src/repl.rs | 106 +----------- tooling/debugger/src/source_code_printer.rs | 180 ++++++++++++++++++++ 3 files changed, 183 insertions(+), 104 deletions(-) create mode 100644 tooling/debugger/src/source_code_printer.rs diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index 7e0c1605e0a..21834e44f93 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -1,6 +1,7 @@ mod context; mod dap; mod repl; +mod source_code_printer; use std::io::{Read, Write}; diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 09b6e687fea..8fbd10d8db6 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -9,12 +9,7 @@ use nargo::{artifacts::debug::DebugArtifact, ops::DefaultForeignCallExecutor, Na use easy_repl::{command, CommandStatus, Repl}; use std::cell::RefCell; -use codespan_reporting::files::Files; -use noirc_errors::Location; - -use owo_colors::OwoColorize; - -use std::ops::Range; +use crate::source_code_printer::print_source_code_location; pub struct ReplDebugger<'a, B: BlackBoxFunctionSolver> { context: DebugContext<'a, B>, @@ -70,96 +65,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { ); } } - self.show_source_code_location(&location); - } - } - } - - fn print_location_path(&self, loc: Location) { - let line_number = self.debug_artifact.location_line_number(loc).unwrap(); - let column_number = self.debug_artifact.location_column_number(loc).unwrap(); - - println!( - "At {}:{line_number}:{column_number}", - self.debug_artifact.name(loc.file).unwrap() - ); - } - - fn show_source_code_location(&self, location: &OpcodeLocation) { - let locations = self.debug_artifact.debug_symbols[0].opcode_location(location); - let Some(locations) = locations else { return }; - for loc in locations { - self.print_location_path(loc); - - let loc_line_index = self.debug_artifact.location_line_index(loc).unwrap(); - let loc_end_line_index = self.debug_artifact.location_end_line_index(loc).unwrap(); - - // How many lines before or after the location's line we - // print - let context_lines = 5; - - let first_line_to_print = - if loc_line_index < context_lines { 0 } else { loc_line_index - context_lines }; - - let last_line_index = self.debug_artifact.last_line_index(loc).unwrap(); - - // Print all lines that the current location spans - let last_line_to_print = - std::cmp::min(loc_end_line_index + context_lines, last_line_index); - - let source = self.debug_artifact.location_source_code(loc).unwrap(); - - for (current_line_index, line) in source.lines().enumerate() { - let current_line_number = current_line_index + 1; - - if current_line_index < first_line_to_print { - // Ignore lines before range starts - continue; - } else if current_line_index == first_line_to_print && current_line_index > 0 { - // Denote that there's more lines before but we're not showing them - print_line_of_ellipsis(current_line_index); - } - - if current_line_index > last_line_to_print { - // Denote that there's more lines after but we're not showing them, - // and stop printing - print_line_of_ellipsis(current_line_number); - break; - } - - if current_line_index == loc_line_index { - let Range { start: loc_start, end: loc_end } = - self.debug_artifact.location_in_line(loc).unwrap(); - - // Highlight current location from where it starts - // to the end of the current line - println!( - "{:>3} {:2} {}{}{}", - current_line_number, - "->", - &line[0..loc_start].to_string().dimmed(), - &line[loc_start..loc_end], - &line[loc_end..].to_string().dimmed() - ); - } else if current_line_index < loc_end_line_index { - // Highlight current line if it's contained by the current location - println!("{:>3} {:2} {}", current_line_number, "", line); - } else if current_line_index == loc_end_line_index { - let Range { start: loc_start, end: loc_end } = - self.debug_artifact.location_in_end_line(loc).unwrap(); - - // Highlight current location from the beginning - // of the line until the location's own end - println!( - "{:>3} {:2} {}{}", - current_line_number, - "", - &line[loc_start..loc_end], - &line[loc_end..].to_string().dimmed() - ); - } else { - print_dimmed_line(current_line_number, line); - } + print_source_code_location(self.debug_artifact, &location); } } } @@ -407,14 +313,6 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } -fn print_line_of_ellipsis(line_number: usize) { - println!("{}", format!("{:>3} {}", line_number, "...").dimmed()); -} - -fn print_dimmed_line(line_number: usize, line: &str) { - println!("{}", format!("{:>3} {:2} {}", line_number, "", line).dimmed()); -} - pub fn run( blackbox_solver: &B, circuit: &Circuit, diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs new file mode 100644 index 00000000000..c4d7f610aac --- /dev/null +++ b/tooling/debugger/src/source_code_printer.rs @@ -0,0 +1,180 @@ +use acvm::acir::circuit::OpcodeLocation; +use codespan_reporting::files::Files; +use nargo::artifacts::debug::DebugArtifact; +use noirc_errors::Location; +use owo_colors::OwoColorize; +use std::ops::Range; + +#[derive(Debug)] +struct PrintedLine<'a> { + number: usize, + cursor: &'a str, + content: &'a str, + highlight: Option>, +} + +impl<'a> PrintedLine<'a> { + fn ellipsis(line_number: usize) -> Self { + Self { number: line_number, cursor: "", content: "...", highlight: None } + } +} + +#[derive(Debug)] +struct PrintedLocation<'a> { + location: Location, + lines: Vec>, +} + +impl<'a> PrintedLocation<'a> { + fn new(loc: Location) -> Self { + Self { location: loc, lines: [].into() } + } + + fn add_line(&mut self, line: PrintedLine<'a>) { + self.lines.push(line); + } +} + +pub(crate) fn print_source_code_location( + debug_artifact: &DebugArtifact, + location: &OpcodeLocation, +) { + let rendered_locations = render(debug_artifact, location); + + for loc in rendered_locations { + print_location_path(debug_artifact, loc.location); + + for line in loc.lines { + match line.highlight { + Some(highlight) => { + println!( + "{:>3} {:2} {}{}{}", + line.number, + line.cursor, + line.content[0..highlight.start].to_string().dimmed(), + &line.content[highlight.start..highlight.end], + line.content[highlight.end..].to_string().dimmed(), + ); + } + None => { + println!( + "{:>3} {:2} {}", + line.number.dimmed(), + line.cursor.dimmed(), + line.content.to_string().dimmed(), + ); + } + } + } + } +} + +fn print_location_path(debug_artifact: &DebugArtifact, loc: Location) { + let line_number = debug_artifact.location_line_number(loc).unwrap(); + let column_number = debug_artifact.location_column_number(loc).unwrap(); + + println!("At {}:{line_number}:{column_number}", debug_artifact.name(loc.file).unwrap()); +} + +fn render<'a>( + debug_artifact: &'a DebugArtifact, + location: &OpcodeLocation, +) -> Vec> { + let mut rendered_locations: Vec = [].into(); + + let locations = debug_artifact.debug_symbols[0].opcode_location(location); + + //TODO: use map instead? + let Some(locations) = locations else { return rendered_locations }; + + for loc in locations { + let mut rendered_loc = PrintedLocation::new(loc); + + let loc_line_index = debug_artifact.location_line_index(loc).unwrap(); + let loc_end_line_index = debug_artifact.location_end_line_index(loc).unwrap(); + + // How many lines before or after the location's line we + // print + let context_lines = 5; + + let first_line_to_print = + if loc_line_index < context_lines { 0 } else { loc_line_index - context_lines }; + + let last_line_index = debug_artifact.last_line_index(loc).unwrap(); + + // Print all lines that the current location spans + let last_line_to_print = std::cmp::min(loc_end_line_index + context_lines, last_line_index); + + let source = debug_artifact.location_source_code(loc).unwrap(); + + for (current_line_index, line) in source.lines().enumerate() { + let current_line_number = current_line_index + 1; + + if current_line_index < first_line_to_print { + // Ignore lines before range starts + continue; + } else if current_line_index == first_line_to_print && current_line_index > 0 { + // Denote that there's more lines before but we're not showing them + rendered_loc.add_line(PrintedLine::ellipsis(current_line_number)); + break; + } + + if current_line_index > last_line_to_print { + // Denote that there's more lines after but we're not showing them, + // and stop printing + rendered_loc.add_line(PrintedLine::ellipsis(current_line_number)); + break; + } + + if current_line_index < loc_line_index { + rendered_loc.add_line(PrintedLine { + number: current_line_number, + cursor: "", + content: line, + highlight: None, + }); + } else if current_line_index == loc_line_index { + let to_highlight = debug_artifact.location_in_line(loc).unwrap(); + + // Highlight current location from where it starts + // to the end of the current line + rendered_loc.add_line(PrintedLine { + number: current_line_number, + cursor: "->", + content: line, + highlight: Some(to_highlight), + }); + } else if current_line_index < loc_end_line_index { + // Highlight current line if it's contained by the current location + rendered_loc.add_line(PrintedLine { + number: current_line_number, + cursor: "", + content: line, + highlight: Some(Range { start: 0, end: line.len() - 1 }), + }); + } else if current_line_index == loc_end_line_index { + let to_highlight = debug_artifact.location_in_end_line(loc).unwrap(); + + // Highlight current location from the beginning + // of the line until the location's own end + rendered_loc.add_line(PrintedLine { + number: current_line_number, + cursor: "", + content: line, + highlight: Some(to_highlight), + }); + } else { + rendered_loc.add_line(PrintedLine { + number: current_line_number, + cursor: "", + content: line, + highlight: None, + }) + } + } + + rendered_locations.push(rendered_loc); + } + + rendered_locations +} From 7d1881a72ca202b3533e72fa5392ad7e05ed1e20 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 22:15:47 +0100 Subject: [PATCH 06/15] Refactor --- tooling/debugger/src/source_code_printer.rs | 195 ++++++++++---------- 1 file changed, 95 insertions(+), 100 deletions(-) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index c4d7f610aac..3bb6e575d89 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -6,17 +6,10 @@ use owo_colors::OwoColorize; use std::ops::Range; #[derive(Debug)] -struct PrintedLine<'a> { - number: usize, - cursor: &'a str, - content: &'a str, - highlight: Option>, -} - -impl<'a> PrintedLine<'a> { - fn ellipsis(line_number: usize) -> Self { - Self { number: line_number, cursor: "", content: "...", highlight: None } - } +enum PrintedLine<'a> { + Skip, + Ellipsis { number: usize }, + Content { number: usize, cursor: &'a str, content: &'a str, highlight: Option> }, } #[derive(Debug)] @@ -25,16 +18,6 @@ struct PrintedLocation<'a> { lines: Vec>, } -impl<'a> PrintedLocation<'a> { - fn new(loc: Location) -> Self { - Self { location: loc, lines: [].into() } - } - - fn add_line(&mut self, line: PrintedLine<'a>) { - self.lines.push(line); - } -} - pub(crate) fn print_source_code_location( debug_artifact: &DebugArtifact, location: &OpcodeLocation, @@ -45,25 +28,31 @@ pub(crate) fn print_source_code_location( print_location_path(debug_artifact, loc.location); for line in loc.lines { - match line.highlight { - Some(highlight) => { - println!( - "{:>3} {:2} {}{}{}", - line.number, - line.cursor, - line.content[0..highlight.start].to_string().dimmed(), - &line.content[highlight.start..highlight.end], - line.content[highlight.end..].to_string().dimmed(), - ); - } - None => { - println!( - "{:>3} {:2} {}", - line.number.dimmed(), - line.cursor.dimmed(), - line.content.to_string().dimmed(), - ); + match line { + PrintedLine::Skip => {} + PrintedLine::Ellipsis { number } => { + println!("{:>3} {:2} {}", number.dimmed(), "", "...".dimmed(),); } + PrintedLine::Content { number, cursor, content, highlight } => match highlight { + Some(highlight) => { + println!( + "{:>3} {:2} {}{}{}", + number, + cursor, + content[0..highlight.start].to_string().dimmed(), + &content[highlight.start..highlight.end], + content[highlight.end..].to_string().dimmed(), + ); + } + None => { + println!( + "{:>3} {:2} {}", + number.dimmed(), + cursor.dimmed(), + content.to_string().dimmed(), + ); + } + }, } } } @@ -88,8 +77,6 @@ fn render<'a>( let Some(locations) = locations else { return rendered_locations }; for loc in locations { - let mut rendered_loc = PrintedLocation::new(loc); - let loc_line_index = debug_artifact.location_line_index(loc).unwrap(); let loc_end_line_index = debug_artifact.location_end_line_index(loc).unwrap(); @@ -107,73 +94,81 @@ fn render<'a>( let source = debug_artifact.location_source_code(loc).unwrap(); - for (current_line_index, line) in source.lines().enumerate() { - let current_line_number = current_line_index + 1; + let lines = source + .lines() + .enumerate() + .map(|(current_line_index, line)| { + let current_line_number = current_line_index + 1; - if current_line_index < first_line_to_print { // Ignore lines before range starts - continue; - } else if current_line_index == first_line_to_print && current_line_index > 0 { + if current_line_index < first_line_to_print { + return PrintedLine::Skip; + } + // Denote that there's more lines before but we're not showing them - rendered_loc.add_line(PrintedLine::ellipsis(current_line_number)); - break; - } + if current_line_index == first_line_to_print && current_line_index > 0 { + return PrintedLine::Ellipsis { number: current_line_number }; + } - if current_line_index > last_line_to_print { // Denote that there's more lines after but we're not showing them, // and stop printing - rendered_loc.add_line(PrintedLine::ellipsis(current_line_number)); - break; - } + if current_line_index == last_line_to_print + 1 { + return PrintedLine::Ellipsis { number: current_line_number }; + } - if current_line_index < loc_line_index { - rendered_loc.add_line(PrintedLine { - number: current_line_number, - cursor: "", - content: line, - highlight: None, - }); - } else if current_line_index == loc_line_index { - let to_highlight = debug_artifact.location_in_line(loc).unwrap(); - - // Highlight current location from where it starts - // to the end of the current line - rendered_loc.add_line(PrintedLine { - number: current_line_number, - cursor: "->", - content: line, - highlight: Some(to_highlight), - }); - } else if current_line_index < loc_end_line_index { - // Highlight current line if it's contained by the current location - rendered_loc.add_line(PrintedLine { - number: current_line_number, - cursor: "", - content: line, - highlight: Some(Range { start: 0, end: line.len() - 1 }), - }); - } else if current_line_index == loc_end_line_index { - let to_highlight = debug_artifact.location_in_end_line(loc).unwrap(); - - // Highlight current location from the beginning - // of the line until the location's own end - rendered_loc.add_line(PrintedLine { - number: current_line_number, - cursor: "", - content: line, - highlight: Some(to_highlight), - }); - } else { - rendered_loc.add_line(PrintedLine { - number: current_line_number, - cursor: "", - content: line, - highlight: None, - }) - } - } + if current_line_index > last_line_to_print { + return PrintedLine::Skip; + } + + if current_line_index < loc_line_index { + PrintedLine::Content { + number: current_line_number, + cursor: "", + content: line, + highlight: None, + } + } else if current_line_index == loc_line_index { + let to_highlight = debug_artifact.location_in_line(loc).unwrap(); + + // Highlight current location from where it starts + // to the end of the current line + PrintedLine::Content { + number: current_line_number, + cursor: "->", + content: line, + highlight: Some(to_highlight), + } + } else if current_line_index < loc_end_line_index { + // Highlight current line if it's contained by the current location + PrintedLine::Content { + number: current_line_number, + cursor: "", + content: line, + highlight: Some(Range { start: 0, end: line.len() - 1 }), + } + } else if current_line_index == loc_end_line_index { + let to_highlight = debug_artifact.location_in_end_line(loc).unwrap(); + + // Highlight current location from the beginning + // of the line until the location's own end + PrintedLine::Content { + number: current_line_number, + cursor: "", + content: line, + highlight: Some(to_highlight), + } + } else { + PrintedLine::Content { + number: current_line_number, + cursor: "", + content: line, + highlight: None, + } + } + }) + .collect(); - rendered_locations.push(rendered_loc); + rendered_locations.push(PrintedLocation { location: loc, lines }); } rendered_locations From 8ff6300dab3d9c323fc48bfb9712a77cab55fed8 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 22:34:06 +0100 Subject: [PATCH 07/15] Handle all cases properly --- tooling/debugger/src/source_code_printer.rs | 40 ++++++++++----------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 3bb6e575d89..789daea12b2 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -99,28 +99,16 @@ fn render<'a>( .enumerate() .map(|(current_line_index, line)| { let current_line_number = current_line_index + 1; - - // Ignore lines before range starts + if current_line_index < first_line_to_print { - return PrintedLine::Skip; - } - - // Denote that there's more lines before but we're not showing them - if current_line_index == first_line_to_print && current_line_index > 0 { - return PrintedLine::Ellipsis { number: current_line_number }; - } - - // Denote that there's more lines after but we're not showing them, - // and stop printing - if current_line_index == last_line_to_print + 1 { - return PrintedLine::Ellipsis { number: current_line_number }; - } - - if current_line_index > last_line_to_print { - return PrintedLine::Skip; - } - - if current_line_index < loc_line_index { + // Ignore lines before the context window we choose to show + PrintedLine::Skip + } else if current_line_index == first_line_to_print && current_line_index > 0 { + // Denote that there's more lines before but we're not showing them + PrintedLine::Ellipsis { number: current_line_number } + } else if current_line_index < loc_line_index { + // Print lines before the location start + // without highlighting PrintedLine::Content { number: current_line_number, cursor: "", @@ -157,13 +145,21 @@ fn render<'a>( content: line, highlight: Some(to_highlight), } - } else { + } else if current_line_index < last_line_to_print { + // Print lines after the location end + // without highlighting PrintedLine::Content { number: current_line_number, cursor: "", content: line, highlight: None, } + } else if current_line_index == last_line_to_print && last_line_to_print < last_line_index { + // Denote that there's more lines after but we're not showing them, + // and stop printing + PrintedLine::Ellipsis { number: current_line_number } + } else { + PrintedLine::Skip } }) .collect(); From 8771e3c2b822d340d8e4a1134fc2550d60dc9274 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 22 Dec 2023 22:43:05 +0100 Subject: [PATCH 08/15] one more case --- tooling/debugger/src/source_code_printer.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 789daea12b2..754407fda6a 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -99,11 +99,14 @@ fn render<'a>( .enumerate() .map(|(current_line_index, line)| { let current_line_number = current_line_index + 1; - + if current_line_index < first_line_to_print { // Ignore lines before the context window we choose to show PrintedLine::Skip - } else if current_line_index == first_line_to_print && current_line_index > 0 { + } else if current_line_index == first_line_to_print + && current_line_index > 0 + && current_line_index < loc_line_index + { // Denote that there's more lines before but we're not showing them PrintedLine::Ellipsis { number: current_line_number } } else if current_line_index < loc_line_index { @@ -154,7 +157,9 @@ fn render<'a>( content: line, highlight: None, } - } else if current_line_index == last_line_to_print && last_line_to_print < last_line_index { + } else if current_line_index == last_line_to_print + && last_line_to_print < last_line_index + { // Denote that there's more lines after but we're not showing them, // and stop printing PrintedLine::Ellipsis { number: current_line_number } From ed45623aa87d920c37a4b53e932585e9878ff1c0 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 28 Dec 2023 13:26:55 +0100 Subject: [PATCH 09/15] Refactor --- tooling/debugger/src/source_code_printer.rs | 263 +++++++++++--------- 1 file changed, 149 insertions(+), 114 deletions(-) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 754407fda6a..2f9be4c0bbc 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -12,22 +12,33 @@ enum PrintedLine<'a> { Content { number: usize, cursor: &'a str, content: &'a str, highlight: Option> }, } -#[derive(Debug)] -struct PrintedLocation<'a> { - location: Location, - lines: Vec>, +#[derive(Clone)] +struct LocationPrintContext { + file_lines: Range, + printed_lines: Range, + location_lines: Range, + location_offset_in_first_line: Range, + location_offset_in_last_line: Range, } +// 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, location: &OpcodeLocation, ) { - let rendered_locations = render(debug_artifact, location); + let locations = debug_artifact.debug_symbols[0].opcode_location(location); + let Some(locations) = locations else { return; }; + + let locations = locations.iter(); - for loc in rendered_locations { - print_location_path(debug_artifact, loc.location); + for loc in locations { + print_location_path(debug_artifact, *loc); + + let lines = render_location(debug_artifact, loc); - for line in loc.lines { + for line in lines { match line { PrintedLine::Skip => {} PrintedLine::Ellipsis { number } => { @@ -65,112 +76,136 @@ fn print_location_path(debug_artifact: &DebugArtifact, loc: Location) { println!("At {}:{line_number}:{column_number}", debug_artifact.name(loc.file).unwrap()); } -fn render<'a>( - debug_artifact: &'a DebugArtifact, - location: &OpcodeLocation, -) -> Vec> { - let mut rendered_locations: Vec = [].into(); - - let locations = debug_artifact.debug_symbols[0].opcode_location(location); - - //TODO: use map instead? - let Some(locations) = locations else { return rendered_locations }; - - for loc in locations { - let loc_line_index = debug_artifact.location_line_index(loc).unwrap(); - let loc_end_line_index = debug_artifact.location_end_line_index(loc).unwrap(); - - // How many lines before or after the location's line we - // print - let context_lines = 5; - - let first_line_to_print = - if loc_line_index < context_lines { 0 } else { loc_line_index - context_lines }; - - let last_line_index = debug_artifact.last_line_index(loc).unwrap(); - - // Print all lines that the current location spans - let last_line_to_print = std::cmp::min(loc_end_line_index + context_lines, last_line_index); - - let source = debug_artifact.location_source_code(loc).unwrap(); - - let lines = source - .lines() - .enumerate() - .map(|(current_line_index, line)| { - let current_line_number = current_line_index + 1; - - if current_line_index < first_line_to_print { - // Ignore lines before the context window we choose to show - PrintedLine::Skip - } else if current_line_index == first_line_to_print - && current_line_index > 0 - && current_line_index < loc_line_index - { - // Denote that there's more lines before but we're not showing them - PrintedLine::Ellipsis { number: current_line_number } - } else if current_line_index < loc_line_index { - // Print lines before the location start - // without highlighting - PrintedLine::Content { - number: current_line_number, - cursor: "", - content: line, - highlight: None, - } - } else if current_line_index == loc_line_index { - let to_highlight = debug_artifact.location_in_line(loc).unwrap(); - - // Highlight current location from where it starts - // to the end of the current line - PrintedLine::Content { - number: current_line_number, - cursor: "->", - content: line, - highlight: Some(to_highlight), - } - } else if current_line_index < loc_end_line_index { - // Highlight current line if it's contained by the current location - PrintedLine::Content { - number: current_line_number, - cursor: "", - content: line, - highlight: Some(Range { start: 0, end: line.len() - 1 }), - } - } else if current_line_index == loc_end_line_index { - let to_highlight = debug_artifact.location_in_end_line(loc).unwrap(); - - // Highlight current location from the beginning - // of the line until the location's own end - PrintedLine::Content { - number: current_line_number, - cursor: "", - content: line, - highlight: Some(to_highlight), - } - } else if current_line_index < last_line_to_print { - // Print lines after the location end - // without highlighting - PrintedLine::Content { - number: current_line_number, - cursor: "", - content: line, - highlight: None, - } - } else if current_line_index == last_line_to_print - && last_line_to_print < last_line_index - { - // Denote that there's more lines after but we're not showing them, - // and stop printing - PrintedLine::Ellipsis { number: current_line_number } - } else { - PrintedLine::Skip - } - }) - .collect(); - - rendered_locations.push(PrintedLocation { location: loc, lines }); +fn render_line( + current: usize, + content: &str, + loc_context: LocationPrintContext, +) -> PrintedLine<'_> { + let file_lines = loc_context.file_lines; + let printed_lines = loc_context.printed_lines; + let location_lines = loc_context.location_lines; + let line_number = current + 1; + + if current < printed_lines.start { + // Ignore lines before the context window we choose to show + PrintedLine::Skip + } else if 0 < current && current == printed_lines.start && current < location_lines.start { + // Denote that there's more lines before but we're not showing them + PrintedLine::Ellipsis { number: line_number } + } else if current < location_lines.start { + // Print lines before the location start without highlighting + PrintedLine::Content { number: line_number, cursor: "", content, highlight: None } + } else if current == location_lines.start { + // Highlight current location from where it starts to the end of the current line + PrintedLine::Content { + number: line_number, + cursor: "->", + content, + highlight: Some(loc_context.location_offset_in_first_line), + } + } else if current < location_lines.end { + // Highlight current line if it's contained by the current location + PrintedLine::Content { + number: line_number, + cursor: "", + content, + highlight: Some(Range { start: 0, end: content.len() - 1 }), + } + } else if current == location_lines.end { + // Highlight current location from the beginning of the line until the location's own end + PrintedLine::Content { + number: line_number, + cursor: "", + content, + highlight: Some(loc_context.location_offset_in_last_line), + } + } else if current < printed_lines.end { + // Print lines after the location end without highlighting + PrintedLine::Content { number: line_number, cursor: "", content, highlight: None } + } else if current == printed_lines.end && printed_lines.end < file_lines.end { + // Denote that there's more lines after but we're not showing them + PrintedLine::Ellipsis { number: line_number } + } else { + PrintedLine::Skip } +} - rendered_locations +// Given a Location in a DebugArtifact, returns a line iterator that specifies how to +// print the location's file. +// +// Consider for example the file (line numbers added to facilitate this doc): +// ``` +// 1 use dep::std::hash::poseidon; +// 2 +// 3 fn main(x1: [Field; 2], y1: pub Field, x2: [Field; 4], y2: pub Field) { +// 4 let hash1 = poseidon::bn254::hash_2(x1); +// 5 assert(hash1 == y1); +// 6 +// 7 let hash2 = poseidon::bn254::hash_4(x2); +// 8 assert(hash2 == y2); +// 9 } +// 10 +// ``` +// +// If the location to render is `poseidon::bn254::hash_2(x1)`, we'll render the file as: +// ``` +// 1 use dep::std::hash::poseidon; +// 2 +// 3 fn main(x1: [Field; 2], y1: pub Field, x2: [Field; 4], y2: pub Field) { +// 4 let hash1 = poseidon::bn254::hash_2(x1); +// 5 -> assert(hash1 == y1); +// 6 +// 7 let hash2 = poseidon::bn254::hash_4(x2); +// 8 assert(hash2 == y2); +// 9 } +// 10 ... +// ``` +// +// This is the result of: +// 1. Limiting the amount of printed lines to 5 before and 5 after the location. +// 2. Using ellipsis (...) to denote when some file lines have been left out of the render. +// 3. Using an arrow cursor (->) to denote where the rendered location starts. +// 4. Highlighting the location (here expressed as a block for the sake of the explanation). +// +// Note that locations may span multiple lines, so this function deals with that too. +fn render_location<'a>( + debug_artifact: &'a DebugArtifact, + loc: &'a Location, +) -> impl Iterator> { + let loc = *loc; + + let file_lines = Range { start: 0, end: debug_artifact.last_line_index(loc).unwrap() }; + + // Sub-range of file lines that this location spans + let location_lines = Range { + start: debug_artifact.location_line_index(loc).unwrap(), + end: debug_artifact.location_end_line_index(loc).unwrap(), + }; + + // How many lines before or after the location's lines we print + let context_lines = 5; + + // Sub-range of lines that we'll print, which includes location + context lines + let first_line_to_print = + if location_lines.start < context_lines { 0 } else { location_lines.start - context_lines }; + let last_line_to_print = std::cmp::min(location_lines.end + context_lines, file_lines.end); + let printed_lines = Range { start: first_line_to_print, end: last_line_to_print }; + + // Range of the location relative to its starting and ending lines + let location_offset_in_first_line = debug_artifact.location_in_line(loc).unwrap(); + let location_offset_in_last_line = debug_artifact.location_in_end_line(loc).unwrap(); + + let context = LocationPrintContext { + file_lines, + printed_lines, + location_lines, + location_offset_in_first_line, + location_offset_in_last_line, + }; + + let source = debug_artifact.location_source_code(loc).unwrap(); + source + .lines() + .enumerate() + .map(move |(index, content)| render_line(index, content, context.clone())) } From bb5c0faeac054c7a890309026d97463b860abbdf Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 28 Dec 2023 13:34:40 +0100 Subject: [PATCH 10/15] refactor --- tooling/debugger/src/source_code_printer.rs | 52 ++++++++++++--------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 2f9be4c0bbc..1dc05291bf1 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -41,29 +41,10 @@ pub(crate) fn print_source_code_location( for line in lines { match line { PrintedLine::Skip => {} - PrintedLine::Ellipsis { number } => { - println!("{:>3} {:2} {}", number.dimmed(), "", "...".dimmed(),); + PrintedLine::Ellipsis { number } => print_ellipsis(number), + PrintedLine::Content { number, cursor, content, highlight } => { + print_content(number, cursor, content, highlight) } - PrintedLine::Content { number, cursor, content, highlight } => match highlight { - Some(highlight) => { - println!( - "{:>3} {:2} {}{}{}", - number, - cursor, - content[0..highlight.start].to_string().dimmed(), - &content[highlight.start..highlight.end], - content[highlight.end..].to_string().dimmed(), - ); - } - None => { - println!( - "{:>3} {:2} {}", - number.dimmed(), - cursor.dimmed(), - content.to_string().dimmed(), - ); - } - }, } } } @@ -76,6 +57,33 @@ fn print_location_path(debug_artifact: &DebugArtifact, loc: Location) { println!("At {}:{line_number}:{column_number}", debug_artifact.name(loc.file).unwrap()); } +fn print_ellipsis(number: usize) { + println!("{:>3} {:2} {}", number.dimmed(), "", "...".dimmed()); +} + +fn print_content(number: usize, cursor: &str, content: &str, highlight: Option>) { + match highlight { + Some(highlight) => { + println!( + "{:>3} {:2} {}{}{}", + number, + cursor, + content[0..highlight.start].to_string().dimmed(), + &content[highlight.start..highlight.end], + content[highlight.end..].to_string().dimmed(), + ); + } + None => { + println!( + "{:>3} {:2} {}", + number.dimmed(), + cursor.dimmed(), + content.to_string().dimmed(), + ); + } + } +} + fn render_line( current: usize, content: &str, From a25b2c6714b4b46c4356bf28ec3efbcaff837ca8 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 28 Dec 2023 15:22:26 +0100 Subject: [PATCH 11/15] Add regression test for debug artifact --- tooling/nargo/src/artifacts/debug.rs | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 43316bef9ef..dafad77d606 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -171,3 +171,70 @@ impl<'a> Files<'a> for DebugArtifact { }) } } + +#[cfg(test)] +mod tests { + use crate::artifacts::debug::DebugArtifact; + use acvm::acir::circuit::OpcodeLocation; + use fm::FileManager; + use noirc_errors::{debug_info::DebugInfo, Location, Span}; + use std::collections::BTreeMap; + use std::ops::Range; + use std::path::Path; + use std::path::PathBuf; + use tempfile::{tempdir, TempDir}; + + // Returns the absolute path to the file + fn create_dummy_file(dir: &TempDir, file_name: &Path) -> PathBuf { + let file_path = dir.path().join(file_name); + let _file = std::fs::File::create(&file_path).unwrap(); + file_path + } + + // Tests that location_in_line correctly handles + // locations spanning multiple lines. + // For example, given the snippet: + // ``` + // permute( + // consts::x5_2_config(), + // state); + // ``` + // We want location_in_line to return the range + // containing `permute(` + #[test] + fn location_in_line_stops_at_end_of_line() { + let source_code = r##"pub fn main(mut state: [Field; 2]) -> [Field; 2] { + state = permute( + consts::x5_2_config(), + state); + + state +}"##; + + let dir = tempdir().unwrap(); + let file_name = Path::new("main.nr"); + create_dummy_file(&dir, file_name); + + let mut fm = FileManager::new(dir.path()); + let file_id = fm.add_file_with_source(file_name, source_code.to_string()).unwrap(); + + // Location of + // ``` + // permute( + // consts::x5_2_config(), + // state) + // ``` + let loc = Location::new(Span::inclusive(63, 117), file_id); + + // We don't care about opcodes in this context, + // we just use a dummy to construct debug_symbols + let mut opcode_locations = BTreeMap::>::new(); + opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); + + let debug_symbols = vec![DebugInfo::new(opcode_locations)]; + let debug_artifact = DebugArtifact::new(debug_symbols, &fm); + + let location_in_line = debug_artifact.location_in_line(loc).expect("Expected a range"); + assert_eq!(location_in_line, Range { start: 12, end: 20 }); + } +} From f050df89294ea93563086ab6bd2c10281329556b Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 28 Dec 2023 17:00:08 +0100 Subject: [PATCH 12/15] Add tests --- Cargo.lock | 1 + tooling/debugger/Cargo.toml | 5 +- tooling/debugger/src/source_code_printer.rs | 140 +++++++++++++++++--- tooling/nargo/src/artifacts/debug.rs | 4 +- 4 files changed, 126 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1750f5d284e..c02d70a3744 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2789,6 +2789,7 @@ dependencies = [ "noirc_printable_type", "owo-colors", "serde_json", + "tempfile", "thiserror", ] diff --git a/tooling/debugger/Cargo.toml b/tooling/debugger/Cargo.toml index fba4d028d05..0afe28727d1 100644 --- a/tooling/debugger/Cargo.toml +++ b/tooling/debugger/Cargo.toml @@ -20,4 +20,7 @@ codespan-reporting.workspace = true dap.workspace = true easy-repl = "0.2.1" owo-colors = "3" -serde_json.workspace = true \ No newline at end of file +serde_json.workspace = true + +[dev_dependencies] +tempfile.workspace = true \ No newline at end of file diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 1dc05291bf1..1707f9066b7 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -5,14 +5,21 @@ use noirc_errors::Location; use owo_colors::OwoColorize; use std::ops::Range; -#[derive(Debug)] +#[derive(Debug, PartialEq)] enum PrintedLine<'a> { Skip, - Ellipsis { number: usize }, - Content { number: usize, cursor: &'a str, content: &'a str, highlight: Option> }, + Ellipsis { + line_number: usize, + }, + Content { + line_number: usize, + cursor: &'a str, + content: &'a str, + highlight: Option>, + }, } -#[derive(Clone)] +#[derive(Clone, Debug)] struct LocationPrintContext { file_lines: Range, printed_lines: Range, @@ -41,9 +48,9 @@ pub(crate) fn print_source_code_location( for line in lines { match line { PrintedLine::Skip => {} - PrintedLine::Ellipsis { number } => print_ellipsis(number), - PrintedLine::Content { number, cursor, content, highlight } => { - print_content(number, cursor, content, highlight) + PrintedLine::Ellipsis { line_number } => print_ellipsis(line_number), + PrintedLine::Content { line_number, cursor, content, highlight } => { + print_content(line_number, cursor, content, highlight) } } } @@ -57,16 +64,16 @@ fn print_location_path(debug_artifact: &DebugArtifact, loc: Location) { println!("At {}:{line_number}:{column_number}", debug_artifact.name(loc.file).unwrap()); } -fn print_ellipsis(number: usize) { - println!("{:>3} {:2} {}", number.dimmed(), "", "...".dimmed()); +fn print_ellipsis(line_number: usize) { + println!("{:>3} {:2} {}", line_number.dimmed(), "", "...".dimmed()); } -fn print_content(number: usize, cursor: &str, content: &str, highlight: Option>) { +fn print_content(line_number: usize, cursor: &str, content: &str, highlight: Option>) { match highlight { Some(highlight) => { println!( "{:>3} {:2} {}{}{}", - number, + line_number, cursor, content[0..highlight.start].to_string().dimmed(), &content[highlight.start..highlight.end], @@ -76,7 +83,7 @@ fn print_content(number: usize, cursor: &str, content: &str, highlight: Option { println!( "{:>3} {:2} {}", - number.dimmed(), + line_number.dimmed(), cursor.dimmed(), content.to_string().dimmed(), ); @@ -99,14 +106,14 @@ fn render_line( PrintedLine::Skip } else if 0 < current && current == printed_lines.start && current < location_lines.start { // Denote that there's more lines before but we're not showing them - PrintedLine::Ellipsis { number: line_number } + PrintedLine::Ellipsis { line_number } } else if current < location_lines.start { // Print lines before the location start without highlighting - PrintedLine::Content { number: line_number, cursor: "", content, highlight: None } + PrintedLine::Content { line_number, cursor: "", content, highlight: None } } else if current == location_lines.start { // Highlight current location from where it starts to the end of the current line PrintedLine::Content { - number: line_number, + line_number, cursor: "->", content, highlight: Some(loc_context.location_offset_in_first_line), @@ -114,25 +121,25 @@ fn render_line( } else if current < location_lines.end { // Highlight current line if it's contained by the current location PrintedLine::Content { - number: line_number, + line_number, cursor: "", content, - highlight: Some(Range { start: 0, end: content.len() - 1 }), + highlight: Some(Range { start: 0, end: content.len() }), } } else if current == location_lines.end { // Highlight current location from the beginning of the line until the location's own end PrintedLine::Content { - number: line_number, + line_number, cursor: "", content, highlight: Some(loc_context.location_offset_in_last_line), } - } else if current < printed_lines.end { + } else if current < printed_lines.end || printed_lines.end == file_lines.end { // Print lines after the location end without highlighting - PrintedLine::Content { number: line_number, cursor: "", content, highlight: None } + PrintedLine::Content { line_number, cursor: "", content, highlight: None } } else if current == printed_lines.end && printed_lines.end < file_lines.end { // Denote that there's more lines after but we're not showing them - PrintedLine::Ellipsis { number: line_number } + PrintedLine::Ellipsis { line_number } } else { PrintedLine::Skip } @@ -217,3 +224,94 @@ fn render_location<'a>( .enumerate() .map(move |(index, content)| render_line(index, content, context.clone())) } + +#[cfg(test)] +mod tests { + use crate::source_code_printer::render_location; + use crate::source_code_printer::PrintedLine::Content; + use acvm::acir::circuit::OpcodeLocation; + use fm::FileManager; + use nargo::artifacts::debug::DebugArtifact; + use noirc_errors::{debug_info::DebugInfo, Location, Span}; + use std::collections::BTreeMap; + use std::ops::Range; + use std::path::Path; + use std::path::PathBuf; + use tempfile::{tempdir, TempDir}; + + // Returns the absolute path to the file + fn create_dummy_file(dir: &TempDir, file_name: &Path) -> PathBuf { + let file_path = dir.path().join(file_name); + let _file = std::fs::File::create(&file_path).unwrap(); + file_path + } + + #[test] + fn render_multiple_line_location() { + let source_code = r##"pub fn main(mut state: [Field; 2]) -> [Field; 2] { + state = permute( + consts::x5_2_config(), + state); + + state +}"##; + + let dir = tempdir().unwrap(); + let file_name = Path::new("main.nr"); + create_dummy_file(&dir, file_name); + + let mut fm = FileManager::new(dir.path()); + let file_id = fm.add_file_with_source(file_name, source_code.to_string()).unwrap(); + + // Location of + // ``` + // permute( + // consts::x5_2_config(), + // state) + // ``` + let loc = Location::new(Span::inclusive(63, 116), file_id); + + // We don't care about opcodes in this context, + // we just use a dummy to construct debug_symbols + let mut opcode_locations = BTreeMap::>::new(); + opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); + + let debug_symbols = vec![DebugInfo::new(opcode_locations)]; + let debug_artifact = DebugArtifact::new(debug_symbols, &fm); + + let location_rendered: Vec<_> = render_location(&debug_artifact, &loc).collect(); + + assert_eq!( + location_rendered, + vec![ + Content { + line_number: 1, + cursor: "", + content: "pub fn main(mut state: [Field; 2]) -> [Field; 2] {", + highlight: None, + }, + Content { + line_number: 2, + cursor: "->", + content: " state = permute(", + highlight: Some(Range { start: 12, end: 20 }), + }, + Content { + line_number: 3, + cursor: "", + content: " consts::x5_2_config(),", + highlight: Some(Range { start: 0, end: 30 }), + }, + Content { + line_number: 4, + cursor: "", + content: " state);", + highlight: Some(Range { start: 0, end: 14 }), + }, + Content { line_number: 5, cursor: "", content: "", highlight: None }, + Content { line_number: 6, cursor: "", content: " state", highlight: None }, + Content { line_number: 7, cursor: "", content: "}", highlight: None }, + ] + ); + } +} diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index dafad77d606..28503f7c684 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -224,7 +224,7 @@ mod tests { // consts::x5_2_config(), // state) // ``` - let loc = Location::new(Span::inclusive(63, 117), file_id); + let loc = Location::new(Span::inclusive(63, 116), file_id); // We don't care about opcodes in this context, // we just use a dummy to construct debug_symbols @@ -235,6 +235,6 @@ mod tests { let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_in_line = debug_artifact.location_in_line(loc).expect("Expected a range"); - assert_eq!(location_in_line, Range { start: 12, end: 20 }); + assert_eq!(location_in_line, Range { start: 12, end: 19 }); } } From 69025e23496c239242fa9faa005adaab7b965aec Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Thu, 28 Dec 2023 18:31:34 +0100 Subject: [PATCH 13/15] Fix failing test --- tooling/nargo/src/artifacts/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 28503f7c684..633fc7a8ded 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -235,6 +235,6 @@ mod tests { let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_in_line = debug_artifact.location_in_line(loc).expect("Expected a range"); - assert_eq!(location_in_line, Range { start: 12, end: 19 }); + assert_eq!(location_in_line, Range { start: 12, end: 20 }); } } From a7a09c699fac707e2d36d533f92006f30d726a1a Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Fri, 29 Dec 2023 15:26:09 +0100 Subject: [PATCH 14/15] fix merge caused regression --- tooling/debugger/src/repl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index f16bf907ce5..b1af2bc2686 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -111,7 +111,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { println!(" | outputs={:?}", brillig.outputs); for (brillig_index, brillig_opcode) in brillig.bytecode.iter().enumerate() { println!( - "{:>3}{:<2} |{:2} {:?}", + "{:>3}.{:<2} |{:2} {:?}", acir_index, brillig_index, brillig_marker(acir_index, brillig_index), From 590eeb5766af752597033a60dffd93b949b70877 Mon Sep 17 00:00:00 2001 From: Martin Verzilli Date: Wed, 3 Jan 2024 14:59:19 +0100 Subject: [PATCH 15/15] fmt & clippy --- Cargo.lock | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 8484b940759..3266f4e652e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2759,6 +2759,20 @@ dependencies = [ "memoffset 0.6.5", ] +[[package]] +name = "nix" +version = "0.25.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f346ff70e7dbfd675fe90590b92d59ef2de15a8779ae305ebcbfd3f0caf59be4" +dependencies = [ + "autocfg", + "bitflags 1.3.2", + "cfg-if 1.0.0", + "libc", + "memoffset 0.6.5", + "pin-utils", +] + [[package]] name = "nix" version = "0.26.4"