diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index 7b99e5ac02b..820638baba6 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -5,7 +5,6 @@ use std::{ task::{Context, Poll}, }; -use acvm::Language; use async_lsp::{ router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, LanguageClient, LspService, ResponseError, @@ -134,9 +133,7 @@ fn on_code_lens_request( params: CodeLensParams, ) -> impl Future>, ResponseError>> { async move { - // TODO: Requiring `Language` and `is_opcode_supported` to construct a driver makes for some real stinky code - // The driver should not require knowledge of the backend; instead should be implemented as an independent pass (in nargo?) - let mut driver = Driver::new(&Language::R1CS, Box::new(|_op| false)); + let mut driver = Driver::new(); let file_path = ¶ms.text_document.uri.to_file_path().unwrap(); @@ -222,9 +219,7 @@ fn on_did_save_text_document( state: &mut LspState, params: DidSaveTextDocumentParams, ) -> ControlFlow> { - // TODO: Requiring `Language` and `is_opcode_supported` to construct a driver makes for some real stinky code - // The driver should not require knowledge of the backend; instead should be implemented as an independent pass (in nargo?) - let mut driver = Driver::new(&Language::R1CS, Box::new(|_op| false)); + let mut driver = Driver::new(); let file_path = ¶ms.text_document.uri.to_file_path().unwrap(); diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index b464945d375..1702742fc8c 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -1,4 +1,4 @@ -use crate::errors::CliError; +use crate::{errors::CliError, resolver::Resolver}; use acvm::Backend; use clap::Args; use iter_extended::btree_map; @@ -7,8 +7,8 @@ use noirc_driver::CompileOptions; use noirc_errors::reporter::ReportedErrors; use std::path::{Path, PathBuf}; +use super::fs::write_to_file; use super::NargoConfig; -use super::{compile_cmd::setup_driver, fs::write_to_file}; use crate::constants::{PROVER_INPUT_FILE, VERIFIER_INPUT_FILE}; /// Checks the constraint system for errors @@ -23,17 +23,19 @@ pub(crate) fn run( args: CheckCommand, config: NargoConfig, ) -> Result<(), CliError> { - check_from_path(backend, config.program_dir, &args.compile_options)?; + check_from_path(backend, &config.program_dir, &args.compile_options)?; println!("Constraint system successfully built!"); Ok(()) } -fn check_from_path>( - backend: &B, - program_dir: P, +fn check_from_path( + // Backend isn't used but keeping it in the signature allows for better type inference + // TODO: This function doesn't need to exist but requires a little more refactoring + _backend: &B, + program_dir: &Path, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = setup_driver(backend, program_dir.as_ref())?; + let mut driver = Resolver::resolve_root_manifest(program_dir)?; check_crate_and_report_errors(&mut driver, compile_options.deny_warnings)?; // XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files @@ -42,7 +44,7 @@ fn check_from_path>( // For now it is hard-coded to be toml. // // Check for input.toml and verifier.toml - let path_to_root = PathBuf::from(program_dir.as_ref()); + let path_to_root = PathBuf::from(program_dir); let path_to_prover_input = path_to_root.join(format!("{PROVER_INPUT_FILE}.toml")); let path_to_verifier_input = path_to_root.join(format!("{VERIFIER_INPUT_FILE}.toml")); @@ -160,7 +162,7 @@ d2 = ["", "", ""] for path in paths.flatten() { let path = path.path(); assert!( - super::check_from_path(&backend, path.clone(), &config).is_ok(), + super::check_from_path(&backend, &path, &config).is_ok(), "path: {}", path.display() ); @@ -179,7 +181,7 @@ d2 = ["", "", ""] for path in paths.flatten() { let path = path.path(); assert!( - super::check_from_path(&backend, path.clone(), &config).is_err(), + super::check_from_path(&backend, &path, &config).is_err(), "path: {}", path.display() ); @@ -198,7 +200,7 @@ d2 = ["", "", ""] for path in paths.flatten() { let path = path.path(); assert!( - super::check_from_path(&backend, path.clone(), &config).is_ok(), + super::check_from_path(&backend, &path, &config).is_ok(), "path: {}", path.display() ); diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 6d8fd8a2288..9c732396988 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -9,7 +9,6 @@ use clap::Args; use nargo::ops::{preprocess_contract_function, preprocess_program}; -use crate::resolver::DependencyResolutionError; use crate::{constants::TARGET_DIR, errors::CliError, resolver::Resolver}; use super::fs::{ @@ -49,9 +48,13 @@ pub(crate) fn run( // If contracts is set we're compiling every function in a 'contract' rather than just 'main'. if args.contracts { - let mut driver = setup_driver(backend, &config.program_dir)?; + let mut driver = Resolver::resolve_root_manifest(&config.program_dir)?; - let result = driver.compile_contracts(&args.compile_options); + let result = driver.compile_contracts( + backend.np_language(), + &|op| backend.supports_opcode(op), + &args.compile_options, + ); let contracts = report_errors(result, &driver, args.compile_options.deny_warnings)?; // TODO(#1389): I wonder if it is incorrect for nargo-core to know anything about contracts. @@ -101,25 +104,17 @@ pub(crate) fn run( Ok(()) } -pub(super) fn setup_driver( - backend: &B, - program_dir: &Path, -) -> Result { - Resolver::resolve_root_manifest( - program_dir, - backend.np_language(), - // TODO(#1102): Remove need for driver to be aware of backend. - Box::new(|op| B::default().supports_opcode(op)), - ) -} - pub(crate) fn compile_circuit( backend: &B, program_dir: &Path, compile_options: &CompileOptions, ) -> Result> { - let mut driver = setup_driver(backend, program_dir)?; - let result = driver.compile_main(compile_options); + let mut driver = Resolver::resolve_root_manifest(program_dir)?; + let result = driver.compile_main( + backend.np_language(), + &|op| backend.supports_opcode(op), + compile_options, + ); report_errors(result, &driver, compile_options.deny_warnings).map_err(Into::into) } diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index 0066d7c9135..73bd4f6d119 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -178,11 +178,7 @@ mod tests { /// /// This is used for tests. fn file_compiles>(root_file: P) -> bool { - let mut driver = Driver::new( - &acvm::Language::R1CS, - #[allow(deprecated)] - Box::new(acvm::pwg::default_is_opcode_supported(acvm::Language::R1CS)), - ); + let mut driver = Driver::new(); driver.create_local_crate(&root_file, CrateType::Binary); let result = driver.check_crate(false); diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index 4f4364144c2..6db75c4fefd 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -7,10 +7,7 @@ use noirc_driver::{CompileOptions, Driver}; use noirc_frontend::node_interner::FuncId; use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; -use crate::{ - cli::{check_cmd::check_crate_and_report_errors, compile_cmd::setup_driver}, - errors::CliError, -}; +use crate::{cli::check_cmd::check_crate_and_report_errors, errors::CliError, resolver::Resolver}; use super::NargoConfig; @@ -40,7 +37,7 @@ fn run_tests( test_name: &str, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = setup_driver(backend, program_dir)?; + let mut driver = Resolver::resolve_root_manifest(program_dir)?; check_crate_and_report_errors(&mut driver, compile_options.deny_warnings)?; let test_functions = driver.get_all_test_functions_in_crate_matching(test_name); @@ -86,7 +83,7 @@ fn run_test( config: &CompileOptions, ) -> Result<(), CliError> { let program = driver - .compile_no_check(config, main) + .compile_no_check(config, main, backend.np_language(), &|op| backend.supports_opcode(op)) .map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?; // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index 19eafb07065..5bbf2601fc6 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -3,7 +3,6 @@ use std::{ path::{Path, PathBuf}, }; -use acvm::{acir::circuit::Opcode, Language}; use nargo::manifest::{Dependency, PackageManifest}; use noirc_driver::Driver; use noirc_frontend::graph::{CrateId, CrateType}; @@ -70,10 +69,8 @@ impl<'a> Resolver<'a> { /// XXX: Need to handle when a local package changes! pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, - np_language: Language, - is_opcode_supported: Box bool>, ) -> Result { - let mut driver = Driver::new(&np_language, is_opcode_supported); + let mut driver = Driver::new(); let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; let manifest_path = super::find_package_manifest(dir_path)?; diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index c09ffe6b43c..720f0f59ef4 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -4,6 +4,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] use acvm::acir::circuit::Opcode; +use acvm::compiler::CircuitSimplifier; use acvm::Language; use clap::Args; use fm::{FileId, FileManager, FileType}; @@ -25,10 +26,9 @@ mod program; pub use contract::{CompiledContract, ContractFunction, ContractFunctionType}; pub use program::CompiledProgram; +#[derive(Default)] pub struct Driver { context: Context, - language: Language, - is_opcode_supported: Box bool>, } #[derive(Args, Clone, Debug, Serialize, Deserialize)] @@ -73,8 +73,8 @@ pub type Warnings = Vec; pub type ErrorsAndWarnings = Vec; impl Driver { - pub fn new(language: &Language, is_opcode_supported: Box bool>) -> Self { - Driver { context: Context::default(), language: language.clone(), is_opcode_supported } + pub fn new() -> Self { + Self::default() } // TODO(#1599): Move control of the FileManager into nargo @@ -86,12 +86,12 @@ impl Driver { // with the restricted version which only uses one file pub fn compile_file( root_file: PathBuf, - language: &Language, - is_opcode_supported: Box bool>, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> { - let mut driver = Driver::new(language, is_opcode_supported); + let mut driver = Driver::new(); driver.create_local_crate(root_file, CrateType::Binary); - driver.compile_main(&CompileOptions::default()) + driver.compile_main(np_language, is_opcode_supported, &CompileOptions::default()) } /// Adds the File with the local crate root to the file system @@ -206,6 +206,8 @@ impl Driver { /// On error this returns the non-empty list of warnings and errors. pub fn compile_main( &mut self, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, options: &CompileOptions, ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> { let warnings = self.check_crate(options.deny_warnings)?; @@ -221,7 +223,8 @@ impl Driver { } }; - let compiled_program = self.compile_no_check(options, main)?; + let compiled_program = + self.compile_no_check(options, main, np_language, is_opcode_supported)?; if options.print_acir { println!("Compiled ACIR for main:"); @@ -234,6 +237,8 @@ impl Driver { /// Run the frontend to check the crate for errors then compile all contracts if there were none pub fn compile_contracts( &mut self, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, options: &CompileOptions, ) -> Result<(Vec, Warnings), ErrorsAndWarnings> { let warnings = self.check_crate(options.deny_warnings)?; @@ -243,7 +248,13 @@ impl Driver { let mut errors = warnings; for contract in contracts { - match self.compile_contract(contract, options) { + match self.compile_contract( + contract, + // TODO: Remove clone when it implements Copy + np_language.clone(), + is_opcode_supported, + options, + ) { Ok(contract) => compiled_contracts.push(contract), Err(mut more_errors) => errors.append(&mut more_errors), } @@ -281,13 +292,21 @@ impl Driver { fn compile_contract( &self, contract: Contract, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, options: &CompileOptions, ) -> 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 = match self.compile_no_check(options, *function_id) { + let function = match self.compile_no_check( + options, + *function_id, + // TODO: Remove clone when it implements Copy + np_language.clone(), + is_opcode_supported, + ) { Ok(function) => function, Err(err) => { errs.push(err); @@ -342,37 +361,28 @@ impl Driver { &self, options: &CompileOptions, main_function: FuncId, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> Result { let program = monomorphize(main_function, &self.context.def_interner); - let np_language = self.language.clone(); - - let circuit_abi = if options.experimental_ssa { - experimental_create_circuit( - program, - np_language, - &self.is_opcode_supported, - options.show_ssa, - options.show_output, - ) + let (circuit, abi) = if options.experimental_ssa { + experimental_create_circuit(program, options.show_ssa, options.show_output)? } else { - create_circuit( - program, - np_language, - &self.is_opcode_supported, - options.show_ssa, - options.show_output, - ) + create_circuit(program, options.show_ssa, options.show_output)? }; - match circuit_abi { - Ok((circuit, abi)) => Ok(CompiledProgram { circuit, abi }), - 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 - Err(err.into()) - } - } + let abi_len = abi.field_count(); + + let simplifier = CircuitSimplifier::new(abi_len); + let optimized_circuit = + acvm::compiler::compile(circuit, np_language, is_opcode_supported, &simplifier) + .map_err(|_| FileDiagnostic { + file_id: FileId::dummy(), + diagnostic: CustomDiagnostic::from_message("produced an acvm compile error"), + })?; + + Ok(CompiledProgram { circuit: optimized_circuit, abi }) } /// Returns a list of all functions in the current crate marked with #[test] @@ -404,10 +414,3 @@ impl Driver { self.context.def_interner.function_meta(func_id) } } - -impl Default for Driver { - fn default() -> Self { - #[allow(deprecated)] - Self::new(&Language::R1CS, Box::new(acvm::pwg::default_is_opcode_supported(Language::R1CS))) - } -} diff --git a/crates/noirc_driver/src/main.rs b/crates/noirc_driver/src/main.rs index 0dbc0fc1573..b9d5d2583ca 100644 --- a/crates/noirc_driver/src/main.rs +++ b/crates/noirc_driver/src/main.rs @@ -6,11 +6,7 @@ fn main() { const EXTERNAL_DIR2: &str = "dep_a/lib.nr"; const ROOT_DIR_MAIN: &str = "example_real_project/main.nr"; - let mut driver = Driver::new( - &Language::R1CS, - #[allow(deprecated)] - Box::new(acvm::pwg::default_is_opcode_supported(Language::R1CS)), - ); + let mut driver = Driver::new(); // Add local crate to dep graph driver.create_local_crate(ROOT_DIR_MAIN, CrateType::Binary); @@ -23,5 +19,12 @@ fn main() { driver.add_dep(LOCAL_CRATE, crate_id1, "coo4"); driver.add_dep(LOCAL_CRATE, crate_id2, "coo3"); - driver.compile_main(&CompileOptions::default()).ok(); + driver + .compile_main( + Language::R1CS, + #[allow(deprecated)] + &acvm::pwg::default_is_opcode_supported(Language::R1CS), + &CompileOptions::default(), + ) + .ok(); } diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 13026ac8e8c..05204943c49 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -15,8 +15,6 @@ pub mod brillig; use acvm::{ acir::circuit::{opcodes::Opcode as AcirOpcode, Circuit, PublicInputs}, acir::native_types::{Expression, Witness}, - compiler::CircuitSimplifier, - Language, }; use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::vecmap; @@ -67,8 +65,6 @@ pub struct Evaluator { // If we had a composer object, we would not need it pub fn create_circuit( program: Program, - np_language: Language, - is_opcode_supported: &impl Fn(&AcirOpcode) -> bool, enable_logging: bool, show_output: bool, ) -> Result<(Circuit, Abi), RuntimeError> { @@ -85,24 +81,17 @@ pub fn create_circuit( opcodes, .. } = evaluator; - let simplifier = CircuitSimplifier::new(current_witness_index); - let optimized_circuit = acvm::compiler::compile( - Circuit { - current_witness_index, - opcodes, - public_parameters: PublicInputs(public_parameters), - return_values: PublicInputs(return_values.iter().copied().collect()), - }, - np_language, - is_opcode_supported, - &simplifier, - ) - .map_err(|_| RuntimeErrorKind::Spanless(String::from("produced an acvm compile error")))?; + let circuit = Circuit { + current_witness_index, + opcodes, + public_parameters: PublicInputs(public_parameters), + return_values: PublicInputs(return_values.iter().copied().collect()), + }; let (parameters, return_type) = program.main_function_signature; let abi = Abi { parameters, param_witnesses, return_type, return_witnesses: return_values }; - Ok((optimized_circuit, abi)) + Ok((circuit, abi)) } impl Evaluator { diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index 4f11e147809..34189252f37 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -8,10 +8,7 @@ #![allow(dead_code)] use crate::errors::RuntimeError; -use acvm::{ - acir::circuit::{Circuit, Opcode as AcirOpcode, PublicInputs}, - Language, -}; +use acvm::acir::circuit::{Circuit, PublicInputs}; use noirc_abi::Abi; use noirc_frontend::monomorphization::ast::Program; @@ -59,10 +56,9 @@ pub(crate) fn optimize_into_acir( /// Compiles the Program into ACIR and applies optimizations to the arithmetic gates /// This is analogous to `ssa:create_circuit` and this method is called when one wants /// to use the new ssa module to process Noir code. +// TODO: This no longer needs to return a result, but it is kept to match the signature of `create_circuit` pub fn experimental_create_circuit( program: Program, - np_language: Language, - is_opcode_supported: &impl Fn(&AcirOpcode) -> bool, enable_logging: bool, show_output: bool, ) -> Result<(Circuit, Abi), RuntimeError> { @@ -77,26 +73,9 @@ pub fn experimental_create_circuit( PublicInputs(public_abi.param_witnesses.values().flatten().copied().collect()); let return_values = PublicInputs(return_witnesses.into_iter().collect()); - // This region of code will optimize the ACIR bytecode for a particular backend - // it will be removed in the near future and we will subsequently only return the - // unoptimized backend-agnostic bytecode here - let optimized_circuit = { - use crate::errors::RuntimeErrorKind; - use acvm::compiler::CircuitSimplifier; + let circuit = Circuit { current_witness_index, opcodes, public_parameters, return_values }; - let abi_len = abi.field_count(); - - let simplifier = CircuitSimplifier::new(abi_len); - acvm::compiler::compile( - Circuit { current_witness_index, opcodes, public_parameters, return_values }, - np_language, - is_opcode_supported, - &simplifier, - ) - .map_err(|_| RuntimeErrorKind::Spanless(String::from("produced an acvm compile error")))? - }; - - Ok((optimized_circuit, abi)) + Ok((circuit, abi)) } impl Ssa { diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index ba1c2bacd14..18c9722e8ff 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -75,11 +75,7 @@ pub fn compile(args: JsValue) -> JsValue { // For now we default to plonk width = 3, though we can add it as a parameter let language = acvm::Language::PLONKCSat { width: 3 }; - let mut driver = noirc_driver::Driver::new( - &language, - #[allow(deprecated)] - Box::new(acvm::pwg::default_is_opcode_supported(language.clone())), - ); + let mut driver = noirc_driver::Driver::new(); let path = PathBuf::from(&options.entry_point); driver.create_local_crate(path, CrateType::Binary); @@ -92,15 +88,29 @@ pub fn compile(args: JsValue) -> JsValue { if options.contracts { let compiled_contracts = driver - .compile_contracts(&options.compile_options) + .compile_contracts( + // TODO: Remove clone when it implements Copy + language.clone(), + #[allow(deprecated)] + &acvm::pwg::default_is_opcode_supported(language), + &options.compile_options, + ) .expect("Contract compilation failed") .0; ::from_serde(&compiled_contracts).unwrap() } else { 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"); + let compiled_program = driver + .compile_no_check( + &options.compile_options, + main, + // TODO: Remove clone when it implements Copy + language.clone(), + #[allow(deprecated)] + &acvm::pwg::default_is_opcode_supported(language), + ) + .expect("Compilation failed"); ::from_serde(&compiled_program).unwrap() }