diff --git a/crates/nargo/src/cli/build_cmd.rs b/crates/nargo/src/cli/build_cmd.rs index 5af013732a6..337581787d7 100644 --- a/crates/nargo/src/cli/build_cmd.rs +++ b/crates/nargo/src/cli/build_cmd.rs @@ -5,18 +5,19 @@ use std::path::{Path, PathBuf}; use super::{add_std_lib, write_to_file, PROVER_INPUT_FILE, VERIFIER_INPUT_FILE}; -pub(crate) fn run(_args: ArgMatches) -> Result<(), CliError> { +pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let package_dir = std::env::current_dir().unwrap(); - build_from_path(package_dir)?; + let allow_warnings = args.is_present("allow-warnings"); + build_from_path(package_dir, allow_warnings)?; println!("Constraint system successfully built!"); Ok(()) } // This is exposed so that we can run the examples and verify that they pass -pub fn build_from_path>(p: P) -> Result<(), CliError> { +pub fn build_from_path>(p: P, allow_warnings: bool) -> Result<(), CliError> { let backend = crate::backends::ConcreteBackend; let mut driver = Resolver::resolve_root_config(p.as_ref(), backend.np_language())?; add_std_lib(&mut driver); - driver.build(); + driver.build(allow_warnings); // XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files if let Some(x) = driver.compute_abi() { // XXX: The root config should return an enum to determine if we are looking for .json or .toml @@ -56,7 +57,11 @@ mod tests { let paths = std::fs::read_dir(pass_dir).unwrap(); for path in paths.flatten() { let path = path.path(); - assert!(super::build_from_path(path.clone()).is_ok(), "path: {}", path.display()); + assert!( + super::build_from_path(path.clone(), false).is_ok(), + "path: {}", + path.display() + ); } } @@ -69,7 +74,23 @@ mod tests { let paths = std::fs::read_dir(fail_dir).unwrap(); for path in paths.flatten() { let path = path.path(); - assert!(super::build_from_path(path.clone()).is_err(), "path: {}", path.display()); + assert!( + super::build_from_path(path.clone(), false).is_err(), + "path: {}", + path.display() + ); + } + } + + #[test] + fn pass_with_warnings() { + let mut pass_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); + pass_dir.push(&format!("{TEST_DATA_DIR}/pass_dev_mode")); + + let paths = std::fs::read_dir(pass_dir).unwrap(); + for path in paths.flatten() { + let path = path.path(); + assert!(super::build_from_path(path.clone(), true).is_ok(), "path: {}", path.display()); } } } diff --git a/crates/nargo/src/cli/compile_cmd.rs b/crates/nargo/src/cli/compile_cmd.rs index 448cc1ceb60..2497ef9af2a 100644 --- a/crates/nargo/src/cli/compile_cmd.rs +++ b/crates/nargo/src/cli/compile_cmd.rs @@ -15,13 +15,19 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("compile").unwrap(); let circuit_name = args.value_of("circuit_name").unwrap(); let witness = args.is_present("witness"); + let allow_warnings = args.is_present("allow-warnings"); let curr_dir = std::env::current_dir().unwrap(); let mut circuit_path = PathBuf::new(); circuit_path.push(BUILD_DIR); - let result = - generate_circuit_and_witness_to_disk(circuit_name, curr_dir, circuit_path, witness); + let result = generate_circuit_and_witness_to_disk( + circuit_name, + curr_dir, + circuit_path, + witness, + allow_warnings, + ); match result { Ok(_) => Ok(()), Err(e) => Err(e), @@ -33,8 +39,9 @@ pub fn generate_circuit_and_witness_to_disk>( program_dir: P, circuit_dir: P, generate_witness: bool, + allow_warnings: bool, ) -> Result { - let compiled_program = compile_circuit(program_dir.as_ref(), false)?; + let compiled_program = compile_circuit(program_dir.as_ref(), false, allow_warnings)?; let serialized = compiled_program.circuit.to_bytes(); let mut circuit_path = create_named_dir(circuit_dir.as_ref(), "build"); @@ -60,11 +67,13 @@ pub fn generate_circuit_and_witness_to_disk>( pub fn compile_circuit>( program_dir: P, show_ssa: bool, + allow_warnings: bool, ) -> Result { let backend = crate::backends::ConcreteBackend; let mut driver = Resolver::resolve_root_config(program_dir.as_ref(), backend.np_language())?; add_std_lib(&mut driver); - let compiled_program = driver.into_compiled_program(backend.np_language(), show_ssa); + let compiled_program = + driver.into_compiled_program(backend.np_language(), show_ssa, allow_warnings); Ok(compiled_program) } diff --git a/crates/nargo/src/cli/contract_cmd.rs b/crates/nargo/src/cli/contract_cmd.rs index 3dff8ecfdf2..9dcba9b4e11 100644 --- a/crates/nargo/src/cli/contract_cmd.rs +++ b/crates/nargo/src/cli/contract_cmd.rs @@ -11,7 +11,8 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { None => std::env::current_dir().unwrap(), }; - let compiled_program = compile_circuit(&package_dir, false)?; + let allow_warnings = args.is_present("allow-warnings"); + let compiled_program = compile_circuit(&package_dir, false, allow_warnings)?; let backend = crate::backends::ConcreteBackend; let smart_contract_string = backend.eth_contract_from_cs(compiled_program.circuit); diff --git a/crates/nargo/src/cli/gates_cmd.rs b/crates/nargo/src/cli/gates_cmd.rs index 1999f1b8399..41422182bd9 100644 --- a/crates/nargo/src/cli/gates_cmd.rs +++ b/crates/nargo/src/cli/gates_cmd.rs @@ -9,19 +9,21 @@ use crate::errors::CliError; pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("gates").unwrap(); let show_ssa = args.is_present("show-ssa"); - count_gates(show_ssa) + let allow_warnings = args.is_present("allow-warnings"); + count_gates(show_ssa, allow_warnings) } -pub fn count_gates(show_ssa: bool) -> Result<(), CliError> { +pub fn count_gates(show_ssa: bool, allow_warnings: bool) -> Result<(), CliError> { let curr_dir = std::env::current_dir().unwrap(); - count_gates_with_path(curr_dir, show_ssa) + count_gates_with_path(curr_dir, show_ssa, allow_warnings) } pub fn count_gates_with_path>( program_dir: P, show_ssa: bool, + allow_warnings: bool, ) -> Result<(), CliError> { - let compiled_program = compile_circuit(program_dir.as_ref(), show_ssa)?; + let compiled_program = compile_circuit(program_dir.as_ref(), show_ssa, allow_warnings)?; let gates = compiled_program.circuit.gates; // Store counts of each gate type into hashmap. diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 2eef5672b47..b7ecf2bbd88 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -80,6 +80,11 @@ pub fn start_cli() { .help("Emit debug information for the intermediate SSA IR"), ), ) + .arg( + Arg::with_name("allow-warnings") + .long("allow-warnings") + .help("Issue a warning for each unused variable instead of an error"), + ) .get_matches(); let result = match matches.subcommand_name() { @@ -126,20 +131,25 @@ fn write_to_file(bytes: &[u8], path: &Path) -> String { // helper function which tests noir programs by trying to generate a proof and verify it pub fn prove_and_verify(proof_name: &str, prg_dir: &Path, show_ssa: bool) -> bool { let tmp_dir = TempDir::new("p_and_v_tests").unwrap(); - let proof_path = - match prove_cmd::prove_with_path(proof_name, prg_dir, &tmp_dir.into_path(), show_ssa) { - Ok(p) => p, - Err(CliError::Generic(msg)) => { - println!("Error: {}", msg); - return false; - } - Err(CliError::DestinationAlreadyExists(str)) => { - println!("Error, destination {} already exists: ", str); - return false; - } - }; - - verify_cmd::verify_with_path(prg_dir, &proof_path, show_ssa).unwrap() + let proof_path = match prove_cmd::prove_with_path( + proof_name, + prg_dir, + &tmp_dir.into_path(), + show_ssa, + false, + ) { + Ok(p) => p, + Err(CliError::Generic(msg)) => { + println!("Error: {}", msg); + return false; + } + Err(CliError::DestinationAlreadyExists(str)) => { + println!("Error, destination {} already exists: ", str); + return false; + } + }; + + verify_cmd::verify_with_path(prg_dir, &proof_path, show_ssa, false).unwrap() } fn add_std_lib(driver: &mut Driver) { diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index f6d68f841f8..352cccd6543 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -19,18 +19,19 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("prove").unwrap(); let proof_name = args.value_of("proof_name").unwrap(); let show_ssa = args.is_present("show-ssa"); - prove(proof_name, show_ssa) + let allow_warnings = args.is_present("allow-warnings"); + prove(proof_name, show_ssa, allow_warnings) } /// In Barretenberg, the proof system adds a zero witness in the first index, /// So when we add witness values, their index start from 1. const WITNESS_OFFSET: u32 = 1; -fn prove(proof_name: &str, show_ssa: bool) -> Result<(), CliError> { +fn prove(proof_name: &str, show_ssa: bool, allow_warnings: bool) -> Result<(), CliError> { let curr_dir = std::env::current_dir().unwrap(); let mut proof_path = PathBuf::new(); proof_path.push(PROOFS_DIR); - let result = prove_with_path(proof_name, curr_dir, proof_path, show_ssa); + let result = prove_with_path(proof_name, curr_dir, proof_path, show_ssa, allow_warnings); match result { Ok(_) => Ok(()), Err(e) => Err(e), @@ -111,8 +112,13 @@ fn process_abi_with_input( pub fn compile_circuit_and_witness>( program_dir: P, show_ssa: bool, + allow_unused_variables: bool, ) -> Result<(noirc_driver::CompiledProgram, BTreeMap), CliError> { - let compiled_program = super::compile_cmd::compile_circuit(program_dir.as_ref(), show_ssa)?; + let compiled_program = super::compile_cmd::compile_circuit( + program_dir.as_ref(), + show_ssa, + allow_unused_variables, + )?; let solved_witness = solve_witness(program_dir, &compiled_program)?; Ok((compiled_program, solved_witness)) } @@ -200,8 +206,10 @@ pub fn prove_with_path>( program_dir: P, proof_dir: P, show_ssa: bool, + allow_warnings: bool, ) -> Result { - let (compiled_program, solved_witness) = compile_circuit_and_witness(program_dir, show_ssa)?; + let (compiled_program, solved_witness) = + compile_circuit_and_witness(program_dir, show_ssa, allow_warnings)?; let backend = crate::backends::ConcreteBackend; let proof = backend.prove_with_meta(compiled_program.circuit, solved_witness); diff --git a/crates/nargo/src/cli/verify_cmd.rs b/crates/nargo/src/cli/verify_cmd.rs index 2a5e730749e..aa861537cb8 100644 --- a/crates/nargo/src/cli/verify_cmd.rs +++ b/crates/nargo/src/cli/verify_cmd.rs @@ -14,18 +14,19 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { proof_path.push(Path::new(proof_name)); proof_path.set_extension(PROOF_EXT); - let result = verify(proof_name)?; + let allow_warnings = args.is_present("allow-warnings"); + let result = verify(proof_name, allow_warnings)?; println!("Proof verified : {}\n", result); Ok(()) } -fn verify(proof_name: &str) -> Result { +fn verify(proof_name: &str, allow_warnings: bool) -> Result { let curr_dir = std::env::current_dir().unwrap(); let mut proof_path = PathBuf::new(); //or cur_dir? proof_path.push(PROOFS_DIR); proof_path.push(Path::new(proof_name)); proof_path.set_extension(PROOF_EXT); - verify_with_path(&curr_dir, &proof_path, false) + verify_with_path(&curr_dir, &proof_path, false, allow_warnings) } fn process_abi_with_verifier_input( @@ -65,8 +66,9 @@ pub fn verify_with_path>( program_dir: P, proof_path: P, show_ssa: bool, + allow_warnings: bool, ) -> Result { - let compiled_program = compile_circuit(program_dir.as_ref(), show_ssa)?; + let compiled_program = compile_circuit(program_dir.as_ref(), show_ssa, allow_warnings)?; let public_abi = compiled_program.abi.clone().unwrap().public_abi(); let num_pub_params = public_abi.num_parameters(); diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index e58eae82bbc..b33257acad3 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -34,7 +34,7 @@ impl Driver { pub fn compile_file(root_file: PathBuf, np_language: acvm::Language) -> CompiledProgram { let mut driver = Driver::new(&np_language); driver.create_local_crate(root_file, CrateType::Binary); - driver.into_compiled_program(np_language, false) + driver.into_compiled_program(np_language, false, false) } /// Compiles a file and returns true if compilation was successful @@ -123,17 +123,21 @@ impl Driver { // NOTE: Maybe build could be skipped given that now it is a pass through method. /// Statically analyses the local crate - pub fn build(&mut self) { - self.analyse_crate() + pub fn build(&mut self, allow_warnings: bool) { + self.analyse_crate(allow_warnings) } - fn analyse_crate(&mut self) { + fn analyse_crate(&mut self, allow_warnings: bool) { let mut errs = vec![]; CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs); let mut error_count = 0; for errors in &errs { - error_count += errors.errors.len(); - Reporter::with_diagnostics(errors.file_id, &self.context.file_manager, &errors.errors); + error_count += Reporter::with_diagnostics( + errors.file_id, + &self.context.file_manager, + &errors.errors, + allow_warnings, + ); } Reporter::finish(error_count); @@ -154,8 +158,9 @@ impl Driver { mut self, np_language: acvm::Language, show_ssa: bool, + allow_warnings: bool, ) -> CompiledProgram { - self.build(); + self.build(allow_warnings); // Check the crate type // We don't panic here to allow users to `evaluate` libraries @@ -184,12 +189,13 @@ impl Driver { Err(err) => { // The FileId here will be the file id of the file with the main file // Errors will be shown at the callsite without a stacktrace - Reporter::with_diagnostics( + let error_count = Reporter::with_diagnostics( err.location.file, &self.context.file_manager, &[err.to_diagnostic()], + allow_warnings, ); - Reporter::finish(1); + Reporter::finish(error_count); unreachable!("reporter will exit before this point") } }; diff --git a/crates/noirc_errors/src/reporter.rs b/crates/noirc_errors/src/reporter.rs index feb9fb5c54e..88cdf5388b2 100644 --- a/crates/noirc_errors/src/reporter.rs +++ b/crates/noirc_errors/src/reporter.rs @@ -12,11 +12,23 @@ pub struct CustomDiagnostic { message: String, secondaries: Vec, notes: Vec, + kind: DiagnosticKind, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum DiagnosticKind { + Error, + Warning, } impl CustomDiagnostic { pub fn from_message(msg: &str) -> CustomDiagnostic { - Self { message: msg.to_owned(), secondaries: Vec::new(), notes: Vec::new() } + Self { + message: msg.to_owned(), + secondaries: Vec::new(), + notes: Vec::new(), + kind: DiagnosticKind::Error, + } } pub fn simple_error( @@ -28,6 +40,20 @@ impl CustomDiagnostic { message: primary_message, secondaries: vec![CustomLabel::new(secondary_message, secondary_span)], notes: Vec::new(), + kind: DiagnosticKind::Error, + } + } + + pub fn simple_warning( + primary_message: String, + secondary_message: String, + secondary_span: Span, + ) -> CustomDiagnostic { + CustomDiagnostic { + message: primary_message, + secondaries: vec![CustomLabel::new(secondary_message, secondary_span)], + notes: Vec::new(), + kind: DiagnosticKind::Warning, } } @@ -71,39 +97,51 @@ impl CustomLabel { pub struct Reporter; impl Reporter { + /// Writes the given diagnostics to stderr and returns the count + /// of diagnostics that were errors. pub fn with_diagnostics( file_id: FileId, files: &fm::FileManager, diagnostics: &[CustomDiagnostic], - ) { + allow_warnings: bool, + ) -> usize { + let mut error_count = 0; + // Convert each Custom Diagnostic into a diagnostic - let diagnostics: Vec<_> = diagnostics - .iter() - .map(|cd| { - let secondary_labels = cd - .secondaries - .iter() - .map(|sl| { - let start_span = sl.span.start() as usize; - let end_span = sl.span.end() as usize + 1; - Label::secondary(file_id.as_usize(), start_span..end_span) - .with_message(&sl.message) - }) - .collect(); - - Diagnostic::error() - .with_message(&cd.message) - .with_labels(secondary_labels) - .with_notes(cd.notes.clone()) - }) - .collect(); + let diagnostics = diagnostics.iter().map(|cd| { + let diagnostic = match (cd.kind, allow_warnings) { + (DiagnosticKind::Warning, true) => Diagnostic::warning(), + _ => { + error_count += 1; + Diagnostic::error() + } + }; + + let secondary_labels = cd + .secondaries + .iter() + .map(|sl| { + let start_span = sl.span.start() as usize; + let end_span = sl.span.end() as usize + 1; + Label::secondary(file_id.as_usize(), start_span..end_span) + .with_message(&sl.message) + }) + .collect(); + + diagnostic + .with_message(&cd.message) + .with_labels(secondary_labels) + .with_notes(cd.notes.clone()) + }); let writer = StandardStream::stderr(ColorChoice::Always); let config = codespan_reporting::term::Config::default(); - for diagnostic in diagnostics.iter() { - term::emit(&mut writer.lock(), &config, files.as_simple_files(), diagnostic).unwrap(); + for diagnostic in diagnostics { + term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap(); } + + error_count } pub fn finish(error_count: usize) { diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 8c05c2e2a8c..255620aef04 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -58,14 +58,11 @@ impl ResolverError { ResolverError::UnusedVariable { ident } => { let name = &ident.0.contents; - let mut diag = Diagnostic::simple_error( + Diagnostic::simple_warning( format!("unused variable {}", name), "unused variable ".to_string(), - ident.0.span(), - ); - let message = format!("A new variable usually means a constraint has been added and is being unused. \n For this reason, it is almost always a bug to declare a variable and not use it. \n help: if this is intentional, prefix it with an underscore: `_{}`", name); - diag.add_note(message); - diag + ident.span(), + ) } ResolverError::VariableNotDeclared { name, span } => Diagnostic::simple_error( format!("cannot find `{}` in this scope ", name),