diff --git a/Cargo.lock b/Cargo.lock index 1fa151a51f0..c8790acc1b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1934,6 +1934,7 @@ dependencies = [ "noir_lsp", "noirc_abi", "noirc_driver", + "noirc_errors", "noirc_frontend", "predicates 2.1.5", "rustc_version", @@ -1997,7 +1998,6 @@ dependencies = [ "acvm", "clap", "fm", - "iter-extended", "noirc_abi", "noirc_errors", "noirc_evaluator", @@ -2013,7 +2013,6 @@ dependencies = [ "codespan", "codespan-reporting", "fm", - "serde", ] [[package]] diff --git a/crates/nargo_cli/Cargo.toml b/crates/nargo_cli/Cargo.toml index 840232a64c6..fed53e77f22 100644 --- a/crates/nargo_cli/Cargo.toml +++ b/crates/nargo_cli/Cargo.toml @@ -28,6 +28,7 @@ noir_lsp.workspace = true noirc_driver.workspace = true noirc_frontend.workspace = true noirc_abi.workspace = true +noirc_errors.workspace = true acvm.workspace = true toml.workspace = true serde.workspace = true diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index 32e292571b6..26006723797 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -4,6 +4,7 @@ use clap::Args; use iter_extended::btree_map; use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME}; use noirc_driver::CompileOptions; +use noirc_errors::reporter; use std::path::{Path, PathBuf}; use super::NargoConfig; @@ -34,7 +35,15 @@ fn check_from_path>( ) -> Result<(), CliError> { let mut driver = setup_driver(backend, program_dir.as_ref())?; - driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; + let result = driver.check_crate(); + if let Err(errs) = result { + let file_manager = driver.file_manager(); + let error_count = reporter::report_all(file_manager, &errs, compile_options.deny_warnings); + if error_count != 0 { + reporter::finish_report(error_count); + return Err(CliError::CompilationError); + } + } // XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files if let Some((parameters, return_type)) = driver.compute_function_signature() { diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index e4fadfa41c5..ebb83bd2aa1 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -4,6 +4,7 @@ use acvm::{acir::native_types::WitnessMap, Backend}; use clap::Args; use nargo::ops::execute_circuit; use noirc_driver::{CompileOptions, Driver}; +use noirc_errors::reporter; use noirc_frontend::node_interner::FuncId; use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; @@ -39,7 +40,15 @@ fn run_tests( ) -> Result<(), CliError> { let mut driver = setup_driver(backend, program_dir)?; - driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; + let result = driver.check_crate(); + if let Err(errs) = result { + let file_manager = driver.file_manager(); + let error_count = reporter::report_all(file_manager, &errs, compile_options.deny_warnings); + if error_count != 0 { + reporter::finish_report(error_count); + return Err(CliError::CompilationError); + } + } let test_functions = driver.get_all_test_functions_in_crate_matching(test_name); println!("Running {} test functions...", test_functions.len()); diff --git a/crates/noirc_driver/Cargo.toml b/crates/noirc_driver/Cargo.toml index d0775359067..3f75dc7da8e 100644 --- a/crates/noirc_driver/Cargo.toml +++ b/crates/noirc_driver/Cargo.toml @@ -8,7 +8,6 @@ edition.workspace = true [dependencies] clap.workspace = true -iter-extended.workspace = true noirc_errors.workspace = true noirc_frontend.workspace = true noirc_evaluator.workspace = true diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 2224c7faa9b..a468c929cf8 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -6,10 +6,9 @@ use acvm::acir::circuit::Opcode; use acvm::Language; use clap::Args; -use fm::FileType; -use iter_extended::try_vecmap; +use fm::{FileId, FileManager, FileType}; use noirc_abi::FunctionSignature; -use noirc_errors::{reporter, ReportedError}; +use noirc_errors::{reporter, CustomDiagnostic, FileDiagnostic}; use noirc_evaluator::{create_circuit, ssa_refactor::experimental_create_circuit}; use noirc_frontend::graph::{CrateId, CrateName, CrateType, LOCAL_CRATE}; use noirc_frontend::hir::def_map::{Contract, CrateDefMap}; @@ -71,13 +70,18 @@ impl Driver { Driver { context: Context::default(), language: language.clone(), is_opcode_supported } } + // TODO(#1599): Move control of the FileManager into nargo + pub fn file_manager(&self) -> &FileManager { + &self.context.file_manager + } + // This is here for backwards compatibility // with the restricted version which only uses one file pub fn compile_file( root_file: PathBuf, language: &Language, is_opcode_supported: Box bool>, - ) -> Result { + ) -> Result> { let mut driver = Driver::new(language, is_opcode_supported); driver.create_local_crate(root_file, CrateType::Binary); driver.compile_main(&CompileOptions::default()) @@ -167,12 +171,14 @@ impl Driver { /// Run the lexing, parsing, name resolution, and type checking passes, /// returning Err(FrontendError) and printing any errors that were found. - pub fn check_crate(&mut self, options: &CompileOptions) -> Result<(), ReportedError> { + pub fn check_crate(&mut self) -> Result<(), Vec> { let mut errs = vec![]; CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs); - let error_count = - reporter::report_all(&self.context.file_manager, &errs, options.deny_warnings); - reporter::finish_report(error_count) + if errs.is_empty() { + Ok(()) + } else { + Err(errs) + } } pub fn compute_function_signature(&self) -> Option { @@ -189,13 +195,16 @@ impl Driver { pub fn compile_main( &mut self, options: &CompileOptions, - ) -> Result { - self.check_crate(options)?; + ) -> Result> { + self.check_crate()?; let main = match self.main_function() { - Ok(m) => m, - Err(e) => { - println!("cannot compile a program with no main function"); - return Err(e); + Some(m) => m, + None => { + let err = FileDiagnostic { + file_id: FileId::default(), + diagnostic: CustomDiagnostic::from_message("cannot compile crate into a program as the local crate is not a binary. For libraries, please use the check command") + }; + return Err(err.into()); } }; let compiled_program = self.compile_no_check(options, main)?; @@ -210,23 +219,33 @@ impl Driver { pub fn compile_contracts( &mut self, options: &CompileOptions, - ) -> Result, ReportedError> { - self.check_crate(options)?; + ) -> Result, Vec> { + self.check_crate()?; let contracts = self.get_all_contracts(); - let compiled_contracts = - try_vecmap(contracts, |contract| self.compile_contract(contract, options))?; - if options.print_acir { - for compiled_contract in &compiled_contracts { - for contract_function in &compiled_contract.functions { - println!( - "Compiled ACIR for {}::{}:", - compiled_contract.name, contract_function.name - ); - println!("{}", contract_function.bytecode); + let mut compiled_contracts = vec![]; + let mut errs = vec![]; + for contract in contracts { + match self.compile_contract(contract, options) { + Ok(contract) => compiled_contracts.push(contract), + Err(err) => errs.push(err), + } + } + if errs.is_empty() { + if options.print_acir { + for compiled_contract in &compiled_contracts { + for contract_function in &compiled_contract.functions { + println!( + "Compiled ACIR for {}::{}:", + compiled_contract.name, contract_function.name + ); + println!("{}", contract_function.bytecode); + } } } + Ok(compiled_contracts) + } else { + Err(errs.concat()) } - Ok(compiled_contracts) } /// Compile all of the functions associated with a Noir contract. @@ -234,10 +253,18 @@ impl Driver { &self, contract: Contract, options: &CompileOptions, - ) -> Result { - let functions = try_vecmap(&contract.functions, |function_id| { + ) -> Result> { + let mut functions = Vec::new(); + let mut errs = Vec::new(); + for function_id in &contract.functions { let name = self.function_name(*function_id).to_owned(); - let function = self.compile_no_check(options, *function_id)?; + let function = match self.compile_no_check(options, *function_id) { + Ok(function) => function, + Err(err) => { + errs.push(err); + continue; + } + }; let func_meta = self.context.def_interner.function_meta(function_id); let func_type = func_meta .contract_function_type @@ -245,33 +272,36 @@ impl Driver { let function_type = ContractFunctionType::new(func_type, func_meta.is_unconstrained); - Ok(ContractFunction { + functions.push(ContractFunction { name, function_type, abi: function.abi, bytecode: function.circuit, - }) - })?; + }); + } - Ok(CompiledContract { name: contract.name, functions }) + if errs.is_empty() { + Ok(CompiledContract { name: contract.name, functions }) + } else { + Err(errs) + } } /// Returns the FuncId of the 'main' function. /// - Expects check_crate to be called beforehand /// - Panics if no main function is found - pub fn main_function(&self) -> Result { + pub fn main_function(&self) -> Option { // Find the local crate, one should always be present let local_crate = self.context.def_map(LOCAL_CRATE).unwrap(); // Check the crate type // We don't panic here to allow users to `evaluate` libraries which will do nothing if self.context.crate_graph[LOCAL_CRATE].crate_type != CrateType::Binary { - println!("cannot compile crate into a program as the local crate is not a binary. For libraries, please use the check command"); - return Err(ReportedError); - }; - - // All Binaries should have a main function - local_crate.main_function().ok_or(ReportedError) + None + } else { + // All Binaries should have a main function + local_crate.main_function() + } } /// Compile the current crate. Assumes self.check_crate is called beforehand! @@ -280,7 +310,7 @@ impl Driver { &self, options: &CompileOptions, main_function: FuncId, - ) -> Result { + ) -> Result { let program = monomorphize(main_function, &self.context.def_interner); let np_language = self.language.clone(); @@ -308,11 +338,7 @@ 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 call site without a stacktrace - let file = err.location.map(|loc| loc.file); - let files = &self.context.file_manager; - let error = reporter::report(files, &err.into(), file, options.deny_warnings); - reporter::finish_report(error as u32)?; - Err(ReportedError) + Err(err.into()) } } } diff --git a/crates/noirc_errors/Cargo.toml b/crates/noirc_errors/Cargo.toml index 3079f2c5830..ed4c18fd0a2 100644 --- a/crates/noirc_errors/Cargo.toml +++ b/crates/noirc_errors/Cargo.toml @@ -11,4 +11,3 @@ codespan-reporting.workspace = true codespan.workspace = true fm.workspace = true chumsky.workspace = true -serde.workspace = true diff --git a/crates/noirc_errors/src/lib.rs b/crates/noirc_errors/src/lib.rs index ab154639d13..9112f56aece 100644 --- a/crates/noirc_errors/src/lib.rs +++ b/crates/noirc_errors/src/lib.rs @@ -7,14 +7,15 @@ mod position; pub mod reporter; pub use position::{Location, Position, Span, Spanned}; pub use reporter::{CustomDiagnostic, DiagnosticKind}; -use serde::{Deserialize, Serialize}; -/// Returned when the Reporter finishes after reporting errors -#[derive(Copy, Clone, Serialize, Deserialize)] -pub struct ReportedError; - -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct FileDiagnostic { pub file_id: fm::FileId, pub diagnostic: CustomDiagnostic, } + +impl From for Vec { + fn from(value: FileDiagnostic) -> Self { + vec![value] + } +} diff --git a/crates/noirc_errors/src/reporter.rs b/crates/noirc_errors/src/reporter.rs index 2c8be90a280..d09f27d03f1 100644 --- a/crates/noirc_errors/src/reporter.rs +++ b/crates/noirc_errors/src/reporter.rs @@ -1,4 +1,4 @@ -use crate::{FileDiagnostic, ReportedError, Span}; +use crate::{FileDiagnostic, Span}; use codespan_reporting::diagnostic::{Diagnostic, Label}; use codespan_reporting::term; use codespan_reporting::term::termcolor::{ @@ -6,7 +6,7 @@ use codespan_reporting::term::termcolor::{ }; use std::io::Write; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct CustomDiagnostic { message: String, secondaries: Vec, @@ -89,7 +89,7 @@ impl std::fmt::Display for CustomDiagnostic { } } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] struct CustomLabel { message: String, span: Span, @@ -156,7 +156,7 @@ fn convert_diagnostic( diagnostic.with_message(&cd.message).with_labels(secondary_labels).with_notes(cd.notes.clone()) } -pub fn finish_report(error_count: u32) -> Result<(), ReportedError> { +pub fn finish_report(error_count: u32) { if error_count != 0 { let writer = StandardStream::stderr(ColorChoice::Always); let mut writer = writer.lock(); @@ -164,9 +164,5 @@ pub fn finish_report(error_count: u32) -> Result<(), ReportedError> { writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap(); writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap(); writer.reset().ok(); - - Err(ReportedError) - } else { - Ok(()) } } diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 935ef093f4f..638f1a93fba 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,4 +1,4 @@ -use noirc_errors::{CustomDiagnostic as Diagnostic, Location}; +use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location}; use thiserror::Error; #[derive(Debug)] @@ -38,6 +38,13 @@ impl From for RuntimeError { } } +impl From for FileDiagnostic { + fn from(err: RuntimeError) -> Self { + let file_id = err.location.map(|loc| loc.file).unwrap(); + FileDiagnostic { file_id, diagnostic: err.into() } + } +} + #[derive(Error, Debug)] pub enum RuntimeErrorKind { // Array errors diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index bb9a1148b91..e0b03180264 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -68,8 +68,7 @@ pub fn compile(args: JsValue) -> JsValue { debug!("Initializing compiler with default values."); WASMCompileOptions::default() } else { - JsValueSerdeExt::into_serde(&args) - .unwrap_or_else(|_| panic!("Could not deserialize compile arguments")) + JsValueSerdeExt::into_serde(&args).expect("Could not deserialize compile arguments") }; debug!("Compiler configuration {:?}", &options); @@ -92,20 +91,18 @@ pub fn compile(args: JsValue) -> JsValue { // We are always adding std lib implicitly. It comes bundled with binary. add_noir_lib(&mut driver, "std"); - driver.check_crate(&options.compile_options).unwrap_or_else(|_| panic!("Crate check failed")); + driver.check_crate().expect("Crate check failed"); if options.contracts { let compiled_contracts = driver .compile_contracts(&options.compile_options) - .unwrap_or_else(|_| panic!("Contract compilation failed")); + .expect("Contract compilation failed"); ::from_serde(&compiled_contracts).unwrap() } else { - let main = - driver.main_function().unwrap_or_else(|_| panic!("Could not find main function!")); - let compiled_program = driver - .compile_no_check(&options.compile_options, main) - .unwrap_or_else(|_| panic!("Compilation failed")); + let main = driver.main_function().expect("Could not find main function!"); + let compiled_program = + driver.compile_no_check(&options.compile_options, main).expect("Compilation failed"); ::from_serde(&compiled_program).unwrap() }