From aab81210380a89b85785fcc8eb07efbe8a3224e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Thu, 25 Jan 2024 17:05:58 -0500 Subject: [PATCH] feat: Use Brillig output by default for debugging and rename some options Also, when selecting ACIR mode, disable instrumentation by default. Allow instrumentation to be toggled via an CLI option/DAP launch configuration option. --- compiler/noirc_driver/src/lib.rs | 2 +- tooling/nargo_cli/src/cli/dap_cmd.rs | 43 +++---------- tooling/nargo_cli/src/cli/debug_cmd.rs | 84 ++++++++++++++++++-------- 3 files changed, 70 insertions(+), 59 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index c9c03089b84..548442191d8 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -83,7 +83,7 @@ pub struct CompileOptions { pub show_monomorphized: bool, /// Insert debug symbols to inspect variables - #[arg(long)] + #[arg(long, hide = true)] pub instrument_debug: bool, /// Force Brillig output (for step debugging) diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index addc3562c07..760bd55f018 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -2,14 +2,10 @@ use acvm::acir::native_types::WitnessMap; use backend_interface::Backend; use clap::Args; use nargo::constants::PROVER_INPUT_FILE; -use nargo::ops::compile_program_with_debug_state; use nargo::workspace::Workspace; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::Format; -use noirc_driver::{ - file_manager_with_stdlib, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING, -}; +use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::graph::CrateName; use std::io::{BufReader, BufWriter, Read, Write}; @@ -21,8 +17,7 @@ use dap::server::Server; use dap::types::Capabilities; use serde_json::Value; -use super::compile_cmd::report_errors; -use super::debug_cmd::instrument_package_files; +use super::debug_cmd::compile_bin_package_for_debugging; use super::fs::inputs::read_inputs_from_file; use crate::errors::CliError; @@ -86,7 +81,7 @@ fn load_and_compile_project( project_folder: &str, package: Option<&str>, prover_name: &str, - generate_acir: bool, + acir_mode: bool, skip_instrumentation: bool, ) -> Result<(CompiledProgram, WitnessMap), LoadError> { let workspace = find_workspace(project_folder, package) @@ -99,32 +94,12 @@ fn load_and_compile_project( .find(|p| p.is_binary()) .ok_or(LoadError::Generic("No matching binary packages found in workspace".into()))?; - let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); - insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let mut parsed_files = parse_all(&workspace_file_manager); - - let compile_options = CompileOptions { - instrument_debug: !skip_instrumentation, - force_brillig: !generate_acir, - ..CompileOptions::default() - }; - - let debug_state = instrument_package_files(&mut parsed_files, &workspace_file_manager, package); - - let compilation_result = compile_program_with_debug_state( - &workspace_file_manager, - &parsed_files, + let compiled_program = compile_bin_package_for_debugging( + &workspace, package, - &compile_options, - None, - debug_state, - ); - - let compiled_program = report_errors( - compilation_result, - &workspace_file_manager, - compile_options.deny_warnings, - compile_options.silence_warnings, + acir_mode, + skip_instrumentation, + CompileOptions::default(), ) .map_err(|_| LoadError::Generic("Failed to compile project".into()))?; @@ -186,7 +161,7 @@ fn loop_uninitialized_dap( let skip_instrumentation = additional_data .get("skipInstrumentation") .and_then(|v| v.as_bool()) - .unwrap_or(false); + .unwrap_or(generate_acir); eprintln!("Project folder: {}", project_folder); eprintln!("Package: {}", package.unwrap_or("(default)")); diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index fa5fa7c70d4..1439b115baf 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -7,8 +7,10 @@ use clap::Args; use fm::FileManager; use nargo::artifacts::debug::DebugArtifact; use nargo::constants::PROVER_INPUT_FILE; -use nargo::ops::compile_program_with_debug_state; +use nargo::errors::CompileError; +use nargo::ops::{compile_program, compile_program_with_debug_state}; use nargo::package::Package; +use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::{Format, InputValue}; @@ -42,6 +44,14 @@ pub(crate) struct DebugCommand { #[clap(flatten)] compile_options: CompileOptions, + + /// Force ACIR output (disabling instrumentation) + #[clap(long)] + acir_mode: bool, + + /// Disable vars debug instrumentation (enabled by default) + #[clap(long)] + skip_instrumentation: Option, } pub(crate) fn run( @@ -49,9 +59,8 @@ pub(crate) fn run( args: DebugCommand, config: NargoConfig, ) -> Result<(), CliError> { - // TODO: set clap default for flattened flag - let mut args = args.clone(); - args.compile_options.instrument_debug = true; + let acir_mode = args.acir_mode; + let skip_instrumentation = args.skip_instrumentation.unwrap_or(acir_mode); let toml_path = get_package_manifest(&config.program_dir)?; let selection = args.package.map_or(PackageSelection::DefaultOrAll, PackageSelection::Selected); @@ -63,10 +72,6 @@ pub(crate) fn run( let target_dir = &workspace.target_directory_path(); let expression_width = backend.get_backend_info()?; - let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); - insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let mut parsed_files = parse_all(&workspace_file_manager); - let Some(package) = workspace.into_iter().find(|p| p.is_binary()) else { println!( "No matching binary packages found in workspace. Only binary packages can be debugged." @@ -74,22 +79,12 @@ pub(crate) fn run( return Ok(()); }; - let debug_state = instrument_package_files(&mut parsed_files, &workspace_file_manager, package); - - let compilation_result = compile_program_with_debug_state( - &workspace_file_manager, - &parsed_files, + let compiled_program = compile_bin_package_for_debugging( + &workspace, package, - &args.compile_options, - None, - debug_state, - ); - - let compiled_program = report_errors( - compilation_result, - &workspace_file_manager, - args.compile_options.deny_warnings, - args.compile_options.silence_warnings, + acir_mode, + skip_instrumentation, + args.compile_options.clone(), )?; let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); @@ -97,9 +92,50 @@ pub(crate) fn run( run_async(package, compiled_program, &args.prover_name, &args.witness_name, target_dir) } +pub(crate) fn compile_bin_package_for_debugging( + workspace: &Workspace, + package: &Package, + acir_mode: bool, + skip_instrumentation: bool, + compile_options: CompileOptions, +) -> Result { + let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); + insert_all_files_for_workspace_into_file_manager(workspace, &mut workspace_file_manager); + let mut parsed_files = parse_all(&workspace_file_manager); + + let compile_options = CompileOptions { + instrument_debug: !skip_instrumentation, + force_brillig: !acir_mode, + ..compile_options + }; + + let compilation_result = if !skip_instrumentation { + let debug_state = + instrument_package_files(&mut parsed_files, &workspace_file_manager, package); + + compile_program_with_debug_state( + &workspace_file_manager, + &parsed_files, + package, + &compile_options, + None, + debug_state, + ) + } else { + compile_program(&workspace_file_manager, &parsed_files, package, &compile_options, None) + }; + + report_errors( + compilation_result, + &workspace_file_manager, + compile_options.deny_warnings, + compile_options.silence_warnings, + ) +} + /// Add debugging instrumentation to all parsed files belonging to the package /// being compiled -pub(crate) fn instrument_package_files( +fn instrument_package_files( parsed_files: &mut ParsedFiles, file_manager: &FileManager, package: &Package,