chore(driver): Remove backend dependence from driver construction (#1846

)

* chore(driver): Remove backend dependence from driver construction

* derive default

* deduplicate the optimizing pass

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<Output = Result<Option<Vec<CodeLens>>, 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 = &params.text_document.uri.to_file_path().unwrap();
 
@@ -222,9 +219,7 @@ fn on_did_save_text_document(
     state: &mut LspState,
     params: DidSaveTextDocumentParams,
 ) -> ControlFlow<Result<(), async_lsp::Error>> {
-    // 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 = &params.text_document.uri.to_file_path().unwrap();
 

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<B: Backend>(
     args: CheckCommand,
     config: NargoConfig,
 ) -> Result<(), CliError<B>> {
-    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<B: Backend, P: AsRef<Path>>(
-    backend: &B,
-    program_dir: P,
+fn check_from_path<B: Backend>(
+    // 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<B>> {
-    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<B: Backend, P: AsRef<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()
             );

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<B: Backend>(
 
     // 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<B: Backend>(
     Ok(())
 }
 
-pub(super) fn setup_driver<B: Backend>(
-    backend: &B,
-    program_dir: &Path,
-) -> Result<Driver, DependencyResolutionError> {
-    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<B: Backend>(
     backend: &B,
     program_dir: &Path,
     compile_options: &CompileOptions,
 ) -> Result<CompiledProgram, CliError<B>> {
-    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)
 }
 

crates/nargo_cli/src/cli/mod.rs

@@ -178,11 +178,7 @@ mod tests {
     ///
     /// This is used for tests.
     fn file_compiles<P: AsRef<Path>>(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);

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<B: Backend>(
     test_name: &str,
     compile_options: &CompileOptions,
 ) -> Result<(), CliError<B>> {
-    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<B: Backend>(
     config: &CompileOptions,
 ) -> Result<(), CliError<B>> {
     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,

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<dyn Fn(&Opcode) -> bool>,
     ) -> Result<Driver, DependencyResolutionError> {
-        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)?;