From 60057a3457af579e28b107b172e3d84334d5b14a Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 23 Jun 2025 14:27:08 -0500 Subject: [PATCH 1/8] Print ssa locations along with ssa --- Cargo.lock | 1 + compiler/noirc_driver/src/lib.rs | 8 +- compiler/noirc_evaluator/Cargo.toml | 1 + compiler/noirc_evaluator/src/ssa.rs | 47 +++++-- .../noirc_evaluator/src/ssa/ir/printer.rs | 121 ++++++++++++------ tooling/ast_fuzzer/src/compare/interpreted.rs | 7 +- tooling/nargo_cli/src/cli/interpret_cmd.rs | 9 +- 7 files changed, 133 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5c1449bfdf..f909ef9362d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3710,6 +3710,7 @@ dependencies = [ "bn254_blackbox_solver", "cfg-if", "chrono", + "fm", "function_name", "fxhash", "im", diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 71641e30142..aee6bf37715 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -790,9 +790,13 @@ pub fn compile_no_check( let SsaProgramArtifact { program, debug, warnings, names, brillig_names, error_types, .. } = if options.minimal_ssa { - create_program_with_minimal_passes(program, &ssa_evaluator_options)? + create_program_with_minimal_passes( + program, + &ssa_evaluator_options, + &context.file_manager, + )? } else { - create_program(program, &ssa_evaluator_options)? + create_program(program, &ssa_evaluator_options, &context.file_manager)? }; let abi = gen_abi(context, &main_function, return_visibility, error_types); diff --git a/compiler/noirc_evaluator/Cargo.toml b/compiler/noirc_evaluator/Cargo.toml index 2463858c50d..2ed87755718 100644 --- a/compiler/noirc_evaluator/Cargo.toml +++ b/compiler/noirc_evaluator/Cargo.toml @@ -34,6 +34,7 @@ cfg-if.workspace = true smallvec = { version = "1.13.2", features = ["serde"] } vec-collections = "0.4.3" petgraph.workspace = true +fm.workspace = true [dev-dependencies] proptest.workspace = true diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index d8c7581e863..2dfda2cb7a4 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -287,6 +287,7 @@ where let ssa_gen_span_guard = ssa_gen_span.enter(); let mut builder = builder.with_skip_passes(options.skip_passes.clone()).run_passes(primary)?; let passed = std::mem::take(&mut builder.passed); + let files = builder.files; let mut ssa = builder.finish(); let mut ssa_level_warnings = vec![]; @@ -300,12 +301,16 @@ where let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); let ssa_gen_span_guard = ssa_gen_span.enter(); - let mut ssa = - SsaBuilder::from_ssa(ssa, options.ssa_logging.clone(), options.print_codegen_timings) - .with_passed(passed) - .with_skip_passes(options.skip_passes.clone()) - .run_passes(&secondary(&brillig))? - .finish(); + let mut ssa = SsaBuilder::from_ssa( + ssa, + options.ssa_logging.clone(), + options.print_codegen_timings, + files, + ) + .with_passed(passed) + .with_skip_passes(options.skip_passes.clone()) + .run_passes(&secondary(&brillig))? + .finish(); if !options.skip_underconstrained_check { ssa_level_warnings.extend(time( @@ -353,6 +358,7 @@ pub fn optimize_into_acir( options: &SsaEvaluatorOptions, primary: &[SsaPass], secondary: S, + files: &fm::FileManager, ) -> Result where S: for<'b> Fn(&'b Brillig) -> Vec>, @@ -362,6 +368,7 @@ where options.ssa_logging.clone(), options.print_codegen_timings, &options.emit_ssa, + files, )?; optimize_ssa_builder_into_acir(builder, options, primary, secondary) @@ -436,8 +443,9 @@ impl SsaProgramArtifact { pub fn create_program( program: Program, options: &SsaEvaluatorOptions, + files: &fm::FileManager, ) -> Result { - create_program_with_passes(program, options, &primary_passes(options), secondary_passes) + create_program_with_passes(program, options, &primary_passes(options), secondary_passes, files) } /// Compiles the [`Program`] into [`ACIR`][acvm::acir::circuit::Program] using the minimum amount of SSA passes. @@ -448,6 +456,7 @@ pub fn create_program( pub fn create_program_with_minimal_passes( program: Program, options: &SsaEvaluatorOptions, + files: &fm::FileManager, ) -> Result { for func in &program.functions { assert!( @@ -456,7 +465,7 @@ pub fn create_program_with_minimal_passes( func.name ); } - create_program_with_passes(program, options, &minimal_passes(), |_| vec![]) + create_program_with_passes(program, options, &minimal_passes(), |_| vec![], files) } /// Compiles the [`Program`] into [`ACIR`][acvm::acir::circuit::Program] by running it through @@ -467,6 +476,7 @@ pub fn create_program_with_passes( options: &SsaEvaluatorOptions, primary: &[SsaPass], secondary: S, + files: &fm::FileManager, ) -> Result where S: for<'b> Fn(&'b Brillig) -> Vec>, @@ -480,7 +490,7 @@ where let ArtifactsAndWarnings( (generated_acirs, generated_brillig, brillig_function_names, error_types), ssa_level_warnings, - ) = optimize_into_acir(program, options, primary, secondary)?; + ) = optimize_into_acir(program, options, primary, secondary, files)?; assert_eq!( generated_acirs.len(), @@ -630,7 +640,7 @@ fn split_public_and_private_inputs( } // This is just a convenience object to bundle the ssa with `print_ssa_passes` for debug printing. -pub struct SsaBuilder { +pub struct SsaBuilder<'local> { /// The SSA being built; it is the input and the output of every pass ran by the builder. pub ssa: Ssa, /// Options to control which SSA passes to print. @@ -642,14 +652,17 @@ pub struct SsaBuilder { pub passed: HashMap, /// List of SSA pass message fragments that we want to skip, for testing purposes. pub skip_passes: Vec, + + pub files: &'local fm::FileManager, } -impl SsaBuilder { +impl<'local> SsaBuilder<'local> { pub fn from_program( program: Program, ssa_logging: SsaLogging, print_codegen_timings: bool, emit_ssa: &Option, + files: &'local fm::FileManager, ) -> Result { let ssa = ssa_gen::generate_ssa(program)?; if let Some(emit_ssa) = emit_ssa { @@ -661,14 +674,20 @@ impl SsaBuilder { let ssa_path = emit_ssa.with_extension("ssa.json"); write_to_file(&serde_json::to_vec(&ssa).unwrap(), &ssa_path); } - Ok(Self::from_ssa(ssa, ssa_logging, print_codegen_timings).print("Initial SSA")) + Ok(Self::from_ssa(ssa, ssa_logging, print_codegen_timings, files).print("Initial SSA")) } - pub fn from_ssa(ssa: Ssa, ssa_logging: SsaLogging, print_codegen_timings: bool) -> Self { + pub fn from_ssa( + ssa: Ssa, + ssa_logging: SsaLogging, + print_codegen_timings: bool, + files: &'local fm::FileManager, + ) -> Self { Self { ssa_logging, print_codegen_timings, ssa, + files, passed: Default::default(), skip_passes: Default::default(), } @@ -745,7 +764,7 @@ impl SsaBuilder { }; if print_ssa_pass { - println!("After {msg}:\n{}", self.ssa); + println!("After {msg}:\n{}", self.ssa.print_with_files(self.files)); } self } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 3c19b322a00..188be9a2b0e 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -2,6 +2,7 @@ use std::fmt::{Display, Formatter, Result}; use acvm::acir::AcirField; +use fm::codespan_files; use im::Vector; use iter_extended::vecmap; @@ -21,9 +22,27 @@ use super::{ value::{Value, ValueId}, }; -impl Display for Ssa { - fn fmt(&self, f: &mut Formatter<'_>) -> Result { - let globals = (*self.functions[&self.main_id].dfg.globals).clone(); +pub struct Printer<'local> { + ssa: &'local Ssa, + fm: Option<&'local fm::FileManager>, +} + +impl Ssa { + pub fn print_without_files<'local>(&'local self) -> Printer<'local> { + Printer { ssa: self, fm: None } + } + + pub fn print_with_files<'local>( + &'local self, + files: &'local fm::FileManager, + ) -> Printer<'local> { + Printer { ssa: self, fm: Some(files) } + } +} + +impl<'local> Display for Printer<'local> { + fn fmt(&self, f: &mut Formatter) -> Result { + let globals = (*self.ssa.functions[&self.ssa.main_id].dfg.globals).clone(); let globals_dfg = DataFlowGraph::from(globals); for (id, global_value) in globals_dfg.values_iter() { @@ -32,7 +51,7 @@ impl Display for Ssa { writeln!(f, "g{} = {typ} {constant}", id.to_u32())?; } Value::Instruction { instruction, .. } => { - display_instruction(&globals_dfg, *instruction, true, f)?; + display_instruction(&globals_dfg, *instruction, true, self.fm, f)?; } Value::Global(_) => { panic!("Value::Global should only be in the function dfg"); @@ -48,8 +67,9 @@ impl Display for Ssa { writeln!(f)?; } - for function in self.functions.values() { - writeln!(f, "{function}")?; + for function in self.ssa.functions.values() { + display_function(function, self.fm, f)?; + writeln!(f)?; } Ok(()) } @@ -57,12 +77,16 @@ impl Display for Ssa { impl Display for Function { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - display_function(self, f) + display_function(self, None, f) } } /// Helper function for Function's Display impl to pretty-print the function with the given formatter. -fn display_function(function: &Function, f: &mut Formatter) -> Result { +fn display_function( + function: &Function, + files: Option<&fm::FileManager>, + f: &mut Formatter, +) -> Result { if let Some(purity) = function.dfg.purity_of(function.id()) { writeln!(f, "{} {purity} fn {} {} {{", function.runtime(), function.name(), function.id())?; } else { @@ -70,19 +94,24 @@ fn display_function(function: &Function, f: &mut Formatter) -> Result { } for block_id in function.reachable_blocks() { - display_block(&function.dfg, block_id, f)?; + display_block(&function.dfg, block_id, files, f)?; } write!(f, "}}") } /// Display a single block. This will not display the block's successors. -fn display_block(dfg: &DataFlowGraph, block_id: BasicBlockId, f: &mut Formatter) -> Result { +fn display_block( + dfg: &DataFlowGraph, + block_id: BasicBlockId, + fm: Option<&fm::FileManager>, + f: &mut Formatter, +) -> Result { let block = &dfg[block_id]; writeln!(f, " {}({}):", block_id, value_list_with_types(dfg, block.parameters()))?; for instruction in block.instructions() { - display_instruction(dfg, *instruction, false, f)?; + display_instruction(dfg, *instruction, false, fm, f)?; } display_terminator(dfg, block.terminator(), f) @@ -166,6 +195,7 @@ fn display_instruction( dfg: &DataFlowGraph, instruction: InstructionId, in_global_space: bool, + fm: Option<&fm::FileManager>, f: &mut Formatter, ) -> Result { if !in_global_space { @@ -182,7 +212,24 @@ fn display_instruction( write!(f, "{} = ", value_list)?; } - display_instruction_inner(dfg, &dfg[instruction], results, in_global_space, f) + display_instruction_inner(dfg, &dfg[instruction], results, in_global_space, f)?; + + if let Some(fm) = fm { + let call_stack = dfg.get_instruction_call_stack(instruction); + if let Some(location) = call_stack.last() { + if let Ok(name) = fm.as_file_map().get_name(location.file) { + let files = fm.as_file_map(); + let start_index = location.span.start() as usize; + match codespan_files::Files::line_index(files, location.file, start_index) { + // Offset line_index by 1 to convert it into a line number + Ok(line_index) => write!(f, "\t// {name}:{}", line_index + 1)?, + // Showing the file name alone is better than nothing + Err(_) => write!(f, "\t// {name}")?, + } + } + } + } + writeln!(f) } fn display_instruction_inner( @@ -196,48 +243,40 @@ fn display_instruction_inner( match instruction { Instruction::Binary(binary) => { - writeln!(f, "{}", display_binary(binary, dfg)) + write!(f, "{}", display_binary(binary, dfg)) } - Instruction::Cast(lhs, typ) => writeln!(f, "cast {} as {typ}", show(*lhs)), - Instruction::Not(rhs) => writeln!(f, "not {}", show(*rhs)), + Instruction::Cast(lhs, typ) => write!(f, "cast {} as {typ}", show(*lhs)), + Instruction::Not(rhs) => write!(f, "not {}", show(*rhs)), Instruction::Truncate { value, bit_size, max_bit_size } => { let value = show(*value); - writeln!(f, "truncate {value} to {bit_size} bits, max_bit_size: {max_bit_size}",) + write!(f, "truncate {value} to {bit_size} bits, max_bit_size: {max_bit_size}",) } Instruction::Constrain(lhs, rhs, error) => { write!(f, "constrain {} == {}", show(*lhs), show(*rhs))?; - if let Some(error) = error { - display_constrain_error(dfg, error, f) - } else { - writeln!(f) - } + if let Some(error) = error { display_constrain_error(dfg, error, f) } else { Ok(()) } } Instruction::ConstrainNotEqual(lhs, rhs, error) => { write!(f, "constrain {} != {}", show(*lhs), show(*rhs))?; - if let Some(error) = error { - display_constrain_error(dfg, error, f) - } else { - writeln!(f) - } + if let Some(error) = error { display_constrain_error(dfg, error, f) } else { Ok(()) } } Instruction::Call { func, arguments } => { let arguments = value_list(dfg, arguments); - writeln!(f, "call {}({}){}", show(*func), arguments, result_types(dfg, results)) + write!(f, "call {}({}){}", show(*func), arguments, result_types(dfg, results)) } Instruction::Allocate => { - writeln!(f, "allocate{}", result_types(dfg, results)) + write!(f, "allocate{}", result_types(dfg, results)) } Instruction::Load { address } => { - writeln!(f, "load {}{}", show(*address), result_types(dfg, results)) + write!(f, "load {}{}", show(*address), result_types(dfg, results)) } Instruction::Store { address, value } => { - writeln!(f, "store {} at {}", show(*value), show(*address)) + write!(f, "store {} at {}", show(*value), show(*address)) } Instruction::EnableSideEffectsIf { condition } => { - writeln!(f, "enable_side_effects {}", show(*condition)) + write!(f, "enable_side_effects {}", show(*condition)) } Instruction::ArrayGet { array, index, offset } => { - writeln!( + write!( f, "array_get {}, index {}{}{}", show(*array), @@ -251,7 +290,7 @@ fn display_instruction_inner( let index = show(*index); let value = show(*value); let mutable = if *mutable { " mut" } else { "" }; - writeln!( + write!( f, "array_set{} {}, index {}{}, value {}", mutable, @@ -262,22 +301,22 @@ fn display_instruction_inner( ) } Instruction::IncrementRc { value } => { - writeln!(f, "inc_rc {}", show(*value)) + write!(f, "inc_rc {}", show(*value)) } Instruction::DecrementRc { value } => { - writeln!(f, "dec_rc {}", show(*value)) + write!(f, "dec_rc {}", show(*value)) } Instruction::RangeCheck { value, max_bit_size, assert_message } => { let message = assert_message.as_ref().map(|message| format!(", {message:?}")).unwrap_or_default(); - writeln!(f, "range_check {} to {} bits{}", show(*value), *max_bit_size, message) + write!(f, "range_check {} to {} bits{}", show(*value), *max_bit_size, message) } Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { let then_condition = show(*then_condition); let then_value = show(*then_value); let else_condition = show(*else_condition); let else_value = show(*else_value); - writeln!( + write!( f, "if {then_condition} then {then_value} else (if {else_condition}) {else_value}" ) @@ -298,9 +337,9 @@ fn display_instruction_inner( { if let Some(string) = try_byte_array_to_string(elements, dfg) { if is_slice { - return writeln!(f, "make_array &b{:?}", string); + return write!(f, "make_array &b{:?}", string); } else { - return writeln!(f, "make_array b{:?}", string); + return write!(f, "make_array b{:?}", string); } } } @@ -318,9 +357,9 @@ fn display_instruction_inner( write!(f, "{}", value)?; } - writeln!(f, "] : {typ}") + write!(f, "] : {typ}") } - Instruction::Noop => writeln!(f, "nop"), + Instruction::Noop => write!(f, "nop"), } } diff --git a/tooling/ast_fuzzer/src/compare/interpreted.rs b/tooling/ast_fuzzer/src/compare/interpreted.rs index 5b36658c98c..be96820c2cd 100644 --- a/tooling/ast_fuzzer/src/compare/interpreted.rs +++ b/tooling/ast_fuzzer/src/compare/interpreted.rs @@ -89,7 +89,12 @@ impl CompareInterpreted { .collect::>() .join("\n") ); - log::debug!("SSA after step {} ({}):\n{}\n", self.ssa1.step, self.ssa1.msg, self.ssa1.ssa); + log::debug!( + "SSA after step {} ({}):\n{}\n", + self.ssa1.step, + self.ssa1.msg, + self.ssa1.ssa.print_without_files() + ); // Interpret an SSA with a fresh copy of the input values. let interpret = |ssa: &Ssa| ssa.interpret(Value::snapshot_args(&self.ssa_args)); diff --git a/tooling/nargo_cli/src/cli/interpret_cmd.rs b/tooling/nargo_cli/src/cli/interpret_cmd.rs index 6c48daecfe6..567fdf495a0 100644 --- a/tooling/nargo_cli/src/cli/interpret_cmd.rs +++ b/tooling/nargo_cli/src/cli/interpret_cmd.rs @@ -135,6 +135,7 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl &ssa_args, &ssa_return, interpreter_options, + &file_manager, )?; // Run SSA passes in the pipeline and interpret the ones we are interested in. @@ -157,6 +158,7 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl &ssa_args, &ssa_return, interpreter_options, + &file_manager, )?; } } @@ -243,7 +245,7 @@ fn msg_matches(patterns: &[String], msg: &str) -> bool { patterns.iter().any(|p| msg.contains(&p.to_lowercase())) } -fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str) { +fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str, fm: &FileManager) { let print = match options.ssa_logging { SsaLogging::All => true, SsaLogging::None => false, @@ -251,7 +253,7 @@ fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str) { }; if print { ssa.normalize_ids(); - println!("After {msg}:\n{ssa}"); + println!("After {msg}:\n{}", ssa.print_with_files(fm)); } } @@ -289,8 +291,9 @@ fn print_and_interpret_ssa( args: &[Value], return_value: &Option>, interpreter_options: InterpreterOptions, + fm: &FileManager, ) -> Result<(), CliError> { - print_ssa(options, ssa, msg); + print_ssa(options, ssa, msg, fm); interpret_ssa(passes_to_interpret, ssa, msg, args, return_value, interpreter_options) } From 5306614640bdb3f6c02f90ad7e4b588d32c07d46 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 23 Jun 2025 14:50:12 -0500 Subject: [PATCH 2/8] Update more files --- compiler/noirc_evaluator/src/acir/tests/mod.rs | 2 -- compiler/noirc_evaluator/src/ssa.rs | 9 ++++++--- compiler/noirc_evaluator/src/ssa/ir/printer.rs | 12 ++++++------ .../noirc_evaluator/src/ssa/opt/defunctionalize.rs | 1 - compiler/noirc_evaluator/src/ssa/opt/hint.rs | 3 +-- compiler/noirc_evaluator/src/ssa/opt/mod.rs | 7 ++++--- compiler/noirc_evaluator/src/ssa/opt/unrolling.rs | 5 +++-- compiler/noirc_evaluator/src/ssa/parser/tests.rs | 2 +- compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs | 2 +- tooling/ast_fuzzer/src/compare/interpreted.rs | 2 +- tooling/nargo_cli/examples/ssa_pass_impact.rs | 9 +++++++-- tooling/nargo_cli/src/cli/interpret_cmd.rs | 2 +- 12 files changed, 31 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/tests/mod.rs b/compiler/noirc_evaluator/src/acir/tests/mod.rs index 5c1bdfe0c98..9c2d07b891f 100644 --- a/compiler/noirc_evaluator/src/acir/tests/mod.rs +++ b/compiler/noirc_evaluator/src/acir/tests/mod.rs @@ -560,7 +560,6 @@ fn brillig_stdlib_calls_with_regular_brillig_call() { let ssa = builder.finish(); // We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass. let brillig = ssa.to_brillig(&BrilligOptions::default()); - println!("{}", ssa); let (acir_functions, brillig_functions, _, _) = ssa .generate_entry_point_index() @@ -645,7 +644,6 @@ fn brillig_stdlib_calls_with_multiple_acir_calls() { let ssa = builder.finish(); // We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass. let brillig = ssa.to_brillig(&BrilligOptions::default()); - println!("{}", ssa); let (acir_functions, brillig_functions, _, _) = ssa .generate_entry_point_index() diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 2dfda2cb7a4..2c5c9b0bd4a 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -653,7 +653,9 @@ pub struct SsaBuilder<'local> { /// List of SSA pass message fragments that we want to skip, for testing purposes. pub skip_passes: Vec, - pub files: &'local fm::FileManager, + /// Providing a file manager is optional - if provided it can be used to print source + /// locations along with each ssa instructions when debugging. + pub files: Option<&'local fm::FileManager>, } impl<'local> SsaBuilder<'local> { @@ -674,14 +676,15 @@ impl<'local> SsaBuilder<'local> { let ssa_path = emit_ssa.with_extension("ssa.json"); write_to_file(&serde_json::to_vec(&ssa).unwrap(), &ssa_path); } - Ok(Self::from_ssa(ssa, ssa_logging, print_codegen_timings, files).print("Initial SSA")) + Ok(Self::from_ssa(ssa, ssa_logging, print_codegen_timings, Some(files)) + .print("Initial SSA")) } pub fn from_ssa( ssa: Ssa, ssa_logging: SsaLogging, print_codegen_timings: bool, - files: &'local fm::FileManager, + files: Option<&'local fm::FileManager>, ) -> Self { Self { ssa_logging, diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 188be9a2b0e..160899a4ae5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -28,15 +28,15 @@ pub struct Printer<'local> { } impl Ssa { - pub fn print_without_files<'local>(&'local self) -> Printer<'local> { + pub fn print_without_locations<'local>(&'local self) -> Printer<'local> { Printer { ssa: self, fm: None } } pub fn print_with_files<'local>( &'local self, - files: &'local fm::FileManager, + files: Option<&'local fm::FileManager>, ) -> Printer<'local> { - Printer { ssa: self, fm: Some(files) } + Printer { ssa: self, fm: files } } } @@ -424,15 +424,15 @@ fn display_constrain_error( ) -> Result { match error { ConstrainError::StaticString(assert_message_string) => { - writeln!(f, ", {assert_message_string:?}") + write!(f, ", {assert_message_string:?}") } ConstrainError::Dynamic(_, is_string, values) => { if let Some(constant_string) = try_to_extract_string_from_error_payload(*is_string, values, dfg) { - writeln!(f, ", {constant_string:?}") + write!(f, ", {constant_string:?}") } else { - writeln!(f, ", data {}", value_list(dfg, values)) + write!(f, ", data {}", value_list(dfg, values)) } } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs index 2e341d4b41d..9af9f43cdf0 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs @@ -1318,7 +1318,6 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.defunctionalize(); - println!("{}", ssa); assert_ssa_snapshot!(ssa, @r" acir(inline) fn main f0 { diff --git a/compiler/noirc_evaluator/src/ssa/opt/hint.rs b/compiler/noirc_evaluator/src/ssa/opt/hint.rs index 1a6556a9ad1..7ccc3255a33 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/hint.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/hint.rs @@ -24,8 +24,7 @@ mod tests { skip_passes: Default::default(), }; - let builder = SsaBuilder::from_ssa(ssa, options.ssa_logging.clone(), false); - + let builder = SsaBuilder::from_ssa(ssa, options.ssa_logging.clone(), false, None); Ok(builder.run_passes(&primary_passes(options))?.finish()) } diff --git a/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/mod.rs index d5f8386d990..5ec76642da9 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -64,10 +64,10 @@ pub(crate) fn assert_normalized_ssa_equals(mut ssa: super::Ssa, expected: &str) ssa.normalize_ids(); - let ssa = ssa.to_string(); + let ssa = ssa.print_without_locations().to_string(); let ssa = ssa.trim_end(); - let expected_ssa = expected_ssa.to_string(); + let expected_ssa = expected_ssa.print_without_locations().to_string(); let expected_ssa = expected_ssa.trim_end(); if ssa == expected_ssa { @@ -112,6 +112,7 @@ macro_rules! assert_ssa_snapshot { #[allow(unused_mut)] let mut mut_ssa = $ssa; mut_ssa.normalize_ids(); - insta::assert_snapshot!(mut_ssa, $($arg)*) + let ssa_string = mut_ssa.print_without_locations().to_string(); + insta::assert_snapshot!(ssa_string, $($arg)*) }; } diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index 4790aad91e6..d9ba435005e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -1328,7 +1328,7 @@ mod tests { let (ssa, errors) = try_unroll_loops(ssa); assert_eq!(errors.len(), 0, "Unroll should have no errors"); // Check that it's still the original - assert_normalized_ssa_equals(ssa, parse_ssa().to_string().as_str()); + assert_normalized_ssa_equals(ssa, &parse_ssa().print_without_locations().to_string()); } #[test] @@ -1336,7 +1336,8 @@ mod tests { let ssa = brillig_unroll_test_case(); let ssa = ssa.unroll_loops_iteratively(Some(-90)).unwrap(); // Check that it's still the original - assert_normalized_ssa_equals(ssa, brillig_unroll_test_case().to_string().as_str()); + let expected = brillig_unroll_test_case(); + assert_normalized_ssa_equals(ssa, &expected.print_without_locations().to_string()); } #[test] diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index cf68edd31b5..a1aeddfd355 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -7,7 +7,7 @@ use crate::{ fn assert_ssa_roundtrip(src: &str) { let ssa = Ssa::from_str(src).unwrap(); - let ssa = ssa.to_string(); + let ssa = ssa.print_without_locations().to_string(); let ssa = trim_leading_whitespace_from_lines(&ssa); let src = trim_leading_whitespace_from_lines(src); if ssa != src { diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 706ea8fa113..0cd8737880e 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -131,7 +131,7 @@ mod test { let ssa = builder.finish(); let serialized_ssa = &serde_json::to_string(&ssa).unwrap(); let deserialized_ssa: Ssa = serde_json::from_str(serialized_ssa).unwrap(); - let actual_string = format!("{}", deserialized_ssa); + let actual_string = format!("{}", deserialized_ssa.print_without_locations()); let expected_string = "acir(inline) fn main f0 {\n \ b0(v0: Field):\n \ diff --git a/tooling/ast_fuzzer/src/compare/interpreted.rs b/tooling/ast_fuzzer/src/compare/interpreted.rs index be96820c2cd..4ff3adce735 100644 --- a/tooling/ast_fuzzer/src/compare/interpreted.rs +++ b/tooling/ast_fuzzer/src/compare/interpreted.rs @@ -93,7 +93,7 @@ impl CompareInterpreted { "SSA after step {} ({}):\n{}\n", self.ssa1.step, self.ssa1.msg, - self.ssa1.ssa.print_without_files() + self.ssa1.ssa.print_without_locations() ); // Interpret an SSA with a fresh copy of the input values. diff --git a/tooling/nargo_cli/examples/ssa_pass_impact.rs b/tooling/nargo_cli/examples/ssa_pass_impact.rs index 64d65adcd16..582ff961ac0 100644 --- a/tooling/nargo_cli/examples/ssa_pass_impact.rs +++ b/tooling/nargo_cli/examples/ssa_pass_impact.rs @@ -275,12 +275,17 @@ fn collect_ssa_before_and_after( let mut last_msg = "Initial"; for (i, pass) in passes.iter().enumerate() { - let before = pass.msg().contains(name).then(|| format!("{ssa}")); + let before = + pass.msg().contains(name).then(|| format!("{}", ssa.print_without_locations())); ssa = pass.run(ssa)?; if let Some(before) = before { pairs.push(SsaBeforeAndAfter { before: SsaPrint { step: i, msg: last_msg.to_string(), ssa: before }, - after: SsaPrint { step: i + 1, msg: pass.msg().to_string(), ssa: format!("{ssa}") }, + after: SsaPrint { + step: i + 1, + msg: pass.msg().to_string(), + ssa: format!("{}", ssa.print_without_locations()), + }, }); } last_msg = pass.msg(); diff --git a/tooling/nargo_cli/src/cli/interpret_cmd.rs b/tooling/nargo_cli/src/cli/interpret_cmd.rs index 567fdf495a0..ed14feedb2e 100644 --- a/tooling/nargo_cli/src/cli/interpret_cmd.rs +++ b/tooling/nargo_cli/src/cli/interpret_cmd.rs @@ -253,7 +253,7 @@ fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str, fm: &FileM }; if print { ssa.normalize_ids(); - println!("After {msg}:\n{}", ssa.print_with_files(fm)); + println!("After {msg}:\n{}", ssa.print_with_files(Some(fm))); } } From 5f8e242b82787175611008221eb27f2e57042ae3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 23 Jun 2025 14:53:43 -0500 Subject: [PATCH 3/8] More updates --- compiler/noirc_evaluator/src/ssa.rs | 2 +- compiler/noirc_evaluator/src/ssa/ir/printer.rs | 2 +- tooling/nargo_cli/src/cli/interpret_cmd.rs | 2 +- tooling/ssa_verification/src/acir_instruction_builder.rs | 5 +++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 2c5c9b0bd4a..697cbf791e1 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -767,7 +767,7 @@ impl<'local> SsaBuilder<'local> { }; if print_ssa_pass { - println!("After {msg}:\n{}", self.ssa.print_with_files(self.files)); + println!("After {msg}:\n{}", self.ssa.print_with(self.files)); } self } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 160899a4ae5..d93efd81e76 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -32,7 +32,7 @@ impl Ssa { Printer { ssa: self, fm: None } } - pub fn print_with_files<'local>( + pub fn print_with<'local>( &'local self, files: Option<&'local fm::FileManager>, ) -> Printer<'local> { diff --git a/tooling/nargo_cli/src/cli/interpret_cmd.rs b/tooling/nargo_cli/src/cli/interpret_cmd.rs index ed14feedb2e..22633ba6361 100644 --- a/tooling/nargo_cli/src/cli/interpret_cmd.rs +++ b/tooling/nargo_cli/src/cli/interpret_cmd.rs @@ -253,7 +253,7 @@ fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str, fm: &FileM }; if print { ssa.normalize_ids(); - println!("After {msg}:\n{}", ssa.print_with_files(Some(fm))); + println!("After {msg}:\n{}", ssa.print_with(Some(fm))); } } diff --git a/tooling/ssa_verification/src/acir_instruction_builder.rs b/tooling/ssa_verification/src/acir_instruction_builder.rs index 7ac61aed509..5f2b1c5886b 100644 --- a/tooling/ssa_verification/src/acir_instruction_builder.rs +++ b/tooling/ssa_verification/src/acir_instruction_builder.rs @@ -86,7 +86,7 @@ impl InstructionArtifacts { let second_variable_type = Self::get_type(second_variable); let ssa = binary_function(op, first_variable_type, second_variable_type); let serialized_ssa = &serde_json::to_string(&ssa).unwrap(); - let formatted_ssa = format!("{}", ssa); + let formatted_ssa = format!("{}", ssa.print_without_locations()); let program = ssa_to_acir_program(ssa); let serialized_program = AcirProgram::serialize_program(&program); @@ -118,7 +118,7 @@ impl InstructionArtifacts { fn new_by_ssa(ssa: Ssa, instruction_name: String, variable: &Variable) -> Self { let serialized_ssa = &serde_json::to_string(&ssa).unwrap(); - let formatted_ssa = format!("{}", ssa); + let formatted_ssa = format!("{}", ssa.print_without_locations()); let program = ssa_to_acir_program(ssa); let serialized_program = AcirProgram::serialize_program(&program); @@ -239,6 +239,7 @@ fn ssa_to_acir_program(ssa: Ssa) -> AcirProgram { print_codegen_timings: false, passed: HashMap::default(), skip_passes: vec![], + files: None, }; let ssa_evaluator_options = SsaEvaluatorOptions { ssa_logging: SsaLogging::None, From 705e4c9ebd63bf2f134923c123fd90356c0251c2 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 23 Jun 2025 15:02:22 -0500 Subject: [PATCH 4/8] Clippy --- compiler/noirc_driver/src/lib.rs | 2 +- compiler/noirc_evaluator/src/ssa.rs | 13 +++++------ .../noirc_evaluator/src/ssa/ir/printer.rs | 4 ++-- tooling/ast_fuzzer/fuzz/src/lib.rs | 22 ++++++++++++------- tooling/ast_fuzzer/tests/parser.rs | 13 ++++++----- tooling/ast_fuzzer/tests/smoke.rs | 2 +- tooling/nargo_cli/src/cli/interpret_cmd.rs | 1 + tooling/ssa_executor/src/compiler.rs | 1 + tooling/ssa_fuzzer/src/compiler.rs | 2 +- 9 files changed, 34 insertions(+), 26 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index aee6bf37715..1e917452215 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -796,7 +796,7 @@ pub fn compile_no_check( &context.file_manager, )? } else { - create_program(program, &ssa_evaluator_options, &context.file_manager)? + create_program(program, &ssa_evaluator_options, Some(&context.file_manager))? }; let abi = gen_abi(context, &main_function, return_visibility, error_types); diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 697cbf791e1..d0bf4022124 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -358,7 +358,7 @@ pub fn optimize_into_acir( options: &SsaEvaluatorOptions, primary: &[SsaPass], secondary: S, - files: &fm::FileManager, + files: Option<&fm::FileManager>, ) -> Result where S: for<'b> Fn(&'b Brillig) -> Vec>, @@ -443,7 +443,7 @@ impl SsaProgramArtifact { pub fn create_program( program: Program, options: &SsaEvaluatorOptions, - files: &fm::FileManager, + files: Option<&fm::FileManager>, ) -> Result { create_program_with_passes(program, options, &primary_passes(options), secondary_passes, files) } @@ -465,7 +465,7 @@ pub fn create_program_with_minimal_passes( func.name ); } - create_program_with_passes(program, options, &minimal_passes(), |_| vec![], files) + create_program_with_passes(program, options, &minimal_passes(), |_| vec![], Some(files)) } /// Compiles the [`Program`] into [`ACIR`][acvm::acir::circuit::Program] by running it through @@ -476,7 +476,7 @@ pub fn create_program_with_passes( options: &SsaEvaluatorOptions, primary: &[SsaPass], secondary: S, - files: &fm::FileManager, + files: Option<&fm::FileManager>, ) -> Result where S: for<'b> Fn(&'b Brillig) -> Vec>, @@ -664,7 +664,7 @@ impl<'local> SsaBuilder<'local> { ssa_logging: SsaLogging, print_codegen_timings: bool, emit_ssa: &Option, - files: &'local fm::FileManager, + files: Option<&'local fm::FileManager>, ) -> Result { let ssa = ssa_gen::generate_ssa(program)?; if let Some(emit_ssa) = emit_ssa { @@ -676,8 +676,7 @@ impl<'local> SsaBuilder<'local> { let ssa_path = emit_ssa.with_extension("ssa.json"); write_to_file(&serde_json::to_vec(&ssa).unwrap(), &ssa_path); } - Ok(Self::from_ssa(ssa, ssa_logging, print_codegen_timings, Some(files)) - .print("Initial SSA")) + Ok(Self::from_ssa(ssa, ssa_logging, print_codegen_timings, files).print("Initial SSA")) } pub fn from_ssa( diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index d93efd81e76..975913e68aa 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -28,7 +28,7 @@ pub struct Printer<'local> { } impl Ssa { - pub fn print_without_locations<'local>(&'local self) -> Printer<'local> { + pub fn print_without_locations(&self) -> Printer { Printer { ssa: self, fm: None } } @@ -40,7 +40,7 @@ impl Ssa { } } -impl<'local> Display for Printer<'local> { +impl Display for Printer<'_> { fn fmt(&self, f: &mut Formatter) -> Result { let globals = (*self.ssa.functions[&self.ssa.main_id].dfg.globals).clone(); let globals_dfg = DataFlowGraph::from(globals); diff --git a/tooling/ast_fuzzer/fuzz/src/lib.rs b/tooling/ast_fuzzer/fuzz/src/lib.rs index 4fd77063757..31e63f95253 100644 --- a/tooling/ast_fuzzer/fuzz/src/lib.rs +++ b/tooling/ast_fuzzer/fuzz/src/lib.rs @@ -77,12 +77,14 @@ where eprintln!("---\n{}\n---", DisplayAstAsNoir(&program)); } - ssa::create_program_with_passes(program, options, primary, secondary).unwrap_or_else(|e| { - panic!( - "failed to compile program: {}{e}", - msg.map(|s| format!("{s}: ")).unwrap_or_default() - ) - }) + ssa::create_program_with_passes(program, options, primary, secondary, None).unwrap_or_else( + |e| { + panic!( + "failed to compile program: {}{e}", + msg.map(|s| format!("{s}: ")).unwrap_or_default() + ) + }, + ) } /// Compare the execution result and print the inputs if the result is a failure. @@ -193,11 +195,15 @@ pub fn compare_results_interpreted( eprintln!( "---\nSSA 1 after step {} ({}):\n{}", - inputs.ssa1.step, inputs.ssa1.msg, inputs.ssa1.ssa + inputs.ssa1.step, + inputs.ssa1.msg, + inputs.ssa2.ssa.print_without_locations() ); eprintln!( "---\nSSA 2 after step {} ({}):\n{}", - inputs.ssa2.step, inputs.ssa2.msg, inputs.ssa2.ssa + inputs.ssa2.step, + inputs.ssa2.msg, + inputs.ssa2.ssa.print_without_locations() ); // Returning it as-is, so we can see the error message at the bottom as well. diff --git a/tooling/ast_fuzzer/tests/parser.rs b/tooling/ast_fuzzer/tests/parser.rs index 7be3cb9b2f7..80747cca682 100644 --- a/tooling/ast_fuzzer/tests/parser.rs +++ b/tooling/ast_fuzzer/tests/parser.rs @@ -76,12 +76,13 @@ fn arb_ssa_roundtrip() { ssa1.normalize_ids(); // Print to str and parse back. - let ssa2 = Ssa::from_str_no_validation(&ssa1.to_string()).unwrap_or_else(|e| { - let msg = passes.last().map(|p| p.msg()).unwrap_or("Initial SSA"); - print_ast_and_panic(&format!( - "Could not parse SSA after step {last_pass} ({msg}): \n{e:?}" - )) - }); + let ssa2 = Ssa::from_str_no_validation(&ssa1.print_without_locations().to_string()) + .unwrap_or_else(|e| { + let msg = passes.last().map(|p| p.msg()).unwrap_or("Initial SSA"); + print_ast_and_panic(&format!( + "Could not parse SSA after step {last_pass} ({msg}): \n{e:?}" + )) + }); // Not everything is populated by the parser, and unfortunately serializing to JSON doesn't work either. for (func_id, func1) in ssa1.functions { diff --git a/tooling/ast_fuzzer/tests/smoke.rs b/tooling/ast_fuzzer/tests/smoke.rs index 48917c93679..06a72aaa49f 100644 --- a/tooling/ast_fuzzer/tests/smoke.rs +++ b/tooling/ast_fuzzer/tests/smoke.rs @@ -59,7 +59,7 @@ fn arb_program_can_be_executed() { eprintln!("{}", DisplayAstAsNoir(&program)); } - let ssa = ssa::create_program(program.clone(), &options) + let ssa = ssa::create_program(program.clone(), &options, None) .unwrap_or_else(|e| print_ast_and_panic(&format!("Failed to compile program: {e}"))); let inputs = arb_inputs(u, &ssa.program, &abi)?; diff --git a/tooling/nargo_cli/src/cli/interpret_cmd.rs b/tooling/nargo_cli/src/cli/interpret_cmd.rs index 22633ba6361..8f47d7d9264 100644 --- a/tooling/nargo_cli/src/cli/interpret_cmd.rs +++ b/tooling/nargo_cli/src/cli/interpret_cmd.rs @@ -283,6 +283,7 @@ fn interpret_ssa( Ok(()) } +#[allow(clippy::too_many_arguments)] fn print_and_interpret_ssa( options: &SsaEvaluatorOptions, passes_to_interpret: &[String], diff --git a/tooling/ssa_executor/src/compiler.rs b/tooling/ssa_executor/src/compiler.rs index 28d95e512d9..648cd3eae6b 100644 --- a/tooling/ssa_executor/src/compiler.rs +++ b/tooling/ssa_executor/src/compiler.rs @@ -57,6 +57,7 @@ pub fn optimize_ssa_into_acir( print_codegen_timings: options.print_codegen_timings, passed: HashMap::new(), skip_passes: vec![], + files: None, }; optimize_ssa_builder_into_acir( builder, diff --git a/tooling/ssa_fuzzer/src/compiler.rs b/tooling/ssa_fuzzer/src/compiler.rs index 9a13d84e294..dd2a4f7f169 100644 --- a/tooling/ssa_fuzzer/src/compiler.rs +++ b/tooling/ssa_fuzzer/src/compiler.rs @@ -18,7 +18,7 @@ fn optimize_into_acir_and_validate( options: SsaEvaluatorOptions, ) -> Result { let ssa = builder.finish(); - log::debug!("SSA: {:}", ssa); + log::debug!("SSA: {:}", ssa.print_without_locations()); optimize_ssa_into_acir(ssa, options) } From 688b19f037cecc0209b2db76ef1b6b8c228232eb Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 23 Jun 2025 15:09:46 -0500 Subject: [PATCH 5/8] Two tab characters --- compiler/noirc_evaluator/src/ssa/ir/printer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 975913e68aa..e861e5e3e75 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -222,9 +222,9 @@ fn display_instruction( let start_index = location.span.start() as usize; match codespan_files::Files::line_index(files, location.file, start_index) { // Offset line_index by 1 to convert it into a line number - Ok(line_index) => write!(f, "\t// {name}:{}", line_index + 1)?, + Ok(line_index) => write!(f, "\t\t// {name}:{}", line_index + 1)?, // Showing the file name alone is better than nothing - Err(_) => write!(f, "\t// {name}")?, + Err(_) => write!(f, "\t\t// {name}")?, } } } From 3ef959ae60e41e148b7ec9cce58d1210e50cce03 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 24 Jun 2025 07:55:59 -0500 Subject: [PATCH 6/8] Align locations better by padding to minimum len --- .../noirc_evaluator/src/ssa/ir/printer.rs | 49 +++++++++++++++---- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index e861e5e3e75..98b57a84624 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -198,9 +198,26 @@ fn display_instruction( fm: Option<&fm::FileManager>, f: &mut Formatter, ) -> Result { + match display_instruction_buffer(dfg, instruction, in_global_space, fm) { + Ok(string) => write!(f, "{string}"), + Err(_) => Err(std::fmt::Error::default()), + } +} + +fn display_instruction_buffer( + dfg: &DataFlowGraph, + instruction: InstructionId, + in_global_space: bool, + fm: Option<&fm::FileManager>, +) -> std::result::Result { + // Need to write to a Vec and later convert that to a String so we can + // count how large it is to add padding for the location comment later. + let mut buffer: Vec = Vec::new(); + use std::io::Write; + if !in_global_space { // instructions are always indented within a function - write!(f, " ")?; + write!(buffer, " ").map_err(|_| ())?; } let results = dfg.instruction_results(instruction); @@ -209,10 +226,11 @@ fn display_instruction( if in_global_space { value_list = value_list.replace('v', "g"); } - write!(f, "{} = ", value_list)?; + write!(buffer, "{} = ", value_list).map_err(|_| ())?; } - display_instruction_inner(dfg, &dfg[instruction], results, in_global_space, f)?; + display_instruction_inner(dfg, &dfg[instruction], results, in_global_space, &mut buffer) + .map_err(|_| ())?; if let Some(fm) = fm { let call_stack = dfg.get_instruction_call_stack(instruction); @@ -220,16 +238,25 @@ fn display_instruction( if let Ok(name) = fm.as_file_map().get_name(location.file) { let files = fm.as_file_map(); let start_index = location.span.start() as usize; + + // Add some padding before the comment + let arbitrary_padding_size = 50; + if buffer.len() < arbitrary_padding_size { + buffer.resize(arbitrary_padding_size, ' ' as u8); + } + match codespan_files::Files::line_index(files, location.file, start_index) { // Offset line_index by 1 to convert it into a line number - Ok(line_index) => write!(f, "\t\t// {name}:{}", line_index + 1)?, + Ok(line_index) => write!(buffer, "\t// {name}:{}", line_index + 1), // Showing the file name alone is better than nothing - Err(_) => write!(f, "\t\t// {name}")?, + Err(_) => write!(buffer, "\t// {name}"), } + .map_err(|_| ())?; } } } - writeln!(f) + writeln!(buffer).map_err(|_| ())?; + String::from_utf8(buffer).map_err(|_| ()) } fn display_instruction_inner( @@ -237,8 +264,9 @@ fn display_instruction_inner( instruction: &Instruction, results: &[ValueId], in_global_space: bool, - f: &mut Formatter, -) -> Result { + f: &mut Vec, +) -> std::result::Result<(), std::io::Error> { + use std::io::Write; let show = |id| value(dfg, id); match instruction { @@ -420,8 +448,9 @@ pub(crate) fn try_to_extract_string_from_error_payload( fn display_constrain_error( dfg: &DataFlowGraph, error: &ConstrainError, - f: &mut Formatter, -) -> Result { + f: &mut Vec, +) -> std::result::Result<(), std::io::Error> { + use std::io::Write; match error { ConstrainError::StaticString(assert_message_string) => { write!(f, ", {assert_message_string:?}") From 37b6a9099fc2a17e65721c20249436c13a200b8f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 24 Jun 2025 07:58:54 -0500 Subject: [PATCH 7/8] Clippy --- compiler/noirc_evaluator/src/ssa/ir/printer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 98b57a84624..a76e2a09ca9 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -200,7 +200,7 @@ fn display_instruction( ) -> Result { match display_instruction_buffer(dfg, instruction, in_global_space, fm) { Ok(string) => write!(f, "{string}"), - Err(_) => Err(std::fmt::Error::default()), + Err(_) => Err(std::fmt::Error), } } @@ -242,7 +242,7 @@ fn display_instruction_buffer( // Add some padding before the comment let arbitrary_padding_size = 50; if buffer.len() < arbitrary_padding_size { - buffer.resize(arbitrary_padding_size, ' ' as u8); + buffer.resize(arbitrary_padding_size, b' '); } match codespan_files::Files::line_index(files, location.file, start_index) { From 6b30124eebae9a38975bdf2cd3fd3d1c2b5d054b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 24 Jun 2025 11:17:27 -0500 Subject: [PATCH 8/8] Add columns --- .../noirc_evaluator/src/ssa/ir/printer.rs | 64 ++++++++++++------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index a76e2a09ca9..1fdad2beb5c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -233,30 +233,50 @@ fn display_instruction_buffer( .map_err(|_| ())?; if let Some(fm) = fm { - let call_stack = dfg.get_instruction_call_stack(instruction); - if let Some(location) = call_stack.last() { - if let Ok(name) = fm.as_file_map().get_name(location.file) { - let files = fm.as_file_map(); - let start_index = location.span.start() as usize; - - // Add some padding before the comment - let arbitrary_padding_size = 50; - if buffer.len() < arbitrary_padding_size { - buffer.resize(arbitrary_padding_size, b' '); - } + write_location_information(dfg, instruction, fm, &mut buffer).map_err(|_| ())?; + } + writeln!(buffer).map_err(|_| ())?; + String::from_utf8(buffer).map_err(|_| ()) +} - match codespan_files::Files::line_index(files, location.file, start_index) { - // Offset line_index by 1 to convert it into a line number - Ok(line_index) => write!(buffer, "\t// {name}:{}", line_index + 1), - // Showing the file name alone is better than nothing - Err(_) => write!(buffer, "\t// {name}"), - } - .map_err(|_| ())?; +fn write_location_information( + dfg: &DataFlowGraph, + instruction: InstructionId, + fm: &fm::FileManager, + buffer: &mut Vec, +) -> std::io::Result<()> { + use codespan_files::Files; + use std::io::Write; + let call_stack = dfg.get_instruction_call_stack(instruction); + + if let Some(location) = call_stack.last() { + if let Ok(name) = fm.as_file_map().get_name(location.file) { + let files = fm.as_file_map(); + let start_index = location.span.start() as usize; + + // Add some padding before the comment + let arbitrary_padding_size = 50; + if buffer.len() < arbitrary_padding_size { + buffer.resize(arbitrary_padding_size, b' '); } + + write!(buffer, "\t// {name}")?; + + let Ok(line_index) = files.line_index(location.file, start_index) else { + return Ok(()); + }; + + // Offset index by 1 to get the line number + write!(buffer, ":{}", line_index + 1)?; + + let Ok(column_number) = files.column_number(location.file, line_index, start_index) + else { + return Ok(()); + }; + write!(buffer, ":{}", column_number)?; } } - writeln!(buffer).map_err(|_| ())?; - String::from_utf8(buffer).map_err(|_| ()) + Ok(()) } fn display_instruction_inner( @@ -265,7 +285,7 @@ fn display_instruction_inner( results: &[ValueId], in_global_space: bool, f: &mut Vec, -) -> std::result::Result<(), std::io::Error> { +) -> std::io::Result<()> { use std::io::Write; let show = |id| value(dfg, id); @@ -449,7 +469,7 @@ fn display_constrain_error( dfg: &DataFlowGraph, error: &ConstrainError, f: &mut Vec, -) -> std::result::Result<(), std::io::Error> { +) -> std::io::Result<()> { use std::io::Write; match error { ConstrainError::StaticString(assert_message_string) => {