From 35e1584aac1e63c9e88174bfeda50817ff66ec17 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Tue, 17 Oct 2023 22:57:11 +0100 Subject: [PATCH 01/10] feat: noir-wasm takes dependency graph --- compiler/wasm/src/compile.rs | 87 +++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 0f7baff4819..8b19120232d 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -9,8 +9,12 @@ use noirc_driver::{ add_dep, compile_contract, compile_main, prepare_crate, prepare_dependency, CompileOptions, CompiledContract, CompiledProgram, }; -use noirc_frontend::{graph::CrateGraph, hir::Context}; -use std::path::Path; +use noirc_frontend::{ + graph::{CrateGraph, CrateId}, + hir::Context, +}; +use serde::Deserialize; +use std::{collections::HashMap, path::Path}; use wasm_bindgen::prelude::*; use crate::errors::JsCompileError; @@ -24,14 +28,26 @@ extern "C" { pub type StringArray; } +#[derive(Deserialize)] +struct DependencyGraph { + root_dependencies: Vec, + library_dependencies: HashMap>, +} + #[wasm_bindgen] pub fn compile( entry_point: String, contracts: Option, - dependencies: Option, + dependency_graph: JsValue, ) -> Result { console_error_panic_hook::set_once(); + let dependency_graph: DependencyGraph = + ::into_serde(&dependency_graph).unwrap_or(DependencyGraph { + root_dependencies: vec![], + library_dependencies: HashMap::new(), + }); + let root = Path::new("/"); let fm = FileManager::new(root, Box::new(get_non_stdlib_asset)); let graph = CrateGraph::default(); @@ -40,12 +56,7 @@ pub fn compile( let path = Path::new(&entry_point); let crate_id = prepare_crate(&mut context, path); - let dependencies: Vec = dependencies - .map(|array| array.iter().map(|element| element.as_string().unwrap()).collect()) - .unwrap_or_default(); - for dependency in dependencies { - add_noir_lib(&mut context, dependency.as_str()); - } + process_dependency_graph(&mut context, dependency_graph); let compile_options = CompileOptions::default(); @@ -85,32 +96,44 @@ pub fn compile( } } -fn add_noir_lib(context: &mut Context, library_name: &str) { - let path_to_lib = Path::new(&library_name).join("lib.nr"); - let library_crate_id = prepare_dependency(context, &path_to_lib); +fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) { + let mut crate_names: HashMap = HashMap::new(); - add_dep(context, *context.root_crate_id(), library_crate_id, library_name.parse().unwrap()); - - // TODO: Remove this code that attaches every crate to every other crate as a dependency - let root_crate_id = context.root_crate_id(); - let stdlib_crate_id = context.stdlib_crate_id(); - let other_crate_ids: Vec<_> = context - .crate_graph - .iter_keys() - .filter(|crate_id| { - // We don't want to attach this crate to itself or stdlib, nor re-attach it to the root crate - crate_id != &library_crate_id - && crate_id != root_crate_id - && crate_id != stdlib_crate_id - }) - .collect(); + // register libraries + for lib in &dependency_graph.root_dependencies { + let crate_id = add_noir_lib(context, &lib); + crate_names.insert(lib.to_string(), crate_id); + } - for crate_id in other_crate_ids { - context - .crate_graph - .add_dep(crate_id, library_name.parse().unwrap(), library_crate_id) - .unwrap_or_else(|_| panic!("ICE: Cyclic error triggered by {library_name} library")); + // make root crate as depending on it + for lib in &dependency_graph.root_dependencies { + let crate_id = crate_names.get(lib.as_str()).unwrap().clone(); + let root_crate_id = context.root_crate_id().clone(); + add_dep(context, root_crate_id, crate_id, lib.parse().unwrap()); } + + for (lib_name, dependencies) in dependency_graph.library_dependencies { + let crate_id = crate_names.get(lib_name.as_str()).unwrap().clone(); + + for dependency_name in dependencies { + let lib_crate_id = match crate_names.entry(dependency_name.clone()) { + std::collections::hash_map::Entry::Occupied(entry) => entry.into_mut(), + std::collections::hash_map::Entry::Vacant(entry) => { + let crate_id = add_noir_lib(context, &dependency_name); + entry.insert(crate_id) + } + }; + + add_dep(context, crate_id, lib_crate_id.clone(), dependency_name.parse().unwrap()); + } + } +} + +fn add_noir_lib(context: &mut Context, library_name: &str) -> CrateId { + let path_to_lib = Path::new(&library_name).join("lib.nr"); + let library_crate_id = prepare_dependency(context, &path_to_lib); + + library_crate_id } fn preprocess_program(program: CompiledProgram) -> PreprocessedProgram { From 16d8ffac78c7314ce87c6ba27b05dbe1b57d2bd8 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Fri, 20 Oct 2023 08:23:29 +0100 Subject: [PATCH 02/10] refactor: CompileError is an Error subclass in JS --- compiler/wasm/src/compile.rs | 14 +++-- compiler/wasm/src/errors.rs | 99 +++++++++++++++++++++++++++--------- 2 files changed, 86 insertions(+), 27 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 8b19120232d..5adaca9d356 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -17,7 +17,7 @@ use serde::Deserialize; use std::{collections::HashMap, path::Path}; use wasm_bindgen::prelude::*; -use crate::errors::JsCompileError; +use crate::errors::{CompileError, JsCompileError}; const BACKEND_IDENTIFIER: &str = "acvm-backend-barretenberg"; @@ -68,7 +68,11 @@ pub fn compile( if contracts.unwrap_or_default() { let compiled_contract = compile_contract(&mut context, crate_id, &compile_options) .map_err(|errs| { - JsCompileError::new("Failed to compile contract", errs, &context.file_manager) + CompileError::with_file_diagnostics( + "Failed to compile contract", + errs, + &context.file_manager, + ) })? .0; @@ -82,7 +86,11 @@ pub fn compile( } else { let compiled_program = compile_main(&mut context, crate_id, &compile_options, None, true) .map_err(|errs| { - JsCompileError::new("Failed to compile program", errs, &context.file_manager) + CompileError::with_file_diagnostics( + "Failed to compile program", + errs, + &context.file_manager, + ) })? .0; diff --git a/compiler/wasm/src/errors.rs b/compiler/wasm/src/errors.rs index 7090bf6f19f..57295916dc5 100644 --- a/compiler/wasm/src/errors.rs +++ b/compiler/wasm/src/errors.rs @@ -1,5 +1,6 @@ use gloo_utils::format::JsValueSerdeExt; -use serde::{Deserialize, Serialize}; +use js_sys::JsString; +use serde::Serialize; use wasm_bindgen::prelude::*; use fm::FileManager; @@ -17,66 +18,116 @@ export type Diagnostic = { }>; } -interface CompileError { +export interface CompileError extends Error { + message: string; diagnostics: ReadonlyArray; } "#; -#[derive(Serialize, Deserialize)] -struct JsDiagnosticLabel { +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(extends = js_sys::Error, js_name = "CompileError", typescript_type = "CompileError")] + #[derive(Clone, Debug, PartialEq, Eq)] + pub type JsCompileError; + + #[wasm_bindgen(constructor, js_class = "Error")] + fn constructor(message: JsString) -> JsCompileError; +} + +impl JsCompileError { + const DIAGNOSTICS_PROP: &'static str = "diagnostics"; + const NAME_PROP: &'static str = "name"; + const ERROR_NAME: &'static str = "CompileError"; + + pub fn new(message: String, diagnostics: Vec) -> Self { + let err = JsCompileError::constructor(JsString::from(message)); + + js_sys::Reflect::set( + &err, + &JsString::from(JsCompileError::NAME_PROP), + &JsString::from(JsCompileError::ERROR_NAME), + ) + .unwrap(); + + js_sys::Reflect::set( + &err, + &JsString::from(JsCompileError::DIAGNOSTICS_PROP), + &::from_serde(&diagnostics).unwrap(), + ) + .unwrap(); + + err + } +} + +impl From for JsCompileError { + fn from(value: String) -> Self { + JsCompileError::new(value, vec![]) + } +} + +impl From for JsCompileError { + fn from(value: CompileError) -> Self { + JsCompileError::new(value.message, value.diagnostics) + } +} + +#[derive(Serialize)] +struct DiagnosticLabel { message: String, start: u32, end: u32, } -#[derive(Serialize, Deserialize)] -struct JsDiagnostic { +#[derive(Serialize)] +pub struct Diagnostic { message: String, - file_path: String, - secondaries: Vec, + file: String, + secondaries: Vec, } -impl JsDiagnostic { - fn new(file_diagnostic: &FileDiagnostic, file_path: String) -> JsDiagnostic { +impl Diagnostic { + fn new(file_diagnostic: &FileDiagnostic, file: String) -> Diagnostic { let diagnostic = &file_diagnostic.diagnostic; let message = diagnostic.message.clone(); let secondaries = diagnostic .secondaries .iter() - .map(|label| JsDiagnosticLabel { + .map(|label| DiagnosticLabel { message: label.message.clone(), start: label.span.start(), end: label.span.end(), }) .collect(); - JsDiagnostic { message, file_path, secondaries } + Diagnostic { message, file, secondaries } } } -#[wasm_bindgen(getter_with_clone, js_name = "CompileError")] -pub struct JsCompileError { - pub message: js_sys::JsString, - pub diagnostics: JsValue, +#[derive(Serialize)] +pub struct CompileError { + pub message: String, + pub diagnostics: Vec, } -impl JsCompileError { - pub fn new( +impl CompileError { + pub fn new(message: &str) -> CompileError { + CompileError { message: message.to_string(), diagnostics: vec![] } + } + + pub fn with_file_diagnostics( message: &str, file_diagnostics: Vec, file_manager: &FileManager, - ) -> JsCompileError { + ) -> CompileError { let diagnostics: Vec<_> = file_diagnostics .iter() .map(|err| { - JsDiagnostic::new(err, file_manager.path(err.file_id).to_str().unwrap().to_string()) + Diagnostic::new(err, file_manager.path(err.file_id).to_str().unwrap().to_string()) }) .collect(); - JsCompileError { - message: js_sys::JsString::from(message.to_string()), - diagnostics: ::from_serde(&diagnostics).unwrap(), - } + CompileError { message: message.to_string(), diagnostics } } } From 5390dddb7e3645f7a3a69f1ab5823602d18aa90a Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Fri, 20 Oct 2023 10:43:54 +0100 Subject: [PATCH 03/10] refactor: address code review --- Cargo.lock | 1 + Cargo.toml | 2 +- compiler/noirc_frontend/Cargo.toml | 1 + compiler/noirc_frontend/src/graph/mod.rs | 3 +- compiler/wasm/src/compile.rs | 75 +++++++++++++----------- 5 files changed, 45 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8fb282dda98..ed2a53e0266 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2674,6 +2674,7 @@ dependencies = [ "noirc_printable_type", "regex", "rustc-hash", + "serde", "serde_json", "small-ord-set", "smol_str", diff --git a/Cargo.toml b/Cargo.toml index e80de516e72..0921ce0cc02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,7 +77,7 @@ dirs = "4" lsp-types = "0.94" serde = { version = "1.0.136", features = ["derive"] } serde_json = "1.0" -smol_str = "0.1.17" +smol_str = { version = "0.1.17", features = ["serde"] } thiserror = "1.0.21" toml = "0.7.2" tower = "0.4" diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 1cb6e0c5c51..d30394159c6 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -18,6 +18,7 @@ chumsky.workspace = true thiserror.workspace = true smol_str.workspace = true serde_json.workspace = true +serde.workspace = true rustc-hash = "1.1.0" small-ord-set = "0.1.3" regex = "1.9.1" diff --git a/compiler/noirc_frontend/src/graph/mod.rs b/compiler/noirc_frontend/src/graph/mod.rs index 3a40c87d8e7..11024163199 100644 --- a/compiler/noirc_frontend/src/graph/mod.rs +++ b/compiler/noirc_frontend/src/graph/mod.rs @@ -8,6 +8,7 @@ use std::{fmt::Display, str::FromStr}; use fm::FileId; use rustc_hash::{FxHashMap, FxHashSet}; +use serde::Deserialize; use smol_str::SmolStr; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -32,7 +33,7 @@ impl CrateId { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd, Deserialize)] pub struct CrateName(SmolStr); impl CrateName { diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 5adaca9d356..b95063b6156 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -1,6 +1,6 @@ use fm::FileManager; use gloo_utils::format::JsValueSerdeExt; -use js_sys::Array; +use js_sys::Object; use nargo::artifacts::{ contract::{PreprocessedContract, PreprocessedContractFunction}, program::PreprocessedProgram, @@ -10,7 +10,7 @@ use noirc_driver::{ CompiledContract, CompiledProgram, }; use noirc_frontend::{ - graph::{CrateGraph, CrateId}, + graph::{CrateGraph, CrateId, CrateName}, hir::Context, }; use serde::Deserialize; @@ -21,32 +21,41 @@ use crate::errors::{CompileError, JsCompileError}; const BACKEND_IDENTIFIER: &str = "acvm-backend-barretenberg"; +#[wasm_bindgen(typescript_custom_section)] +const DEPENDENCY_GRAPH: &'static str = r#" +export type DependencyGraph = { + root_dependencies: readonly string[]; + library_dependencies: Readonly>; +} +"#; + #[wasm_bindgen] extern "C" { - #[wasm_bindgen(extends = Array, js_name = "StringArray", typescript_type = "string[]")] + #[wasm_bindgen(extends = Object, js_name = "DependencyGraph", typescript_type = "DependencyGraph")] #[derive(Clone, Debug, PartialEq, Eq)] - pub type StringArray; + pub type JsDependencyGraph; } #[derive(Deserialize)] struct DependencyGraph { - root_dependencies: Vec, - library_dependencies: HashMap>, + root_dependencies: Vec, + library_dependencies: HashMap>, } #[wasm_bindgen] pub fn compile( entry_point: String, contracts: Option, - dependency_graph: JsValue, + js_dependency_graph: Option, ) -> Result { console_error_panic_hook::set_once(); - let dependency_graph: DependencyGraph = - ::into_serde(&dependency_graph).unwrap_or(DependencyGraph { - root_dependencies: vec![], - library_dependencies: HashMap::new(), - }); + let dependency_graph: DependencyGraph = if let Some(js_dependency_graph) = js_dependency_graph { + ::into_serde(&JsValue::from(js_dependency_graph)) + .map_err(|err| err.to_string())? + } else { + DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() } + }; let root = Path::new("/"); let fm = FileManager::new(root, Box::new(get_non_stdlib_asset)); @@ -105,40 +114,36 @@ pub fn compile( } fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) { - let mut crate_names: HashMap = HashMap::new(); + let mut crate_names: HashMap<&CrateName, CrateId> = HashMap::new(); - // register libraries for lib in &dependency_graph.root_dependencies { - let crate_id = add_noir_lib(context, &lib); - crate_names.insert(lib.to_string(), crate_id); - } + let crate_id = add_noir_lib(context, lib); + crate_names.insert(lib, crate_id); - // make root crate as depending on it - for lib in &dependency_graph.root_dependencies { - let crate_id = crate_names.get(lib.as_str()).unwrap().clone(); - let root_crate_id = context.root_crate_id().clone(); - add_dep(context, root_crate_id, crate_id, lib.parse().unwrap()); + add_dep(context, *context.root_crate_id(), crate_id, lib.clone()); } - for (lib_name, dependencies) in dependency_graph.library_dependencies { - let crate_id = crate_names.get(lib_name.as_str()).unwrap().clone(); + for (lib_name, dependencies) in &dependency_graph.library_dependencies { + // first create the library crate if needed + // this crate might not have been registered yet because of the order of the HashMap + // e.g. {root: [lib1], libs: { lib2 -> [lib3], lib1 -> [lib2] }} + let crate_id = crate_names + .entry(&lib_name) + .or_insert_with(|| add_noir_lib(context, &lib_name)) + .clone(); for dependency_name in dependencies { - let lib_crate_id = match crate_names.entry(dependency_name.clone()) { - std::collections::hash_map::Entry::Occupied(entry) => entry.into_mut(), - std::collections::hash_map::Entry::Vacant(entry) => { - let crate_id = add_noir_lib(context, &dependency_name); - entry.insert(crate_id) - } - }; - - add_dep(context, crate_id, lib_crate_id.clone(), dependency_name.parse().unwrap()); + let dep_crate_id: &CrateId = crate_names + .entry(&dependency_name) + .or_insert_with(|| add_noir_lib(context, &dependency_name)); + + add_dep(context, crate_id, *dep_crate_id, dependency_name.clone()); } } } -fn add_noir_lib(context: &mut Context, library_name: &str) -> CrateId { - let path_to_lib = Path::new(&library_name).join("lib.nr"); +fn add_noir_lib(context: &mut Context, library_name: &CrateName) -> CrateId { + let path_to_lib = Path::new(&library_name.to_string()).join("lib.nr"); let library_crate_id = prepare_dependency(context, &path_to_lib); library_crate_id From 28b4bc27b64ff2176051767947c20316e3bcc984 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Fri, 20 Oct 2023 10:52:36 +0100 Subject: [PATCH 04/10] chore: apply clippy fixes --- compiler/wasm/src/compile.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index b95063b6156..d031c298867 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -127,15 +127,13 @@ fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyG // first create the library crate if needed // this crate might not have been registered yet because of the order of the HashMap // e.g. {root: [lib1], libs: { lib2 -> [lib3], lib1 -> [lib2] }} - let crate_id = crate_names - .entry(&lib_name) - .or_insert_with(|| add_noir_lib(context, &lib_name)) - .clone(); + let crate_id = + *crate_names.entry(lib_name).or_insert_with(|| add_noir_lib(context, lib_name)); for dependency_name in dependencies { let dep_crate_id: &CrateId = crate_names - .entry(&dependency_name) - .or_insert_with(|| add_noir_lib(context, &dependency_name)); + .entry(dependency_name) + .or_insert_with(|| add_noir_lib(context, dependency_name)); add_dep(context, crate_id, *dep_crate_id, dependency_name.clone()); } @@ -144,9 +142,7 @@ fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyG fn add_noir_lib(context: &mut Context, library_name: &CrateName) -> CrateId { let path_to_lib = Path::new(&library_name.to_string()).join("lib.nr"); - let library_crate_id = prepare_dependency(context, &path_to_lib); - - library_crate_id + prepare_dependency(context, &path_to_lib) } fn preprocess_program(program: CompiledProgram) -> PreprocessedProgram { From 87defd6d1d8bfd27049c6e26bca69165d1e32220 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Fri, 20 Oct 2023 13:26:24 +0100 Subject: [PATCH 05/10] refactor: change public parameter name Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- compiler/wasm/src/compile.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index d031c298867..8f9241963e0 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -46,12 +46,12 @@ struct DependencyGraph { pub fn compile( entry_point: String, contracts: Option, - js_dependency_graph: Option, + dependency_graph: Option, ) -> Result { console_error_panic_hook::set_once(); - let dependency_graph: DependencyGraph = if let Some(js_dependency_graph) = js_dependency_graph { - ::into_serde(&JsValue::from(js_dependency_graph)) + let dependency_graph: DependencyGraph = if let Some(dependency_graph) = dependency_graph { + ::into_serde(&JsValue::from(dependency_graph)) .map_err(|err| err.to_string())? } else { DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() } From 7902260fff796929dc5b9e93cf8ce0370bf0934f Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Fri, 20 Oct 2023 13:37:28 +0100 Subject: [PATCH 06/10] test: check that CrateName can serialize cleanly --- compiler/noirc_frontend/src/graph/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/graph/mod.rs b/compiler/noirc_frontend/src/graph/mod.rs index 11024163199..fe1edfa9c72 100644 --- a/compiler/noirc_frontend/src/graph/mod.rs +++ b/compiler/noirc_frontend/src/graph/mod.rs @@ -8,7 +8,7 @@ use std::{fmt::Display, str::FromStr}; use fm::FileId; use rustc_hash::{FxHashMap, FxHashSet}; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use smol_str::SmolStr; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -33,7 +33,7 @@ impl CrateId { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd, Serialize, Deserialize)] pub struct CrateName(SmolStr); impl CrateName { @@ -91,6 +91,14 @@ mod crate_name { assert!(!CrateName::is_valid_name(&bad_char_string)); } } + + #[test] + fn it_can_serialize_and_deserialize() { + let crate_name = "test_crate".parse::().unwrap(); + let serialized = serde_json::to_string(&crate_name).unwrap(); + let deserialized = serde_json::from_str::(&serialized).unwrap(); + assert_eq!(crate_name, deserialized); + } } #[derive(Debug, Clone, Default, PartialEq, Eq)] From cf2e67f61c6337ba1fa0181ae2d6665e3f3975a5 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Fri, 20 Oct 2023 16:43:34 +0100 Subject: [PATCH 07/10] test: check dependency graph --- compiler/wasm/src/compile.rs | 98 ++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 8f9241963e0..e41db278f7a 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -198,3 +198,101 @@ cfg_if::cfg_if! { } } } + +#[cfg(test)] +mod test { + use fm::FileManager; + use noirc_driver::prepare_crate; + use noirc_frontend::{ + graph::{CrateGraph, CrateName}, + hir::Context, + }; + + use super::{process_dependency_graph, DependencyGraph}; + use std::{collections::HashMap, path::Path}; + + fn mock_get_non_stdlib_asset(_path_to_file: &Path) -> std::io::Result { + Ok("".to_string()) + } + + fn setup_test_context() -> Context { + let fm = FileManager::new(Path::new("/"), Box::new(mock_get_non_stdlib_asset)); + let graph = CrateGraph::default(); + let mut context = Context::new(fm, graph); + + prepare_crate(&mut context, Path::new("/main.nr")); + + context + } + + fn crate_name(name: &str) -> CrateName { + name.parse().unwrap() + } + + #[test] + fn test_works_with_empty_dependency_graph() { + let mut context = setup_test_context(); + let dependency_graph = + DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() }; + + process_dependency_graph(&mut context, dependency_graph); + + // one stdlib + one root crate + assert_eq!(context.crate_graph.number_of_crates(), 2); + } + + #[test] + fn test_works_with_root_dependencies() { + let mut context = setup_test_context(); + let dependency_graph = DependencyGraph { + root_dependencies: vec![crate_name("lib1")], + library_dependencies: HashMap::new(), + }; + + process_dependency_graph(&mut context, dependency_graph); + + assert_eq!(context.crate_graph.number_of_crates(), 3); + } + + #[test] + fn test_works_with_duplicate_root_dependencies() { + let mut context = setup_test_context(); + let dependency_graph = DependencyGraph { + root_dependencies: vec![crate_name("lib1"), crate_name("lib1")], + library_dependencies: HashMap::new(), + }; + + process_dependency_graph(&mut context, dependency_graph); + + assert_eq!(context.crate_graph.number_of_crates(), 3); + } + + #[test] + fn test_works_with_transitive_dependencies() { + let mut context = setup_test_context(); + let dependency_graph = DependencyGraph { + root_dependencies: vec![crate_name("lib1")], + library_dependencies: HashMap::from([ + (crate_name("lib1"), vec![crate_name("lib2")]), + (crate_name("lib2"), vec![crate_name("lib3")]), + ]), + }; + + process_dependency_graph(&mut context, dependency_graph); + + assert_eq!(context.crate_graph.number_of_crates(), 5); + } + + #[test] + fn test_works_with_missing_dependencies() { + let mut context = setup_test_context(); + let dependency_graph = DependencyGraph { + root_dependencies: vec![crate_name("lib1")], + library_dependencies: HashMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]), + }; + + process_dependency_graph(&mut context, dependency_graph); + + assert_eq!(context.crate_graph.number_of_crates(), 5); + } +} From 15cf4168c1d3b65bc15133153cb1a42ba5423f60 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Mon, 23 Oct 2023 09:42:32 +0100 Subject: [PATCH 08/10] test: add e2e test for script with deps --- .github/workflows/test-noir_wasm.yml | 10 +- compiler/wasm/fixtures/deps/lib-a/Nargo.toml | 8 ++ compiler/wasm/fixtures/deps/lib-a/src/lib.nr | 7 ++ compiler/wasm/fixtures/deps/lib-b/Nargo.toml | 7 ++ compiler/wasm/fixtures/deps/lib-b/src/lib.nr | 4 + .../wasm/fixtures/deps/noir-script/Nargo.toml | 8 ++ .../fixtures/deps/noir-script/src/main.nr | 4 + .../simple}/noir-script/Nargo.toml | 0 .../simple}/noir-script/src/main.nr | 0 compiler/wasm/test/browser/index.test.ts | 110 +++++++++++------- compiler/wasm/test/node/index.test.ts | 76 +++++++++--- compiler/wasm/test/shared.ts | 10 +- 12 files changed, 184 insertions(+), 60 deletions(-) create mode 100644 compiler/wasm/fixtures/deps/lib-a/Nargo.toml create mode 100644 compiler/wasm/fixtures/deps/lib-a/src/lib.nr create mode 100644 compiler/wasm/fixtures/deps/lib-b/Nargo.toml create mode 100644 compiler/wasm/fixtures/deps/lib-b/src/lib.nr create mode 100644 compiler/wasm/fixtures/deps/noir-script/Nargo.toml create mode 100644 compiler/wasm/fixtures/deps/noir-script/src/main.nr rename compiler/wasm/{ => fixtures/simple}/noir-script/Nargo.toml (100%) rename compiler/wasm/{ => fixtures/simple}/noir-script/src/main.nr (100%) diff --git a/.github/workflows/test-noir_wasm.yml b/.github/workflows/test-noir_wasm.yml index 86362ef7ceb..eea4074e00a 100644 --- a/.github/workflows/test-noir_wasm.yml +++ b/.github/workflows/test-noir_wasm.yml @@ -101,12 +101,16 @@ jobs: name: nargo path: ./nargo - - name: Compile test program with Nargo CLI - working-directory: ./compiler/wasm/noir-script + - name: Compile fixtures with Nargo CLI + working-directory: ./compiler/wasm/fixtures run: | nargo_binary=${{ github.workspace }}/nargo/nargo chmod +x $nargo_binary - $nargo_binary compile + for dir in $(ls -d */); do + pushd $dir/noir-script + $nargo_binary compile + popd + done - name: Install Yarn dependencies uses: ./.github/actions/setup diff --git a/compiler/wasm/fixtures/deps/lib-a/Nargo.toml b/compiler/wasm/fixtures/deps/lib-a/Nargo.toml new file mode 100644 index 00000000000..3a02ccc1086 --- /dev/null +++ b/compiler/wasm/fixtures/deps/lib-a/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name="lib_a" +type="lib" +authors = [""] +compiler_version = "0.1" + +[dependencies] +lib_b = { path = "../lib-b" } diff --git a/compiler/wasm/fixtures/deps/lib-a/src/lib.nr b/compiler/wasm/fixtures/deps/lib-a/src/lib.nr new file mode 100644 index 00000000000..abb4302ba38 --- /dev/null +++ b/compiler/wasm/fixtures/deps/lib-a/src/lib.nr @@ -0,0 +1,7 @@ + +use dep::lib_b::assert_non_zero; + +pub fn divide(a: u64, b: u64) -> u64 { + assert_non_zero(b); + a / b +} diff --git a/compiler/wasm/fixtures/deps/lib-b/Nargo.toml b/compiler/wasm/fixtures/deps/lib-b/Nargo.toml new file mode 100644 index 00000000000..99db61e4bfa --- /dev/null +++ b/compiler/wasm/fixtures/deps/lib-b/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name="lib_b" +type="lib" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/compiler/wasm/fixtures/deps/lib-b/src/lib.nr b/compiler/wasm/fixtures/deps/lib-b/src/lib.nr new file mode 100644 index 00000000000..ad8b26020e3 --- /dev/null +++ b/compiler/wasm/fixtures/deps/lib-b/src/lib.nr @@ -0,0 +1,4 @@ + +pub fn assert_non_zero(x: u64) { + assert(x != 0); +} diff --git a/compiler/wasm/fixtures/deps/noir-script/Nargo.toml b/compiler/wasm/fixtures/deps/noir-script/Nargo.toml new file mode 100644 index 00000000000..0df22c2be7e --- /dev/null +++ b/compiler/wasm/fixtures/deps/noir-script/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name="noir_wasm_testing" +type="bin" +authors = [""] +compiler_version = "0.1" + +[dependencies] +lib_a = { path="../lib-a" } diff --git a/compiler/wasm/fixtures/deps/noir-script/src/main.nr b/compiler/wasm/fixtures/deps/noir-script/src/main.nr new file mode 100644 index 00000000000..d46597fca2e --- /dev/null +++ b/compiler/wasm/fixtures/deps/noir-script/src/main.nr @@ -0,0 +1,4 @@ +use dep::lib_a::divide; +fn main(x : u64, y : pub u64) { + divide(x, y); +} diff --git a/compiler/wasm/noir-script/Nargo.toml b/compiler/wasm/fixtures/simple/noir-script/Nargo.toml similarity index 100% rename from compiler/wasm/noir-script/Nargo.toml rename to compiler/wasm/fixtures/simple/noir-script/Nargo.toml diff --git a/compiler/wasm/noir-script/src/main.nr b/compiler/wasm/fixtures/simple/noir-script/src/main.nr similarity index 100% rename from compiler/wasm/noir-script/src/main.nr rename to compiler/wasm/fixtures/simple/noir-script/src/main.nr diff --git a/compiler/wasm/test/browser/index.test.ts b/compiler/wasm/test/browser/index.test.ts index 02263d9adfb..cad2ada0c61 100644 --- a/compiler/wasm/test/browser/index.test.ts +++ b/compiler/wasm/test/browser/index.test.ts @@ -1,68 +1,98 @@ import { expect } from '@esm-bundle/chai'; import initNoirWasm, { compile } from '@noir-lang/noir_wasm'; import { initializeResolver } from '@noir-lang/source-resolver'; -import { nargoArtifactPath, noirSourcePath } from '../shared'; +import { + depsScriptExpectedArtifact, + depsScriptSourcePath, + libASourcePath, + libBSourcePath, + simpleScriptExpectedArtifact, + simpleScriptSourcePath, +} from '../shared'; beforeEach(async () => { await initNoirWasm(); }); async function getFileContent(path: string): Promise { - const mainnrSourceURL = new URL(path, import.meta.url); - const response = await fetch(mainnrSourceURL); + const url = new URL(path, import.meta.url); + const response = await fetch(url); return await response.text(); } -async function getSource(): Promise { - return getFileContent(noirSourcePath); -} - // eslint-disable-next-line @typescript-eslint/no-explicit-any -async function getPrecompiledSource(): Promise { - const compiledData = await getFileContent(nargoArtifactPath); +async function getPrecompiledSource(path: string): Promise { + const compiledData = await getFileContent(path); return JSON.parse(compiledData); } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export async function compileNoirSource(noir_source: string): Promise { - console.log('Compiling Noir source...'); +describe('noir wasm', () => { + describe('can compile script without dependencies', () => { + beforeEach(async () => { + const source = await getFileContent(simpleScriptSourcePath); + initializeResolver((id: string) => { + console.log(`Resolving source ${id}`); - initializeResolver((id: string) => { - console.log(`Resolving source ${id}`); + if (typeof source === 'undefined') { + throw Error(`Could not resolve source for '${id}'`); + } else if (id !== '/main.nr') { + throw Error(`Unexpected id: '${id}'`); + } else { + return source; + } + }); + }); - const source = noir_source; + it('matching nargos compilation', async () => { + const wasmCircuit = await compile('/main.nr'); + const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); - if (typeof source === 'undefined') { - throw Error(`Could not resolve source for '${id}'`); - } else if (id !== '/main.nr') { - throw Error(`Unexpected id: '${id}'`); - } else { - return source; - } + // We don't expect the hashes to match due to how `noir_wasm` handles dependencies + expect(wasmCircuit.bytecode).to.eq(cliCircuit.bytecode); + expect(wasmCircuit.abi).to.deep.eq(cliCircuit.abi); + expect(wasmCircuit.backend).to.eq(cliCircuit.backend); + }).timeout(20e3); // 20 seconds }); - try { - const compiled_noir = compile('main.nr'); + describe('can compile script with dependencies', () => { + beforeEach(async () => { + const [scriptSource, libASource, libBSource] = await Promise.all([ + getFileContent(depsScriptSourcePath), + getFileContent(libASourcePath), + getFileContent(libBSourcePath), + ]); - console.log('Noir source compilation done.'); + initializeResolver((file: string) => { + switch (file) { + case '/script/main.nr': + return scriptSource; - return compiled_noir; - } catch (e) { - console.log('Error while compiling:', e); - } -} + case '/lib_a/lib.nr': + return libASource; + + case '/lib_b/lib.nr': + return libBSource; -describe('noir wasm compilation', () => { - it('matches nargos compilation', async () => { - const source = await getSource(); + default: + return ''; + } + }); + }); - const wasmCircuit = await compileNoirSource(source); + it('matching nargos compilation', async () => { + const wasmCircuit = await compile('/script/main.nr', false, { + root_dependencies: ['lib_a'], + library_dependencies: { + lib_a: ['lib_b'], + }, + }); - const cliCircuit = await getPrecompiledSource(); + const cliCircuit = await getPrecompiledSource(depsScriptExpectedArtifact); - // We don't expect the hashes to match due to how `noir_wasm` handles dependencies - expect(wasmCircuit.bytecode).to.eq(cliCircuit.bytecode); - expect(wasmCircuit.abi).to.deep.eq(cliCircuit.abi); - expect(wasmCircuit.backend).to.eq(cliCircuit.backend); - }).timeout(20e3); // 20 seconds + // We don't expect the hashes to match due to how `noir_wasm` handles dependencies + expect(wasmCircuit.bytecode).to.eq(cliCircuit.bytecode); + expect(wasmCircuit.abi).to.deep.eq(cliCircuit.abi); + expect(wasmCircuit.backend).to.eq(cliCircuit.backend); + }).timeout(20e3); // 20 seconds + }); }); diff --git a/compiler/wasm/test/node/index.test.ts b/compiler/wasm/test/node/index.test.ts index 4ec6d83c3c3..a491ea4e55b 100644 --- a/compiler/wasm/test/node/index.test.ts +++ b/compiler/wasm/test/node/index.test.ts @@ -1,26 +1,72 @@ import { expect } from 'chai'; -import { nargoArtifactPath, noirSourcePath } from '../shared'; +import { + depsScriptSourcePath, + depsScriptExpectedArtifact, + libASourcePath, + libBSourcePath, + simpleScriptSourcePath, + simpleScriptExpectedArtifact, +} from '../shared'; import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; +import { join, resolve } from 'node:path'; import { compile } from '@noir-lang/noir_wasm'; +import { initializeResolver } from '@noir-lang/source-resolver'; -const absoluteNoirSourcePath = join(__dirname, noirSourcePath); -const absoluteNargoArtifactPath = join(__dirname, nargoArtifactPath); +// const absoluteNoirSourcePath = ; +// const absoluteNargoArtifactPath = join(__dirname, nargoArtifactPath); // eslint-disable-next-line @typescript-eslint/no-explicit-any -async function getPrecompiledSource(): Promise { - const compiledData = readFileSync(absoluteNargoArtifactPath).toString(); +async function getPrecompiledSource(path: string): Promise { + const compiledData = readFileSync(resolve(__dirname, path)).toString(); return JSON.parse(compiledData); } describe('noir wasm compilation', () => { - it('matches nargos compilation', async () => { - const wasmCircuit = await compile(absoluteNoirSourcePath); - const cliCircuit = await getPrecompiledSource(); - - // We don't expect the hashes to match due to how `noir_wasm` handles dependencies - expect(wasmCircuit.bytecode).to.eq(cliCircuit.bytecode); - expect(wasmCircuit.abi).to.deep.eq(cliCircuit.abi); - expect(wasmCircuit.backend).to.eq(cliCircuit.backend); - }).timeout(10e3); + describe('can compile simple scripts', () => { + it('matching nargos compilation', async () => { + const wasmCircuit = await compile(join(__dirname, simpleScriptSourcePath)); + const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); + + // We don't expect the hashes to match due to how `noir_wasm` handles dependencies + expect(wasmCircuit.bytecode).to.eq(cliCircuit.bytecode); + expect(wasmCircuit.abi).to.deep.eq(cliCircuit.abi); + expect(wasmCircuit.backend).to.eq(cliCircuit.backend); + }).timeout(10e3); + }); + + describe('can compile scripts with dependencies', () => { + beforeEach(() => { + initializeResolver((file) => { + switch (file) { + case '/script/main.nr': + return readFileSync(join(__dirname, depsScriptSourcePath), 'utf-8'); + + case '/lib_a/lib.nr': + return readFileSync(join(__dirname, libASourcePath), 'utf-8'); + + case '/lib_b/lib.nr': + return readFileSync(join(__dirname, libBSourcePath), 'utf-8'); + + default: + return ''; + } + }); + }); + + it('matching nargos compilation', async () => { + const wasmCircuit = await compile('/script/main.nr', false, { + root_dependencies: ['lib_a'], + library_dependencies: { + lib_a: ['lib_b'], + }, + }); + + const cliCircuit = await getPrecompiledSource(depsScriptExpectedArtifact); + + // We don't expect the hashes to match due to how `noir_wasm` handles dependencies + expect(wasmCircuit.bytecode).to.eq(cliCircuit.bytecode); + expect(wasmCircuit.abi).to.deep.eq(cliCircuit.abi); + expect(wasmCircuit.backend).to.eq(cliCircuit.backend); + }).timeout(10e3); + }); }); diff --git a/compiler/wasm/test/shared.ts b/compiler/wasm/test/shared.ts index f726316cd74..6fc370f7ac8 100644 --- a/compiler/wasm/test/shared.ts +++ b/compiler/wasm/test/shared.ts @@ -1,2 +1,8 @@ -export const noirSourcePath = '../../noir-script/src/main.nr'; -export const nargoArtifactPath = '../../noir-script/target/noir_wasm_testing.json'; +export const simpleScriptSourcePath = '../../fixtures/simple/noir-script/src/main.nr'; +export const simpleScriptExpectedArtifact = '../../fixtures/simple/noir-script/target/noir_wasm_testing.json'; + +export const depsScriptSourcePath = '../../fixtures/deps/noir-script/src/main.nr'; +export const depsScriptExpectedArtifact = '../../fixtures/deps/noir-script/target/noir_wasm_testing.json'; + +export const libASourcePath = '../../fixtures/deps/lib-a/src/lib.nr'; +export const libBSourcePath = '../../fixtures/deps/lib-b/src/lib.nr'; From b9a6a6862e25b9b4419445248ba23e1940e2b40a Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Mon, 23 Oct 2023 12:44:56 +0100 Subject: [PATCH 09/10] fix: ts definition of diagnostic --- compiler/wasm/src/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/wasm/src/errors.rs b/compiler/wasm/src/errors.rs index 57295916dc5..9aafcadc27f 100644 --- a/compiler/wasm/src/errors.rs +++ b/compiler/wasm/src/errors.rs @@ -10,7 +10,7 @@ use noirc_errors::FileDiagnostic; const DIAGNOSTICS: &'static str = r#" export type Diagnostic = { message: string; - file_path: string; + file: string; secondaries: ReadonlyArray<{ message: string; start: number; From df506d691e4290b80c9dadd4b896a50097ce5d89 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Mon, 23 Oct 2023 17:42:27 +0100 Subject: [PATCH 10/10] fix: address code review --- compiler/noirc_frontend/src/graph/mod.rs | 7 ++----- compiler/wasm/test/node/index.test.ts | 4 +--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/graph/mod.rs b/compiler/noirc_frontend/src/graph/mod.rs index fe1edfa9c72..452aef74b36 100644 --- a/compiler/noirc_frontend/src/graph/mod.rs +++ b/compiler/noirc_frontend/src/graph/mod.rs @@ -93,11 +93,8 @@ mod crate_name { } #[test] - fn it_can_serialize_and_deserialize() { - let crate_name = "test_crate".parse::().unwrap(); - let serialized = serde_json::to_string(&crate_name).unwrap(); - let deserialized = serde_json::from_str::(&serialized).unwrap(); - assert_eq!(crate_name, deserialized); + fn it_rejects_bad_crate_names_when_deserializing() { + assert!(serde_json::from_str::("bad-name").is_err()); } } diff --git a/compiler/wasm/test/node/index.test.ts b/compiler/wasm/test/node/index.test.ts index a491ea4e55b..2b430abdca4 100644 --- a/compiler/wasm/test/node/index.test.ts +++ b/compiler/wasm/test/node/index.test.ts @@ -12,9 +12,6 @@ import { join, resolve } from 'node:path'; import { compile } from '@noir-lang/noir_wasm'; import { initializeResolver } from '@noir-lang/source-resolver'; -// const absoluteNoirSourcePath = ; -// const absoluteNargoArtifactPath = join(__dirname, nargoArtifactPath); - // eslint-disable-next-line @typescript-eslint/no-explicit-any async function getPrecompiledSource(path: string): Promise { const compiledData = readFileSync(resolve(__dirname, path)).toString(); @@ -36,6 +33,7 @@ describe('noir wasm compilation', () => { describe('can compile scripts with dependencies', () => { beforeEach(() => { + // this test requires a custom resolver in order to correctly resolve dependencies initializeResolver((file) => { switch (file) { case '/script/main.nr':