From 6bfcc4e4e25fd233354bcb7f2cec58dcdd1d3fb8 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 16 May 2023 08:33:40 +0100 Subject: [PATCH 1/5] feat: pass in opcode support function --- crates/nargo_cli/src/cli/check_cmd.rs | 7 ++++++- crates/nargo_cli/src/cli/compile_cmd.rs | 7 ++++++- crates/nargo_cli/src/cli/mod.rs | 6 +++++- crates/nargo_cli/src/cli/test_cmd.rs | 7 ++++++- crates/nargo_cli/src/resolver.rs | 5 +++-- crates/noirc_driver/src/lib.rs | 15 +++++++++------ crates/noirc_driver/src/main.rs | 6 +++++- crates/noirc_frontend/src/node_interner.rs | 10 ++++++++-- crates/wasm/src/compile.rs | 6 +++++- 9 files changed, 53 insertions(+), 16 deletions(-) diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index 9664930466b..c57bb2a1fd9 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -32,7 +32,12 @@ fn check_from_path>( p: P, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = Resolver::resolve_root_manifest(p.as_ref(), backend.np_language())?; + let mut driver = Resolver::resolve_root_manifest( + p.as_ref(), + backend.np_language(), + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(backend.np_language())), + )?; driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 531560b87db..fbdc9330da8 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -63,7 +63,12 @@ fn setup_driver( backend: &B, program_dir: &Path, ) -> Result { - Resolver::resolve_root_manifest(program_dir, backend.np_language()) + Resolver::resolve_root_manifest( + program_dir, + backend.np_language(), + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(backend.np_language())), + ) } pub(crate) fn compile_circuit( diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index d41dc1a815a..d8c9be46e7d 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -128,7 +128,11 @@ mod tests { /// /// This is used for tests. fn file_compiles>(root_file: P) -> bool { - let mut driver = Driver::new(&acvm::Language::R1CS); + let mut driver = Driver::new( + &acvm::Language::R1CS, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)), + ); driver.create_local_crate(&root_file, CrateType::Binary); crate::resolver::add_std_lib(&mut driver); driver.file_compiles() diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index 139d33b6c3d..024eaa20600 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -37,7 +37,12 @@ fn run_tests( test_name: &str, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = Resolver::resolve_root_manifest(program_dir, backend.np_language())?; + let mut driver = Resolver::resolve_root_manifest( + program_dir, + backend.np_language(), + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(backend.np_language())), + )?; driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index ddd607058ed..2ce26a44b26 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use acvm::Language; +use acvm::{acir::circuit::Opcode, Language}; use nargo::manifest::{Dependency, PackageManifest}; use noirc_driver::Driver; use noirc_frontend::graph::{CrateId, CrateName, CrateType}; @@ -71,8 +71,9 @@ impl<'a> Resolver<'a> { 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); + let mut driver = Driver::new(&np_language, is_opcode_supported); 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 a2fbed21885..377e0376837 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -3,6 +3,7 @@ #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] +use acvm::acir::circuit::Opcode; use acvm::Language; use clap::Args; use contract::ContractFunction; @@ -56,9 +57,10 @@ impl Default for CompileOptions { } impl Driver { - pub fn new(np_language: &Language) -> Self { - let mut driver = Driver { context: Context::default(), language: np_language.clone() }; - driver.context.def_interner.set_language(np_language); + pub fn new(language: &Language, is_opcode_supported: Box bool>) -> Self { + let mut driver = Driver { context: Context::default(), language: language.clone() }; + driver.context.def_interner.set_language(language); + driver.context.def_interner.set_opcode_support(is_opcode_supported); driver } @@ -66,9 +68,10 @@ impl Driver { // with the restricted version which only uses one file pub fn compile_file( root_file: PathBuf, - np_language: acvm::Language, + language: &Language, + is_opcode_supported: Box bool>, ) -> Result { - let mut driver = Driver::new(&np_language); + let mut driver = Driver::new(language, is_opcode_supported); driver.create_local_crate(root_file, CrateType::Binary); driver.compile_main(&CompileOptions::default()) } @@ -318,6 +321,6 @@ impl Driver { impl Default for Driver { fn default() -> Self { - Self::new(&Language::R1CS) + Self::new(&Language::R1CS, Box::new(|opcode| matches!(opcode, Opcode::Arithmetic(_)))) } } diff --git a/crates/noirc_driver/src/main.rs b/crates/noirc_driver/src/main.rs index 0d9127d04bd..a0a2e497b78 100644 --- a/crates/noirc_driver/src/main.rs +++ b/crates/noirc_driver/src/main.rs @@ -1,3 +1,4 @@ +use acvm::acir::circuit::Opcode; use noirc_driver::{CompileOptions, Driver}; use noirc_frontend::graph::{CrateType, LOCAL_CRATE}; fn main() { @@ -5,7 +6,10 @@ 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(&acvm::Language::R1CS); + let mut driver = Driver::new( + &acvm::Language::R1CS, + Box::new(|opcode| matches!(opcode, Opcode::Arithmetic(_))), + ); // Add local crate to dep graph driver.create_local_crate(ROOT_DIR_MAIN, CrateType::Binary); diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index d8ea11ae89c..0566a112ac9 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -71,6 +71,7 @@ pub struct NodeInterner { //used for fallback mechanism language: Language, + is_opcode_supported: Box bool>, delayed_type_checks: Vec, @@ -259,6 +260,8 @@ impl Default for NodeInterner { next_type_variable_id: 0, globals: HashMap::new(), language: Language::R1CS, + #[allow(deprecated)] + is_opcode_supported: Box::new(acvm::default_is_opcode_supported(Language::R1CS)), delayed_type_checks: vec![], struct_methods: HashMap::new(), primitive_methods: HashMap::new(), @@ -583,14 +586,17 @@ impl NodeInterner { self.language = language.clone(); } + pub fn set_opcode_support(&mut self, is_opcode_supported: Box bool>) { + self.is_opcode_supported = is_opcode_supported; + } + #[allow(deprecated)] pub fn foreign(&self, opcode: &str) -> bool { - let is_supported = acvm::default_is_opcode_supported(self.language.clone()); let black_box_func = match acvm::acir::BlackBoxFunc::lookup(opcode) { Some(black_box_func) => black_box_func, None => return false, }; - is_supported(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall { + (self.is_opcode_supported)(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall { name: black_box_func, inputs: Vec::new(), outputs: Vec::new(), diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index ecf2b789365..e515372cf3a 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -76,7 +76,11 @@ 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); + let mut driver = noirc_driver::Driver::new( + &language, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(language.clone())), + ); let path = PathBuf::from(&options.entry_point); driver.create_local_crate(path, CrateType::Binary); From 92d2da779cd1e0c76a09375df5bf498d763c9675 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 16 May 2023 11:39:03 +0100 Subject: [PATCH 2/5] chore: remove langauge from `NodeInterner` --- crates/noirc_driver/src/lib.rs | 1 - crates/noirc_frontend/src/hir/mod.rs | 10 +++++++--- crates/noirc_frontend/src/main.rs | 7 ++++++- crates/noirc_frontend/src/node_interner.rs | 6 ------ 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 377e0376837..b8ddd02bc63 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -59,7 +59,6 @@ impl Default for CompileOptions { impl Driver { pub fn new(language: &Language, is_opcode_supported: Box bool>) -> Self { let mut driver = Driver { context: Context::default(), language: language.clone() }; - driver.context.def_interner.set_language(language); driver.context.def_interner.set_opcode_support(is_opcode_supported); driver } diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index 6d469a4229d..2ebabf86867 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -6,7 +6,7 @@ pub mod type_check; use crate::graph::{CrateGraph, CrateId}; use crate::node_interner::NodeInterner; -use acvm::Language; +use acvm::acir::circuit::Opcode; use def_map::CrateDefMap; use fm::FileManager; use std::collections::HashMap; @@ -29,7 +29,11 @@ pub struct Context { pub type StorageSlot = u32; impl Context { - pub fn new(file_manager: FileManager, crate_graph: CrateGraph, language: Language) -> Context { + pub fn new( + file_manager: FileManager, + crate_graph: CrateGraph, + is_opcode_supported: Box bool>, + ) -> Context { let mut ctx = Context { def_interner: NodeInterner::default(), def_maps: HashMap::new(), @@ -37,7 +41,7 @@ impl Context { file_manager, storage_slots: HashMap::new(), }; - ctx.def_interner.set_language(&language); + ctx.def_interner.set_opcode_support(is_opcode_supported); ctx } diff --git a/crates/noirc_frontend/src/main.rs b/crates/noirc_frontend/src/main.rs index b1a5c0f950e..023a1440e52 100644 --- a/crates/noirc_frontend/src/main.rs +++ b/crates/noirc_frontend/src/main.rs @@ -27,7 +27,12 @@ fn main() { let crate_id = crate_graph.add_crate_root(CrateType::Library, root_file_id); // initiate context with file manager and crate graph - let mut context = Context::new(fm, crate_graph, acvm::Language::R1CS); + let mut context = Context::new( + fm, + crate_graph, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)), + ); // Now create the CrateDefMap // This is preamble for analysis diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 0566a112ac9..f778c3aa552 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -70,7 +70,6 @@ pub struct NodeInterner { next_type_variable_id: usize, //used for fallback mechanism - language: Language, is_opcode_supported: Box bool>, delayed_type_checks: Vec, @@ -259,7 +258,6 @@ impl Default for NodeInterner { field_indices: HashMap::new(), next_type_variable_id: 0, globals: HashMap::new(), - language: Language::R1CS, #[allow(deprecated)] is_opcode_supported: Box::new(acvm::default_is_opcode_supported(Language::R1CS)), delayed_type_checks: vec![], @@ -582,10 +580,6 @@ impl NodeInterner { self.function_definition_ids[&function] } - pub fn set_language(&mut self, language: &Language) { - self.language = language.clone(); - } - pub fn set_opcode_support(&mut self, is_opcode_supported: Box bool>) { self.is_opcode_supported = is_opcode_supported; } From c472187dc06914769e16ed101c60b9d88fe841c1 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 16 May 2023 11:55:45 +0100 Subject: [PATCH 3/5] chore: add comment and replace non-standard `is_opcode_supported`s --- crates/noirc_driver/src/lib.rs | 4 +++- crates/noirc_driver/src/main.rs | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index b8ddd02bc63..8449eee9722 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -258,6 +258,7 @@ impl Driver { let program = monomorphize(main_function, &self.context.def_interner); let np_language = self.language.clone(); + // TODO: use proper `is_opcode_supported` implementation. let is_opcode_supported = acvm::default_is_opcode_supported(np_language.clone()); let circuit_abi = if options.experimental_ssa { @@ -320,6 +321,7 @@ impl Driver { impl Default for Driver { fn default() -> Self { - Self::new(&Language::R1CS, Box::new(|opcode| matches!(opcode, Opcode::Arithmetic(_)))) + #[allow(deprecated)] + Self::new(&Language::R1CS, Box::new(acvm::default_is_opcode_supported(Language::R1CS))) } } diff --git a/crates/noirc_driver/src/main.rs b/crates/noirc_driver/src/main.rs index a0a2e497b78..ca7c5faa808 100644 --- a/crates/noirc_driver/src/main.rs +++ b/crates/noirc_driver/src/main.rs @@ -1,4 +1,4 @@ -use acvm::acir::circuit::Opcode; +use acvm::Language; use noirc_driver::{CompileOptions, Driver}; use noirc_frontend::graph::{CrateType, LOCAL_CRATE}; fn main() { @@ -7,8 +7,9 @@ fn main() { const ROOT_DIR_MAIN: &str = "example_real_project/main.nr"; let mut driver = Driver::new( - &acvm::Language::R1CS, - Box::new(|opcode| matches!(opcode, Opcode::Arithmetic(_))), + &Language::R1CS, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(Language::R1CS)), ); // Add local crate to dep graph From 777bb1e093f9c8379b3332c0283395ec543ec8f3 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 16 May 2023 12:19:41 +0100 Subject: [PATCH 4/5] chore: prefer usage of `impl` over `dyn` --- crates/nargo_cli/src/resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index 2ce26a44b26..a64c603678e 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -71,7 +71,7 @@ impl<'a> Resolver<'a> { pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, np_language: Language, - is_opcode_supported: Box bool>, + is_opcode_supported: Box bool>, ) -> Result { let mut driver = Driver::new(&np_language, is_opcode_supported); let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; From ea9f84b21a95976b99b5b9f59b20a5a422266e52 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 16 May 2023 15:24:33 +0100 Subject: [PATCH 5/5] chore: fix build --- crates/nargo_cli/src/resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index a64c603678e..2ce26a44b26 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -71,7 +71,7 @@ impl<'a> Resolver<'a> { pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, np_language: Language, - is_opcode_supported: Box bool>, + is_opcode_supported: Box bool>, ) -> Result { let mut driver = Driver::new(&np_language, is_opcode_supported); let (entry_path, crate_type) = super::lib_or_bin(dir_path)?;