Skip to content

Commit

Permalink
feat: Use Brillig output by default for debugging and rename some opt…
Browse files Browse the repository at this point in the history
…ions

Also, when selecting ACIR mode, disable instrumentation by default. Allow
instrumentation to be toggled via an CLI option/DAP launch configuration option.
  • Loading branch information
ggiraldez committed Jan 26, 2024
1 parent 046183f commit aab8121
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 59 deletions.
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
43 changes: 9 additions & 34 deletions tooling/nargo_cli/src/cli/dap_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand All @@ -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()))?;

Expand Down Expand Up @@ -186,7 +161,7 @@ fn loop_uninitialized_dap<R: Read, W: Write>(
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)"));
Expand Down
84 changes: 60 additions & 24 deletions tooling/nargo_cli/src/cli/debug_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -42,16 +44,23 @@ 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<bool>,
}

pub(crate) fn run(
backend: &Backend,
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);
Expand All @@ -63,43 +72,70 @@ 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."
);
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);

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<CompiledProgram, CompileError> {
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,
Expand Down

0 comments on commit aab8121

Please sign in to comment.