diff --git a/Cargo.lock b/Cargo.lock index 346c599cc75..644d31943b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3446,6 +3446,7 @@ dependencies = [ "clap", "color-eyre", "const_format", + "fm", "nargo", "noir_artifact_cli", "noirc_artifacts", diff --git a/acvm-repo/acir/src/circuit/mod.rs b/acvm-repo/acir/src/circuit/mod.rs index 1a2a2e03238..fa6a27d825f 100644 --- a/acvm-repo/acir/src/circuit/mod.rs +++ b/acvm-repo/acir/src/circuit/mod.rs @@ -15,7 +15,12 @@ use msgpack_tagged::MsgpackTagged; pub use opcodes::Opcode; use thiserror::Error; -use std::{collections::HashMap, io::prelude::*, num::ParseIntError, str::FromStr}; +use std::{ + collections::{BTreeMap, HashMap}, + io::prelude::*, + num::ParseIntError, + str::FromStr, +}; use base64::Engine; use flate2::Compression; @@ -341,13 +346,14 @@ impl Deserialize<'a> + MsgpackTagged> Program { impl std::fmt::Display for Circuit { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - display_circuit(self, None, f) + display_circuit(self, None, None, f) } } pub fn display_circuit( circuit: &Circuit, error_types: Option<&HashMap>, + annotations: Option<&BTreeMap>, f: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { let write_witness_indices = @@ -382,6 +388,10 @@ pub fn display_circuit( circuit.assert_messages.iter().cloned().collect::>(); for (index, opcode) in circuit.opcodes.iter().enumerate() { + if let Some(annotation) = annotations.and_then(|annotations| annotations.get(&index)) { + writeln!(f, "// {annotation}")?; + } + display_opcode(opcode, Some(&circuit.return_values), f)?; if let Some(error_types) = error_types { @@ -405,18 +415,20 @@ impl std::fmt::Debug for Circuit { impl std::fmt::Display for Program { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - display_program(self, None, f) + display_program(self, None, None, f) } } pub fn display_program( program: &Program, error_types: Option<&HashMap>, + annotations: Option<&[BTreeMap]>, f: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { for (func_index, function) in program.functions.iter().enumerate() { writeln!(f, "func {func_index}")?; - display_circuit(function, error_types, f)?; + let annotations = annotations.and_then(|annotations: &[_]| annotations.get(func_index)); + display_circuit(function, error_types, annotations, f)?; writeln!(f)?; } for (func_index, function) in program.unconstrained_functions.iter().enumerate() { diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index bd9bc1130ce..8e2d6d03af3 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -13,6 +13,7 @@ use clap::Args; use fm::{FileId, FileManager}; use iter_extended::vecmap; use noirc_abi::{AbiErrorType, AbiNamedValue, AbiParameter, AbiType}; +use noirc_artifacts::annotations::program_opcode_annotations; use noirc_artifacts::contract::{CompiledContract, CompiledContractOutputs, ContractFunction}; use noirc_artifacts::debug::{DebugFile, DebugInfo, FunctionLocation}; use noirc_artifacts::program::CompiledProgram; @@ -155,6 +156,11 @@ pub struct CompileOptions { #[arg(long)] pub print_acir: bool, + /// When displaying the ACIR (see `--print-acir`), annotate each opcode with the + /// Noir source location and code snippet it was compiled from. + #[arg(long)] + pub with_acir_locations: bool, + /// Pretty print benchmark times of each code generation pass #[arg(long, hide = true)] pub benchmark_codegen: bool, @@ -316,6 +322,7 @@ impl Default for CompileOptions { show_brillig: false, show_brillig_opcode_advisories: false, print_acir: false, + with_acir_locations: false, benchmark_codegen: false, deny_warnings: false, silence_warnings: false, @@ -525,7 +532,10 @@ pub fn compile_main( if options.print_acir { noirc_errors::println_to_stdout!("Compiled ACIR for main:"); - noirc_errors::println_to_stdout!("{}", display_compiled_program(&compiled_program)); + noirc_errors::println_to_stdout!( + "{}", + display_compiled_program(&compiled_program, options.with_acir_locations) + ); } Ok((compiled_program, warnings)) @@ -590,7 +600,14 @@ pub fn compile_contract( "Compiled ACIR for {}::{} (non-transformed):", compiled_contract.name, contract_function.name ); - println!("{}", contract_function.bytecode); + println!( + "{}", + display_contract_function( + contract_function, + &compiled_contract.file_map, + options.with_acir_locations + ) + ); } } Ok((compiled_contract, warnings)) @@ -1035,17 +1052,48 @@ fn ssa_report_to_custom_diagnostic(error: SsaReport) -> CustomDiagnostic { } } -pub fn display_compiled_program(program: &CompiledProgram) -> String { - ProgramDisplay { program: &program.program, error_types: &program.abi.error_types }.to_string() +pub fn display_compiled_program(program: &CompiledProgram, with_locations: bool) -> String { + let annotations = + program_annotations(&program.program, &program.debug, &program.file_map, with_locations); + ProgramDisplay { program: &program.program, error_types: &program.abi.error_types, annotations } + .to_string() +} + +fn display_contract_function( + function: &ContractFunction, + file_map: &BTreeMap, + with_locations: bool, +) -> String { + let annotations = + program_annotations(&function.bytecode, &function.debug, file_map, with_locations); + let error_types = BTreeMap::new(); + ProgramDisplay { program: &function.bytecode, error_types: &error_types, annotations } + .to_string() +} + +/// Builds per-circuit opcode annotations for every ACIR function of a program, or +/// `None` when source locations were not requested. +fn program_annotations( + program: &Program, + debug: &[DebugInfo], + file_map: &BTreeMap, + with_locations: bool, +) -> Option>> { + with_locations.then(|| program_opcode_annotations(program, debug, file_map)) } /// Formats an ACIR [Program] together with its ABI error types so that any static /// assertion payloads embedded in the program are rendered as a `// message` comment /// next to the relevant ACIR/Brillig opcode. This is the same display used by /// `nargo compile --print-acir`. +/// +/// When `annotations` are present (see [program_opcode_annotations]), each annotated +/// ACIR opcode is additionally preceded by a `// file:line:col: snippet` comment +/// describing the Noir source it was compiled from. struct ProgramDisplay<'a, F: AcirField> { program: &'a Program, error_types: &'a BTreeMap, + annotations: Option>>, } impl std::fmt::Display for ProgramDisplay<'_, F> { @@ -1061,6 +1109,6 @@ impl std::fmt::Display for ProgramDisplay<'_, F> { } }) .collect::>(); - display_program(self.program, Some(&error_types), f) + display_program(self.program, Some(&error_types), self.annotations.as_deref(), f) } } diff --git a/compiler/noirc_driver/tests/print_acir.rs b/compiler/noirc_driver/tests/print_acir.rs index 09b8a827f6c..79bcfb8c429 100644 --- a/compiler/noirc_driver/tests/print_acir.rs +++ b/compiler/noirc_driver/tests/print_acir.rs @@ -47,7 +47,7 @@ fn print_acir_renders_static_assertion_payload() { "#; let program = compile(source, false); - let displayed = display_compiled_program(&program); + let displayed = display_compiled_program(&program, false); insta::assert_snapshot!(displayed, @r" func 0 @@ -119,6 +119,141 @@ fn dynamic_custom_error_type_is_preserved() { ); } +#[test] +fn print_acir_with_locations_annotates_opcode_runs() { + let source = r#" + fn main(x: u32, y: u32) -> pub u32 { + let sum = x * y; + assert(sum != 10); + sum + } + "#; + + let program = compile(source, false); + let displayed = display_compiled_program(&program, true); + + // Each run of opcodes compiled from the same source span gets a single + // `// file:line:col: snippet` comment above it. + insta::assert_snapshot!(displayed, @r" + func 0 + private parameters: [w0, w1] + public parameters: [] + return values: [w2] + BLACKBOX::RANGE input: w0, bits: 32 + BLACKBOX::RANGE input: w1, bits: 32 + // main.nr:3:19: x * y + ASSERT w3 = w0*w1 + BLACKBOX::RANGE input: w3, bits: 32 // attempt to multiply with overflow + // main.nr:4:9: assert(sum != 10) + BRILLIG CALL func: 0, predicate: 1, inputs: [w3 - 10], outputs: [w4] + ASSERT 0 = w3*w4 - 10*w4 - 1 + // no source location + ASSERT w2 = w3 + + unconstrained func 0: directive_invert + 0: @21 = const u32 1 + 1: @20 = const u32 0 + 2: @0 = calldata copy [@20; @21] + 3: @2 = const field 0 + 4: @3 = field eq @0, @2 + 5: jump if @3 to 8 + 6: @1 = const field 1 + 7: @0 = field field_div @1, @0 + 8: stop @[@20; @21] + "); +} + +#[test] +fn print_acir_with_locations_shows_inlined_caller_chain() { + let source = r#" + fn square(v: u32) -> u32 { + v * v + } + + fn main(x: u32) -> pub u32 { + square(x) + } + "#; + + let program = compile(source, false); + let displayed = display_compiled_program(&program, true); + + // Opcodes from the inlined `square` body are annotated with the location + // inside `square` plus a `(via ...)` trail back to the call site in `main`. + insta::assert_snapshot!(displayed, @r" + func 0 + private parameters: [w0] + public parameters: [] + return values: [w1] + BLACKBOX::RANGE input: w0, bits: 32 + // main.nr:3:9: v * v (via main.nr:7:9) + ASSERT w2 = w0*w0 + BLACKBOX::RANGE input: w2, bits: 32 // attempt to multiply with overflow + // no source location + ASSERT w1 = w2 + "); +} + +#[test] +fn print_acir_with_locations_collapses_multi_line_snippets() { + let source = r#" + fn main(x: u32) -> pub u32 { + let y = x + * 3 + * x; + y + } + "#; + + let program = compile(source, false); + let displayed = display_compiled_program(&program, true); + + // A span covering several source lines is collapsed to a single line, + // so the two nested multiplication spans remain distinguishable. + insta::assert_snapshot!(displayed, @r" + func 0 + private parameters: [w0] + public parameters: [] + return values: [w1] + BLACKBOX::RANGE input: w0, bits: 32 + // main.nr:3:17: x * 3 + ASSERT w2 = 3*w0 + BLACKBOX::RANGE input: w2, bits: 32 // attempt to multiply with overflow + // main.nr:3:17: x * 3 * x + ASSERT w3 = w0*w2 + BLACKBOX::RANGE input: w3, bits: 32 // attempt to multiply with overflow + // no source location + ASSERT w1 = w3 + "); +} + +#[test] +fn print_acir_with_locations_round_trips_through_parser() { + let source = r#" + fn main(x: u32, y: u32) -> pub u32 { + let sum = x * y; + assert(sum != 10); + sum + } + "#; + + let program = compile(source, false); + + // Location annotations are `//` comments, which the ACIR parser skips, so + // the annotated display must parse to exactly the same circuit as the + // unannotated one. + let parse = |displayed: &str| { + let circuit_text = displayed + .strip_prefix("func 0\n") + .expect("displayed program should start with the `func 0` header"); + Circuit::from_str(circuit_text).expect("displayed ACIR should be parseable") + }; + + let annotated = parse(&display_compiled_program(&program, true)); + let unannotated = parse(&display_compiled_program(&program, false)); + assert_eq!(annotated, unannotated); +} + #[test] fn print_acir_renders_brillig_assertion_payload() { let source = r#" @@ -128,7 +263,7 @@ fn print_acir_renders_brillig_assertion_payload() { "#; let program = compile(source, true); - let displayed = display_compiled_program(&program); + let displayed = display_compiled_program(&program, false); insta::assert_snapshot!(displayed, @r#" func 0 diff --git a/tooling/inspector/Cargo.toml b/tooling/inspector/Cargo.toml index 8b91b76f61d..fb0cc1f1b38 100644 --- a/tooling/inspector/Cargo.toml +++ b/tooling/inspector/Cargo.toml @@ -24,6 +24,7 @@ serde_json.workspace = true color-eyre.workspace = true const_format.workspace = true acir.workspace = true +fm.workspace = true noirc_artifacts.workspace = true noirc_artifacts_info.workspace = true noir_artifact_cli.workspace = true diff --git a/tooling/inspector/src/cli/print_acir_cmd.rs b/tooling/inspector/src/cli/print_acir_cmd.rs index aa5dcfcaf9c..0270aebc805 100644 --- a/tooling/inspector/src/cli/print_acir_cmd.rs +++ b/tooling/inspector/src/cli/print_acir_cmd.rs @@ -1,8 +1,14 @@ +use std::collections::BTreeMap; use std::path::PathBuf; +use acir::FieldElement; +use acir::circuit::{Program, display_program}; use clap::Args; use color_eyre::eyre; +use fm::FileId; use noir_artifact_cli::Artifact; +use noirc_artifacts::annotations::program_opcode_annotations; +use noirc_artifacts::debug::{DebugFile, ProgramDebugInfo}; #[derive(Debug, Clone, Args)] pub(crate) struct PrintAcirCommand { @@ -12,6 +18,11 @@ pub(crate) struct PrintAcirCommand { /// Name of the function to print, if the artifact is a contract. #[clap(long)] contract_fn: Option, + + /// Annotate each opcode with the Noir source location and code snippet it was + /// compiled from, using the debug symbols embedded in the artifact. + #[clap(long)] + with_locations: bool, } pub(crate) fn run(args: PrintAcirCommand) -> eyre::Result<()> { @@ -20,7 +31,12 @@ pub(crate) fn run(args: PrintAcirCommand) -> eyre::Result<()> { match artifact { Artifact::Program(program) => { println!("Compiled ACIR for main:"); - println!("{}", program.bytecode); + print_program( + &program.bytecode, + &program.debug_symbols, + &program.file_map, + args.with_locations, + ); } Artifact::Contract(contract) => { println!("Compiled circuits for contract '{}':", contract.name); @@ -30,10 +46,39 @@ pub(crate) fn run(args: PrintAcirCommand) -> eyre::Result<()> { .filter(|f| args.contract_fn.as_ref().is_none_or(|n| *n == f.name)) { println!("Compiled ACIR for function '{}':", function.name); - println!("{}", function.bytecode); + print_program( + &function.bytecode, + &function.debug_symbols, + &contract.file_map, + args.with_locations, + ); } } } Ok(()) } + +fn print_program( + program: &Program, + debug_symbols: &ProgramDebugInfo, + file_map: &BTreeMap, + with_locations: bool, +) { + let annotations = with_locations + .then(|| program_opcode_annotations(program, &debug_symbols.debug_infos, file_map)); + println!("{}", AnnotatedProgram { program, annotations }); +} + +/// Displays a [Program], attaching source-location annotations to its ACIR opcodes +/// when present. +struct AnnotatedProgram<'a> { + program: &'a Program, + annotations: Option>>, +} + +impl std::fmt::Display for AnnotatedProgram<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + display_program(self.program, None, self.annotations.as_deref(), f) + } +} diff --git a/tooling/inspector/tests/print_acir_tests.rs b/tooling/inspector/tests/print_acir_tests.rs new file mode 100644 index 00000000000..4547caf441e --- /dev/null +++ b/tooling/inspector/tests/print_acir_tests.rs @@ -0,0 +1,56 @@ +use assert_cmd::prelude::*; +use predicates::prelude::*; +use std::path::PathBuf; +use std::process::Command; + +fn inspector_command() -> Command { + #[allow(deprecated)] + Command::cargo_bin("noir-inspector").unwrap() +} + +// A different test program from the one `info_tests.rs` compiles, so the two +// test binaries never race on the same artifact file. +fn test_program_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../test_programs/execution_success/a_3_add") +} + +/// get test program artifact path, always recompiling to ensure correct version +fn test_artifact_path() -> PathBuf { + let program_dir = test_program_dir(); + + #[allow(deprecated)] + let mut nargo = Command::cargo_bin("nargo").unwrap(); + nargo.arg("--program-dir").arg(&program_dir).arg("compile").arg("--force"); + + nargo.assert().success(); + + program_dir.join("target/a_3_add.json") +} + +#[test] +fn test_print_acir_with_locations() { + let artifact = test_artifact_path(); + + inspector_command() + .arg("print-acir") + .arg(&artifact) + .arg("--with-locations") + .assert() + .success() + .stdout( + predicate::str::contains("// ") + .and(predicate::str::contains("src/main.nr:4:5: assert(x == z)")), + ); +} + +#[test] +fn test_print_acir_without_locations_has_no_annotations() { + let artifact = test_artifact_path(); + + inspector_command() + .arg("print-acir") + .arg(&artifact) + .assert() + .success() + .stdout(predicate::str::contains("// ").not().and(predicate::str::contains("ASSERT"))); +} diff --git a/tooling/noirc_artifacts/src/annotations.rs b/tooling/noirc_artifacts/src/annotations.rs new file mode 100644 index 00000000000..b7e1801a018 --- /dev/null +++ b/tooling/noirc_artifacts/src/annotations.rs @@ -0,0 +1,137 @@ +//! Source-location annotations for displayed ACIR opcodes. +//! +//! Uses the call stacks recorded in [DebugInfo] and the sources in an artifact's +//! `file_map` to describe, for each ACIR opcode, the Noir source it was compiled +//! from. The annotations are meant to be attached as `//` comments when displaying +//! a circuit (see `display_program` in the `acir` crate). + +use std::collections::BTreeMap; +use std::path::Path; + +use acir::AcirField; +use acir::circuit::{AcirOpcodeLocation, Program}; +use fm::FileId; +use noirc_errors::Location; +use noirc_errors::call_stack::CallStack; +use noirc_errors::reporter::line_and_column_from_span; + +use crate::debug::{DebugFile, DebugInfo}; + +/// Builds the per-circuit opcode annotations (see [acir_opcode_annotations]) for every +/// ACIR function of a program. `debug` must be parallel to `program.functions`. +pub fn program_opcode_annotations( + program: &Program, + debug: &[DebugInfo], + file_map: &BTreeMap, +) -> Vec> { + program + .functions + .iter() + .zip(debug) + .map(|(circuit, debug_info)| { + acir_opcode_annotations(debug_info, file_map, circuit.opcodes.len()) + }) + .collect() +} + +/// Builds `opcode index -> comment` annotations describing the Noir source each opcode +/// of a single ACIR circuit was compiled from. +/// +/// Only the first opcode of a consecutive run sharing the same call stack is annotated, +/// so a source expression that expands to several opcodes gets a single comment. An +/// opcode with no recorded location that follows an annotated one is marked explicitly, +/// so that it doesn't appear to belong to the preceding run. +pub fn acir_opcode_annotations( + debug_info: &DebugInfo, + file_map: &BTreeMap, + num_opcodes: usize, +) -> BTreeMap { + let mut annotations = BTreeMap::new(); + let mut previous_call_stack = None; + let current_dir = std::env::current_dir().ok(); + + for index in 0..num_opcodes { + let call_stack_id = debug_info.acir_locations.get(&AcirOpcodeLocation::new(index)).copied(); + if call_stack_id == previous_call_stack { + continue; + } + // Opcodes before the first attributed one are left bare rather than marked + // as having no source location. + if call_stack_id.is_none() && previous_call_stack.is_none() { + continue; + } + previous_call_stack = call_stack_id; + + let annotation = call_stack_id.and_then(|call_stack_id| { + let call_stack = debug_info.location_tree.get_call_stack(call_stack_id); + format_call_stack_annotation(&call_stack, file_map, current_dir.as_deref()) + }); + annotations.insert(index, annotation.unwrap_or_else(|| "no source location".to_string())); + } + + annotations +} + +/// Formats a call stack as `file:line:col: snippet`, where the location and snippet are +/// those of the innermost frame. If the innermost frame was reached through inlined +/// calls, the callers are appended as ` (via caller1 <- caller2)`, innermost caller +/// first. Returns `None` if no frame has a resolvable location. +fn format_call_stack_annotation( + call_stack: &CallStack, + file_map: &BTreeMap, + current_dir: Option<&Path>, +) -> Option { + let locations: Vec<&Location> = + call_stack.into_iter().filter(|location| !location.is_dummy()).collect(); + let (innermost, callers) = locations.split_last()?; + + let mut annotation = format_location(innermost, file_map, current_dir, true)?; + + let callers = callers + .iter() + .rev() + .filter_map(|location| format_location(location, file_map, current_dir, false)) + .collect::>(); + if !callers.is_empty() { + annotation.push_str(&format!(" (via {})", callers.join(" <- "))); + } + + Some(annotation) +} + +/// Formats a single [Location] as `file:line:col`, appending `: snippet` when requested. +/// The file path is shown relative to the current directory when possible. The snippet +/// is the source text covered by the location's span, with whitespace runs (including +/// newlines) collapsed to single spaces and truncated to a maximum length. +/// Returns `None` if the location's file is not in the file map. +fn format_location( + location: &Location, + file_map: &BTreeMap, + current_dir: Option<&Path>, + with_snippet: bool, +) -> Option { + let file = file_map.get(&location.file)?; + let path = current_dir + .and_then(|current_dir| file.path.strip_prefix(current_dir).ok()) + .unwrap_or(&file.path); + let (line, column) = line_and_column_from_span(&file.source, &location.span); + let mut result = format!("{}:{line}:{column}", path.display()); + + if with_snippet + && let Some(snippet) = + file.source.get(location.span.start() as usize..location.span.end() as usize) + { + const MAX_SNIPPET_LENGTH: usize = 80; + + let snippet = snippet.split_whitespace().collect::>().join(" "); + result.push_str(": "); + if snippet.chars().count() > MAX_SNIPPET_LENGTH { + result.extend(snippet.chars().take(MAX_SNIPPET_LENGTH)); + result.push('…'); + } else { + result.push_str(&snippet); + } + } + + Some(result) +} diff --git a/tooling/noirc_artifacts/src/lib.rs b/tooling/noirc_artifacts/src/lib.rs index fec02eaf63e..eced2de649f 100644 --- a/tooling/noirc_artifacts/src/lib.rs +++ b/tooling/noirc_artifacts/src/lib.rs @@ -9,6 +9,7 @@ use serde::{Deserializer, Serializer, de::Visitor}; +pub mod annotations; pub mod contract; pub mod debug; mod debug_vars;