Skip to content

Commit

Permalink
feat: DAP Preflight and debugger compilation options (#4185)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Part of #3015 

## Summary\*

This PR adds a preflight mode to DAP in order to make it easier to
identify and report back problems to the user when compiling the project
for debugging. This preflight mode is invoked from the VS.Code extension
before starting the debugging session, and with the same arguments as
those that will be used for the session. If the compiler finds any error
either loading or compiling the project, the error is reported to stderr
which allows the extension to parse the output and present the
diagnostic messages to the user.

This also changes the default compilation mode to output Brillig code
and adds new commands line options to Nargo's `debug` command and launch
options to the DAP mode to control the mode and whether to inject
instrumentation code to track variables or not.

The `debug` options are:
- `--acir-mode`, force output of ACIR, which disables instrumentation by
default
- `--skip-instrumentation={true,false}` to control injection of
instrumentation code to track variables values during the debugging
session.

Similarly, for DAP two launch options can be provided: `generateAcir`
and `skipInstrumentation`.

## Additional Context

The default is to output in Brillig mode with instrumentation for
tracking variables, as this makes it easier to follow along with
stepping through the code. If ACIR mode is selected, instrumentation is
disabled by default. Instrumentation can be forcefully enabled or
disabled by the provided CLI option.

## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [X] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Martin Verzilli <martin.verzilli@gmail.com>
  • Loading branch information
ggiraldez and mverzilli committed Feb 14, 2024
1 parent 9256a7d commit e0ad0b2
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 72 deletions.
1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
"indexmap",
"injective",
"Inlines",
"instrumenter",
"interner",
"intrinsics",
"jmp",
Expand Down
7 changes: 6 additions & 1 deletion tooling/debugger/src/dap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,12 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> {
};
let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) {
Ok(index) => line_to_opcodes[index].1,
Err(index) => line_to_opcodes[index].1,
Err(index) => {
if index >= line_to_opcodes.len() {
return None;
}
line_to_opcodes[index].1
}
};
Some(found_index)
}
Expand Down
19 changes: 19 additions & 0 deletions tooling/debugger/src/errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use thiserror::Error;

#[derive(Debug, Error)]
pub enum DapError {
#[error("{0}")]
PreFlightGenericError(String),

#[error(transparent)]
LoadError(#[from] LoadError),

#[error(transparent)]
ServerError(#[from] dap::errors::ServerError),
}

#[derive(Debug, Error)]
pub enum LoadError {
#[error("{0}")]
Generic(String),
}
1 change: 1 addition & 0 deletions tooling/debugger/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod context;
mod dap;
pub mod errors;
mod foreign_calls;
mod repl;
mod source_code_printer;
Expand Down
148 changes: 103 additions & 45 deletions tooling/nargo_cli/src/cli/dap_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,52 @@ 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_instrumenter;
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};
use std::path::Path;

use dap::errors::ServerError;
use dap::requests::Command;
use dap::responses::ResponseBody;
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;

use super::NargoConfig;

use noir_debugger::errors::{DapError, LoadError};

#[derive(Debug, Clone, Args)]
pub(crate) struct DapCommand {
/// Override the expression width requested by the backend.
#[arg(long, value_parser = parse_expression_width)]
expression_width: Option<ExpressionWidth>,

#[clap(long)]
preflight_check: bool,

#[clap(long)]
preflight_project_folder: Option<String>,

#[clap(long)]
preflight_package: Option<String>,

#[clap(long)]
preflight_prover_name: Option<String>,

#[clap(long)]
preflight_generate_acir: bool,

#[clap(long)]
preflight_skip_instrumentation: bool,
}

fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
Expand All @@ -50,8 +64,6 @@ fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error
}
}

struct LoadError(&'static str);

fn find_workspace(project_folder: &str, package: Option<&str>) -> Option<Workspace> {
let Ok(toml_path) = get_package_manifest(Path::new(project_folder)) else {
eprintln!("ERROR: Failed to get package manifest");
Expand All @@ -72,63 +84,59 @@ fn find_workspace(project_folder: &str, package: Option<&str>) -> Option<Workspa
}
}

fn workspace_not_found_error_msg(project_folder: &str, package: Option<&str>) -> String {
match package {
Some(pkg) => format!(
r#"Noir Debugger could not load program from {}, package {}"#,
project_folder, pkg
),
None => format!(r#"Noir Debugger could not load program from {}"#, project_folder),
}
}

fn load_and_compile_project(
project_folder: &str,
package: Option<&str>,
prover_name: &str,
expression_width: ExpressionWidth,
acir_mode: bool,
skip_instrumentation: bool,
) -> Result<(CompiledProgram, WitnessMap), LoadError> {
let workspace =
find_workspace(project_folder, package).ok_or(LoadError("Cannot open workspace"))?;

let workspace = find_workspace(project_folder, package)
.ok_or(LoadError::Generic(workspace_not_found_error_msg(project_folder, package)))?;
let package = workspace
.into_iter()
.find(|p| p.is_binary())
.ok_or(LoadError("No matching binary packages found in workspace"))?;

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);
.ok_or(LoadError::Generic("No matching binary packages found in workspace".into()))?;

let compile_options =
CompileOptions { instrument_debug: true, force_brillig: true, ..CompileOptions::default() };

let debug_state = instrument_package_files(&mut parsed_files, &workspace_file_manager, package);

let compilation_result = compile_program_with_debug_instrumenter(
&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("Failed to compile project"))?;
.map_err(|_| LoadError::Generic("Failed to compile project".into()))?;

let compiled_program = nargo::ops::transform_program(compiled_program, expression_width);

let (inputs_map, _) =
read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &compiled_program.abi)
.map_err(|_| LoadError("Failed to read program inputs"))?;
.map_err(|_| {
LoadError::Generic(format!("Failed to read program inputs from {}", prover_name))
})?;
let initial_witness = compiled_program
.abi
.encode(&inputs_map, None)
.map_err(|_| LoadError("Failed to encode inputs"))?;
.map_err(|_| LoadError::Generic("Failed to encode inputs".into()))?;

Ok((compiled_program, initial_witness))
}

fn loop_uninitialized_dap<R: Read, W: Write>(
mut server: Server<R, W>,
expression_width: ExpressionWidth,
) -> Result<(), ServerError> {
) -> Result<(), DapError> {
loop {
let req = match server.poll_request()? {
Some(req) => req,
Expand Down Expand Up @@ -163,6 +171,13 @@ fn loop_uninitialized_dap<R: Read, W: Write>(
.and_then(|v| v.as_str())
.unwrap_or(PROVER_INPUT_FILE);

let generate_acir =
additional_data.get("generateAcir").and_then(|v| v.as_bool()).unwrap_or(false);
let skip_instrumentation = additional_data
.get("skipInstrumentation")
.and_then(|v| v.as_bool())
.unwrap_or(generate_acir);

eprintln!("Project folder: {}", project_folder);
eprintln!("Package: {}", package.unwrap_or("(default)"));
eprintln!("Prover name: {}", prover_name);
Expand All @@ -172,6 +187,8 @@ fn loop_uninitialized_dap<R: Read, W: Write>(
package,
prover_name,
expression_width,
generate_acir,
skip_instrumentation,
) {
Ok((compiled_program, initial_witness)) => {
server.respond(req.ack()?)?;
Expand All @@ -186,8 +203,8 @@ fn loop_uninitialized_dap<R: Read, W: Write>(
)?;
break;
}
Err(LoadError(message)) => {
server.respond(req.error(message))?;
Err(LoadError::Generic(message)) => {
server.respond(req.error(message.as_str()))?;
}
}
}
Expand All @@ -206,17 +223,58 @@ fn loop_uninitialized_dap<R: Read, W: Write>(
Ok(())
}

fn run_preflight_check(
expression_width: ExpressionWidth,
args: DapCommand,
) -> Result<(), DapError> {
let project_folder = if let Some(project_folder) = args.preflight_project_folder {
project_folder
} else {
return Err(DapError::PreFlightGenericError("Noir Debugger could not initialize because the IDE (for example, VS Code) did not specify a project folder to debug.".into()));
};

let package = args.preflight_package.as_deref();
let prover_name = args.preflight_prover_name.as_deref().unwrap_or(PROVER_INPUT_FILE);

let _ = load_and_compile_project(
project_folder.as_str(),
package,
prover_name,
expression_width,
args.preflight_generate_acir,
args.preflight_skip_instrumentation,
)?;

Ok(())
}

pub(crate) fn run(
backend: &Backend,
args: DapCommand,
_config: NargoConfig,
) -> Result<(), CliError> {
let expression_width =
args.expression_width.unwrap_or_else(|| backend.get_backend_info_or_default());

// When the --preflight-check flag is present, we run Noir's DAP server in "pre-flight mode", which test runs
// the DAP initialization code without actually starting the DAP server.
//
// This lets the client IDE present any initialization issues (compiler version mismatches, missing prover files, etc)
// in its own interface.
//
// This was necessary due to the VS Code project being reluctant to let extension authors capture
// stderr output generated by a DAP server wrapped in DebugAdapterExecutable.
//
// Exposing this preflight mode lets us gracefully handle errors that happen *before*
// the DAP loop is established, which otherwise are considered "out of band" by the maintainers of the DAP spec.
// More details here: https://github.com/microsoft/vscode/issues/108138
if args.preflight_check {
return run_preflight_check(expression_width, args).map_err(CliError::DapError);
}

let output = BufWriter::new(std::io::stdout());
let input = BufReader::new(std::io::stdin());
let server = Server::new(input, output);

let expression_width =
args.expression_width.unwrap_or_else(|| backend.get_backend_info_or_default());

loop_uninitialized_dap(server, expression_width).map_err(CliError::DapError)
}
Loading

0 comments on commit e0ad0b2

Please sign in to comment.