diff --git a/Cargo.lock b/Cargo.lock index 960ad27dbcb..f7d2e63662f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,9 +4,9 @@ version = 3 [[package]] name = "acir" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a8e08ddd57e123075a643f9515f712c34ebcfd6333628a4e8972af41f21a694" +checksum = "269617cfc7050d47915308c2ae20c33642cbfbc5c25097ba10a522539556065b" dependencies = [ "acir_field", "bincode", @@ -18,9 +18,9 @@ dependencies = [ [[package]] name = "acir_field" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "729849a0f1ca22f267c83b4d04043f6fa44f159a6606dc58e586c131c67ab774" +checksum = "d0ace9d882b85479aa678c7272aa717acdcabe1f4d0cb6236cd17dcda8c4bb32" dependencies = [ "ark-bn254", "ark-ff", @@ -32,9 +32,9 @@ dependencies = [ [[package]] name = "acvm" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f5f1c272779208fe823fe2a327d5e85efb6b8861e858507df4d532ec372514ee" +checksum = "f5eeabcd9c38133eb860c1ad4c0ced8d0032f9f22b6de0699bddacb69bd772ec" dependencies = [ "acir", "acvm_blackbox_solver", @@ -70,9 +70,9 @@ dependencies = [ [[package]] name = "acvm_blackbox_solver" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ee8080267f367360522f24a5cd0de2bf7b9d31795266471c69e352083ceda39" +checksum = "53e3636cfc0b6a6e6e02a5b813b12941ef0979ec532290ee83587f42d7610dae" dependencies = [ "acir", "blake2", @@ -85,9 +85,9 @@ dependencies = [ [[package]] name = "acvm_stdlib" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e4b52ef8c927058b451576fa7ab7e173c2b302ee81a0337d2420d3ca01c71d5e" +checksum = "87fc3882d621cb13af2793e35fff3aa8ba34959c486808e665212d5423835386" dependencies = [ "acir", ] @@ -533,9 +533,9 @@ dependencies = [ [[package]] name = "brillig" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30647da3d1347c3f3a3c0e955f75b1fd3a91c123ee87d6b55d94c042d68e2ae6" +checksum = "23339a18ea207882da08076a70564b5a44bad80ce2ebb3a46e89172979ae09fb" dependencies = [ "acir_field", "serde", @@ -543,9 +543,9 @@ dependencies = [ [[package]] name = "brillig_vm" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0dc5c4ce5b6553b4d48f9895ca6b336a3672f1dc802792a37f13dcb8902d47b5" +checksum = "024f52eb86d8f320cbbf8c2f231acdd84fe2171d3d1926c4bc95d64e933aa23e" dependencies = [ "acir", "acvm_blackbox_solver", @@ -2047,6 +2047,7 @@ dependencies = [ "async-lsp", "codespan-lsp", "codespan-reporting", + "fm", "lsp-types 0.94.0", "noirc_driver", "noirc_errors", @@ -2064,6 +2065,7 @@ dependencies = [ "acvm", "build-data", "console_error_panic_hook", + "fm", "getrandom", "gloo-utils", "log", diff --git a/Cargo.toml b/Cargo.toml index 42028afd18f..2dde6ff67c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ edition = "2021" rust-version = "1.66" [workspace.dependencies] -acvm = "0.19.0" +acvm = "0.19.1" arena = { path = "crates/arena" } fm = { path = "crates/fm" } iter-extended = { path = "crates/iter-extended" } @@ -40,7 +40,7 @@ noir_wasm = { path = "crates/wasm" } cfg-if = "1.0.0" clap = { version = "4.1.4", features = ["derive"] } -codespan = {version = "0.11.1", features = ["serialization"]} +codespan = { version = "0.11.1", features = ["serialization"] } codespan-lsp = "0.11.1" codespan-reporting = "0.11.1" chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" } diff --git a/crates/fm/src/file_reader.rs b/crates/fm/src/file_reader.rs index f0a5fe625aa..df4e49b919a 100644 --- a/crates/fm/src/file_reader.rs +++ b/crates/fm/src/file_reader.rs @@ -11,6 +11,16 @@ use std::path::Path; #[cfg_attr(target_os = "windows", prefix = r"std\")] // Note reversed slash direction struct StdLibAssets; +#[cfg(target_os = "windows")] +pub(super) fn is_stdlib_asset(path: &Path) -> bool { + path.starts_with("std\\") +} + +#[cfg(not(target_os = "windows"))] +pub(super) fn is_stdlib_asset(path: &Path) -> bool { + path.starts_with("std/") +} + cfg_if::cfg_if! { if #[cfg(target_arch = "wasm32")] { use wasm_bindgen::{prelude::*, JsValue}; @@ -41,6 +51,7 @@ cfg_if::cfg_if! { } } } else { + pub(crate) fn read_file_to_string(path_to_file: &Path) -> Result { match StdLibAssets::get(path_to_file.to_str().unwrap()) { diff --git a/crates/fm/src/lib.rs b/crates/fm/src/lib.rs index d22d74c757a..368043ea601 100644 --- a/crates/fm/src/lib.rs +++ b/crates/fm/src/lib.rs @@ -7,10 +7,11 @@ mod file_map; mod file_reader; pub use file_map::{File, FileId, FileMap}; +use file_reader::is_stdlib_asset; use std::{ collections::HashMap, - path::{Path, PathBuf}, + path::{Component, Path, PathBuf}, }; pub const FILE_EXTENSION: &str = "nr"; @@ -18,30 +19,43 @@ pub const FILE_EXTENSION: &str = "nr"; #[derive(Clone, Debug, PartialEq, Eq, Hash)] struct VirtualPath(PathBuf); -#[derive(Debug, Default)] +#[derive(Debug)] pub struct FileManager { + root: PathBuf, file_map: file_map::FileMap, id_to_path: HashMap, path_to_id: HashMap, } impl FileManager { - // XXX: Maybe use a AsRef here, for API ergonomics - pub fn add_file(&mut self, path_to_file: &Path) -> Option { + pub fn new(root: &Path) -> Self { + Self { + root: normalize_path(root), + file_map: Default::default(), + id_to_path: Default::default(), + path_to_id: Default::default(), + } + } + + pub fn add_file(&mut self, file_name: &Path) -> Option { // Handle both relative file paths and std/lib virtual paths. - let base = Path::new(".").canonicalize().expect("Base path canonicalize failed"); - let res = path_to_file.canonicalize().unwrap_or_else(|_| path_to_file.to_path_buf()); - let resolved_path = res.strip_prefix(base).unwrap_or(&res); + let resolved_path: PathBuf = if is_stdlib_asset(file_name) { + // Special case for stdlib where we want to read specifically the `std/` relative path + // TODO: The stdlib path should probably be an absolute path rooted in something people would never create + file_name.to_path_buf() + } else { + normalize_path(&self.root.join(file_name)) + }; // Check that the resolved path already exists in the file map, if it is, we return it. - let path_to_file = virtualize_path(resolved_path); + let path_to_file = virtualize_path(&resolved_path); if let Some(file_id) = self.path_to_id.get(&path_to_file) { return Some(*file_id); } // Otherwise we add the file - let source = file_reader::read_file_to_string(resolved_path).ok()?; - let file_id = self.file_map.add_file(resolved_path.to_path_buf().into(), source); + let source = file_reader::read_file_to_string(&resolved_path).ok()?; + let file_id = self.file_map.add_file(resolved_path.into(), source); self.register_path(file_id, path_to_file); Some(file_id) } @@ -87,6 +101,37 @@ impl FileManager { } } +/// Replacement for `std::fs::canonicalize` that doesn't verify the path exists. +/// +/// Plucked from https://github.com/rust-lang/cargo/blob/fede83ccf973457de319ba6fa0e36ead454d2e20/src/cargo/util/paths.rs#L61 +/// Advice from https://www.reddit.com/r/rust/comments/hkkquy/comment/fwtw53s/ +fn normalize_path(path: &Path) -> PathBuf { + let mut components = path.components().peekable(); + let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { + components.next(); + PathBuf::from(c.as_os_str()) + } else { + PathBuf::new() + }; + + for component in components { + match component { + Component::Prefix(..) => unreachable!(), + Component::RootDir => { + ret.push(component.as_os_str()); + } + Component::CurDir => {} + Component::ParentDir => { + ret.pop(); + } + Component::Normal(c) => { + ret.push(c); + } + } + } + ret +} + /// Takes a path to a noir file. This will panic on paths to directories /// Returns the file path with the extension removed fn virtualize_path(path: &Path) -> VirtualPath { @@ -102,46 +147,49 @@ mod tests { use super::*; use tempfile::{tempdir, TempDir}; - fn dummy_file_path(dir: &TempDir, file_name: &str) -> PathBuf { + fn create_dummy_file(dir: &TempDir, file_name: &Path) { let file_path = dir.path().join(file_name); let _file = std::fs::File::create(file_path.clone()).unwrap(); - - file_path } #[test] fn path_resolve_file_module() { let dir = tempdir().unwrap(); - let file_path = dummy_file_path(&dir, "my_dummy_file.nr"); - let mut fm = FileManager::default(); + let entry_file_name = Path::new("my_dummy_file.nr"); + create_dummy_file(&dir, entry_file_name); + + let mut fm = FileManager::new(dir.path()); - let file_id = fm.add_file(&file_path).unwrap(); + let file_id = fm.add_file(entry_file_name).unwrap(); - let _foo_file_path = dummy_file_path(&dir, "foo.nr"); + let dep_file_name = Path::new("foo.nr"); + create_dummy_file(&dir, dep_file_name); fm.resolve_path(file_id, "foo").unwrap(); } #[test] fn path_resolve_file_module_other_ext() { let dir = tempdir().unwrap(); - let file_path = dummy_file_path(&dir, "foo.noir"); + let file_name = Path::new("foo.noir"); + create_dummy_file(&dir, file_name); - let mut fm = FileManager::default(); + let mut fm = FileManager::new(dir.path()); - let file_id = fm.add_file(&file_path).unwrap(); + let file_id = fm.add_file(&file_name).unwrap(); assert!(fm.path(file_id).ends_with("foo")); } #[test] fn path_resolve_sub_module() { - let mut fm = FileManager::default(); - let dir = tempdir().unwrap(); + let mut fm = FileManager::new(dir.path()); + // Create a lib.nr file at the root. // we now have dir/lib.nr - let file_path = dummy_file_path(&dir, "lib.nr"); + let file_name = Path::new("lib.nr"); + create_dummy_file(&dir, file_name); - let file_id = fm.add_file(&file_path).unwrap(); + let file_id = fm.add_file(&file_name).unwrap(); // Create a sub directory // we now have: @@ -154,14 +202,14 @@ mod tests { // we no have: // - dir/lib.nr // - dir/sub_dir/foo.nr - let _foo_file_path = dummy_file_path(&sub_dir, "foo.nr"); + create_dummy_file(&sub_dir, Path::new("foo.nr")); // Add a parent module for the sub_dir // we no have: // - dir/lib.nr // - dir/sub_dir.nr // - dir/sub_dir/foo.nr - let _sub_dir_root_file_path = dummy_file_path(&dir, &format!("{}.nr", sub_dir_name)); + create_dummy_file(&dir, Path::new(&format!("{}.nr", sub_dir_name))); // First check for the sub_dir.nr file and add it to the FileManager let sub_dir_file_id = fm.resolve_path(file_id, sub_dir_name).unwrap(); @@ -176,20 +224,22 @@ mod tests { /// they should both resolve to ../foo.nr #[test] fn path_resolve_modules_with_different_paths_as_same_file() { - let mut fm = FileManager::default(); - - // Create a lib.nr file at the root. let dir = tempdir().unwrap(); let sub_dir = TempDir::new_in(&dir).unwrap(); let sub_sub_dir = TempDir::new_in(&sub_dir).unwrap(); - let file_path = dummy_file_path(&dir, "lib.nr"); - // Create another file in a subdirectory with a convoluted path - let second_file_path = dummy_file_path(&sub_sub_dir, "./../../lib.nr"); + let mut fm = FileManager::new(dir.path()); + + // Create a lib.nr file at the root. + let file_name = Path::new("lib.nr"); + create_dummy_file(&dir, file_name); + + // Create another path with `./` and `../` inside it + let second_file_name = PathBuf::from(sub_sub_dir.path()).join("./../../lib.nr"); // Add both files to the file manager - let file_id = fm.add_file(&file_path).unwrap(); - let second_file_id = fm.add_file(&second_file_path).unwrap(); + let file_id = fm.add_file(&file_name).unwrap(); + let second_file_id = fm.add_file(&second_file_name).unwrap(); assert_eq!(file_id, second_file_id); } diff --git a/crates/lsp/Cargo.toml b/crates/lsp/Cargo.toml index 47278ab5291..2611abd67dc 100644 --- a/crates/lsp/Cargo.toml +++ b/crates/lsp/Cargo.toml @@ -11,6 +11,7 @@ edition.workspace = true acvm.workspace = true codespan-lsp.workspace = true codespan-reporting.workspace = true +fm.workspace = true lsp-types.workspace = true noirc_driver.workspace = true noirc_errors.workspace = true diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index cab74bd5560..953540226ad 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -2,17 +2,19 @@ mod lib_hacky; use std::env; use std::{ - future::Future, + future::{self, Future}, ops::{self, ControlFlow}, + path::PathBuf, pin::Pin, task::{self, Poll}, }; use async_lsp::{ - router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, LanguageClient, - LspService, ResponseError, + router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, ErrorCode, + LanguageClient, LspService, ResponseError, }; use codespan_reporting::files; +use fm::FileManager; use lsp_types::{ notification, request, CodeLens, CodeLensOptions, CodeLensParams, Command, Diagnostic, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, @@ -22,7 +24,10 @@ use lsp_types::{ }; use noirc_driver::{check_crate, create_local_crate}; use noirc_errors::{DiagnosticKind, FileDiagnostic}; -use noirc_frontend::{graph::CrateType, hir::Context}; +use noirc_frontend::{ + graph::{CrateGraph, CrateType}, + hir::Context, +}; use serde_json::Value as JsonValue; use tower::Service; @@ -31,12 +36,13 @@ const TEST_CODELENS_TITLE: &str = "▶\u{fe0e} Run Test"; // State for the LSP gets implemented on this struct and is internal to the implementation pub struct LspState { + root_path: Option, client: ClientSocket, } impl LspState { fn new(client: &ClientSocket) -> Self { - Self { client: client.clone() } + Self { client: client.clone(), root_path: None } } } @@ -131,9 +137,13 @@ impl LspService for NargoLspService { // and params passed in. fn on_initialize( - _state: &mut LspState, - _params: InitializeParams, + state: &mut LspState, + params: InitializeParams, ) -> impl Future> { + if let Some(root_uri) = params.root_uri { + state.root_path = root_uri.to_file_path().ok(); + } + async { let text_document_sync = TextDocumentSyncOptions { save: Some(true.into()), ..Default::default() }; @@ -160,12 +170,25 @@ fn on_shutdown( } fn on_code_lens_request( - _state: &mut LspState, + state: &mut LspState, params: CodeLensParams, ) -> impl Future>, ResponseError>> { let file_path = ¶ms.text_document.uri.to_file_path().unwrap(); - let mut context = Context::default(); + let mut context = match &state.root_path { + Some(root_path) => { + let fm = FileManager::new(root_path); + let graph = CrateGraph::default(); + Context::new(fm, graph) + } + None => { + let err = ResponseError::new( + ErrorCode::REQUEST_FAILED, + "Unable to determine the project root path", + ); + return future::ready(Err(err)); + } + }; let crate_id = create_local_crate(&mut context, file_path, CrateType::Binary); @@ -202,13 +225,9 @@ fn on_code_lens_request( lenses.push(lens); } - async move { - if lenses.is_empty() { - Ok(None) - } else { - Ok(Some(lenses)) - } - } + let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; + + future::ready(res) } fn on_initialized( @@ -251,8 +270,20 @@ fn on_did_save_text_document( params: DidSaveTextDocumentParams, ) -> ControlFlow> { let file_path = ¶ms.text_document.uri.to_file_path().unwrap(); - - let mut context = Context::default(); + let mut context = match &state.root_path { + Some(root_path) => { + let fm = FileManager::new(root_path); + let graph = CrateGraph::default(); + Context::new(fm, graph) + } + None => { + let err = ResponseError::new( + ErrorCode::REQUEST_FAILED, + "Unable to determine the project root path", + ); + return ControlFlow::Break(Err(err.into())); + } + }; let crate_id = create_local_crate(&mut context, file_path, CrateType::Binary); diff --git a/crates/lsp/src/lib_hacky.rs b/crates/lsp/src/lib_hacky.rs index 36feee27348..0fea6c43144 100644 --- a/crates/lsp/src/lib_hacky.rs +++ b/crates/lsp/src/lib_hacky.rs @@ -4,13 +4,14 @@ use std::{ collections::HashMap, fs, - future::Future, + future::{self, Future}, ops::{self, ControlFlow}, path::{Path, PathBuf}, }; -use async_lsp::{LanguageClient, ResponseError}; +use async_lsp::{ErrorCode, LanguageClient, ResponseError}; use codespan_reporting::files; +use fm::FileManager; use lsp_types::{ CodeLens, CodeLensOptions, CodeLensParams, Command, Diagnostic, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, DidCloseTextDocumentParams, @@ -21,10 +22,11 @@ use lsp_types::{ use noirc_driver::{check_crate, create_local_crate, create_non_local_crate, propagate_dep}; use noirc_errors::{DiagnosticKind, FileDiagnostic}; use noirc_frontend::{ - graph::{CrateId, CrateName, CrateType}, + graph::{CrateGraph, CrateId, CrateName, CrateType}, hir::Context, }; +// I'm guessing this is here so the `lib.rs` file compiles use crate::LspState; const TEST_COMMAND: &str = "nargo.test"; @@ -41,9 +43,13 @@ const TEST_CODELENS_TITLE: &str = "▶\u{fe0e} Run Test"; // and params passed in. pub fn on_initialize( - _state: &mut LspState, - _params: InitializeParams, + state: &mut LspState, + params: InitializeParams, ) -> impl Future> { + if let Some(root_uri) = params.root_uri { + state.root_path = root_uri.to_file_path().ok(); + } + async { let text_document_sync = TextDocumentSyncOptions { save: Some(true.into()), ..Default::default() }; @@ -70,30 +76,33 @@ pub fn on_shutdown( } pub fn on_code_lens_request( - _state: &mut LspState, + state: &mut LspState, params: CodeLensParams, ) -> impl Future>, ResponseError>> { let actual_path = params.text_document.uri.to_file_path().unwrap(); - let (mut driver, current_crate_id) = create_context_at_path(actual_path); + let (mut context, crate_id) = match create_context_at_path(&state.root_path, &actual_path) { + Err(err) => return future::ready(Err(err)), + Ok(res) => res, + }; // We ignore the warnings and errors produced by compilation for producing codelenses // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut driver, current_crate_id, false, false); + let _ = check_crate(&mut context, crate_id, false, false); - let fm = &driver.file_manager; + let fm = &context.file_manager; let files = fm.as_simple_files(); - let tests = driver.get_all_test_functions_in_crate_matching(¤t_crate_id, ""); + let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); let mut lenses: Vec = vec![]; for func_id in tests { - let location = driver.function_meta(&func_id).name.location; + let location = context.function_meta(&func_id).name.location; let file_id = location.file; // TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file if file_id.as_usize() != 0 { continue; } - let func_name = driver.function_name(&func_id); + let func_name = context.function_name(&func_id); let range = byte_span_to_range(files, file_id.as_usize(), location.span.into()).unwrap_or_default(); @@ -109,13 +118,9 @@ pub fn on_code_lens_request( lenses.push(lens); } - async move { - if lenses.is_empty() { - Ok(None) - } else { - Ok(Some(lenses)) - } - } + let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; + + future::ready(res) } pub fn on_initialized( @@ -197,7 +202,11 @@ pub fn on_did_save_text_document( params: DidSaveTextDocumentParams, ) -> ControlFlow> { let actual_path = params.text_document.uri.to_file_path().unwrap(); - let (mut context, crate_id) = create_context_at_path(actual_path.clone()); + let actual_file_name = actual_path.file_name(); + let (mut context, crate_id) = match create_context_at_path(&state.root_path, &actual_path) { + Err(err) => return ControlFlow::Break(Err(err.into())), + Ok(res) => res, + }; let file_diagnostics = match check_crate(&mut context, crate_id, false, false) { Ok(warnings) => warnings, @@ -213,13 +222,13 @@ pub fn on_did_save_text_document( // TODO(AD): HACK, undo these total hacks once we have a proper approach if file_id.as_usize() == 0 { // main.nr case - if actual_path.file_name().unwrap().to_str() != Some("main.nr") - && actual_path.file_name().unwrap().to_str() != Some("lib.nr") + if actual_file_name.unwrap().to_str() != Some("main.nr") + && actual_file_name.unwrap().to_str() != Some("lib.nr") { continue; } } else if fm.path(file_id).file_name().unwrap().to_str().unwrap() - != actual_path.file_name().unwrap().to_str().unwrap().replace(".nr", "") + != actual_file_name.unwrap().to_str().unwrap().replace(".nr", "") { // every other file case continue; // TODO(AD): HACK, we list all errors, filter by hacky final path component @@ -256,19 +265,30 @@ pub fn on_did_save_text_document( ControlFlow::Continue(()) } -fn create_context_at_path(actual_path: PathBuf) -> (Context, CrateId) { - // 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 context = Context::default(); +fn create_context_at_path( + root_path: &Option, + actual_path: &Path, +) -> Result<(Context, CrateId), ResponseError> { + let mut context = match &root_path { + Some(root_path) => { + let fm = FileManager::new(root_path); + let graph = CrateGraph::default(); + Context::new(fm, graph) + } + None => { + let err = ResponseError::new(ErrorCode::REQUEST_FAILED, "Project has not been built"); + return Err(err); + } + }; - let mut file_path: PathBuf = actual_path; + let mut file_path = actual_path.to_path_buf(); // TODO better naming/unhacking if let Some(new_path) = find_nearest_parent_file(&file_path, &["lib.nr", "main.nr"]) { file_path = new_path; // TODO unhack } let nargo_toml_path = find_nearest_parent_file(&file_path, &["Nargo.toml"]); - let current_crate_id = create_local_crate(&mut context, file_path, CrateType::Binary); + let current_crate_id = create_local_crate(&mut context, &file_path, CrateType::Binary); // TODO(AD): undo hacky dependency resolution if let Some(nargo_toml_path) = nargo_toml_path { @@ -280,12 +300,12 @@ fn create_context_at_path(actual_path: PathBuf) -> (Context, CrateId) { .unwrap() // TODO .join(PathBuf::from(&dependency_path).join("src").join("lib.nr")); let library_crate = - create_non_local_crate(&mut context, path_to_lib, CrateType::Library); + create_non_local_crate(&mut context, &path_to_lib, CrateType::Library); propagate_dep(&mut context, library_crate, &CrateName::new(crate_name).unwrap()); } } } - (context, current_crate_id) + Ok((context, current_crate_id)) } pub fn on_exit(_state: &mut LspState, _params: ()) -> ControlFlow> { diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index 2edf36b1166..d952280f98c 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -88,9 +88,13 @@ pub fn start_cli() -> eyre::Result<()> { // FIXME: I not sure that this is the right place for this tests. #[cfg(test)] mod tests { + use fm::FileManager; use noirc_driver::{check_crate, create_local_crate}; use noirc_errors::reporter; - use noirc_frontend::{graph::CrateType, hir::Context}; + use noirc_frontend::{ + graph::{CrateGraph, CrateType}, + hir::Context, + }; use std::path::{Path, PathBuf}; @@ -99,9 +103,11 @@ mod tests { /// Compiles a file and returns true if compilation was successful /// /// This is used for tests. - fn file_compiles>(root_file: P) -> bool { - let mut context = Context::default(); - let crate_id = create_local_crate(&mut context, &root_file, CrateType::Binary); + fn file_compiles(root_dir: &Path, root_file: &Path) -> bool { + let fm = FileManager::new(root_dir); + let graph = CrateGraph::default(); + let mut context = Context::new(fm, graph); + let crate_id = create_local_crate(&mut context, root_file, CrateType::Binary); let result = check_crate(&mut context, crate_id, false, false); let success = result.is_ok(); @@ -120,10 +126,10 @@ mod tests { let pass_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(format!("{TEST_DATA_DIR}/pass")); - let paths = std::fs::read_dir(pass_dir).unwrap(); + let paths = std::fs::read_dir(&pass_dir).unwrap(); for path in paths.flatten() { let path = path.path(); - assert!(file_compiles(&path), "path: {}", path.display()); + assert!(file_compiles(&pass_dir, &path), "path: {}", path.display()); } } @@ -132,10 +138,10 @@ mod tests { let fail_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(format!("{TEST_DATA_DIR}/fail")); - let paths = std::fs::read_dir(fail_dir).unwrap(); + let paths = std::fs::read_dir(&fail_dir).unwrap(); for path in paths.flatten() { let path = path.path(); - assert!(!file_compiles(&path), "path: {}", path.display()); + assert!(!file_compiles(&fail_dir, &path), "path: {}", path.display()); } } } diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index 9598031b193..63e4cb0ae73 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -3,10 +3,11 @@ use std::{ path::{Path, PathBuf}, }; +use fm::FileManager; use nargo::manifest::{Dependency, Manifest, PackageManifest, WorkspaceConfig}; use noirc_driver::{add_dep, create_local_crate, create_non_local_crate}; use noirc_frontend::{ - graph::{CrateId, CrateName, CrateType}, + graph::{CrateGraph, CrateId, CrateName, CrateType}, hir::Context, }; use thiserror::Error; @@ -88,7 +89,9 @@ pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, package: Option, ) -> Result<(Context, CrateId), DependencyResolutionError> { - let mut context = Context::default(); + let fm = FileManager::new(dir_path); + let graph = CrateGraph::default(); + let mut context = Context::new(fm, graph); let manifest_path = super::find_package_manifest(dir_path)?; let manifest = super::manifest::parse(&manifest_path)?; @@ -97,7 +100,7 @@ pub(crate) fn resolve_root_manifest( Manifest::Package(package) => { let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; - let crate_id = create_local_crate(&mut context, entry_path, crate_type); + let crate_id = create_local_crate(&mut context, &entry_path, crate_type); let pkg_root = manifest_path.parent().expect("Every manifest path has a parent."); resolve_package_manifest(&mut context, crate_id, package, pkg_root)?; @@ -222,11 +225,11 @@ fn resolve_workspace_manifest( .ok_or_else(|| DependencyResolutionError::PackageNotFound(crate_name(local_package)))?; let (entry_path, _crate_type) = super::lib_or_bin(local_crate)?; - let crate_id = create_local_crate(context, entry_path, CrateType::Workspace); + let crate_id = create_local_crate(context, &entry_path, CrateType::Workspace); for (_, package_path) in packages.drain() { let (entry_path, crate_type) = super::lib_or_bin(package_path)?; - create_non_local_crate(context, entry_path, crate_type); + create_non_local_crate(context, &entry_path, crate_type); } Ok(crate_id) diff --git a/crates/nargo_cli/tests/test_data/let_stmt/Nargo.toml b/crates/nargo_cli/tests/test_data/let_stmt/Nargo.toml new file mode 100644 index 00000000000..dc0c2f8917c --- /dev/null +++ b/crates/nargo_cli/tests/test_data/let_stmt/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.8.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data/let_stmt/Prover.toml b/crates/nargo_cli/tests/test_data/let_stmt/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/crates/nargo_cli/tests/test_data/let_stmt/src/main.nr b/crates/nargo_cli/tests/test_data/let_stmt/src/main.nr new file mode 100644 index 00000000000..d24a77851e4 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/let_stmt/src/main.nr @@ -0,0 +1,10 @@ +struct Foo { + a: u64, +} + +fn main() { + let _ = 42; + let _ = 42; + + let Foo { a: _ } = Foo { a: 42 }; +} diff --git a/crates/nargo_cli/tests/test_data/struct/src/main.nr b/crates/nargo_cli/tests/test_data/struct/src/main.nr index 6d61393920d..a6d3eddd8d5 100644 --- a/crates/nargo_cli/tests/test_data/struct/src/main.nr +++ b/crates/nargo_cli/tests/test_data/struct/src/main.nr @@ -74,4 +74,6 @@ fn main(x: Field, y: Field) { let six = legs + eyes as Field; assert(six == 6); + + let Animal { legs: _, eyes: _ } = get_dog(); } diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Prover.toml new file mode 100644 index 00000000000..baad8be126a --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Prover.toml @@ -0,0 +1,38 @@ +c=[2, 4, 3, 0, ] +a=0 +x = [104, 101, 108, 108, 111] + +result = [ + 0x2c, + 0xf2, + 0x4d, + 0xba, + 0x5f, + 0xb0, + 0xa3, + 0x0e, + 0x26, + 0xe8, + 0x3b, + 0x2a, + 0xc5, + 0xb9, + 0xe2, + 0x9e, + 0x1b, + 0x16, + 0x1e, + 0x5c, + 0x1f, + 0xa7, + 0x42, + 0x5e, + 0x73, + 0x04, + 0x33, + 0x62, + 0x93, + 0x8b, + 0x98, + 0x24, +] diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/src/main.nr new file mode 100644 index 00000000000..48ac639ecf0 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/src/main.nr @@ -0,0 +1,277 @@ +use dep::std; + +fn sort(mut a: [u32; 4]) -> [u32; 4] { + for i in 1..4 { + for j in 0..i { + if a[i] < a[j] { + let c = a[j]; + a[j] = a[i]; + a[i] = c; + } + } + } + a +} + +fn call_intrinsic(x: [u8; 5], result: [u8; 32]) { + let mut digest = std::hash::sha256(x); + digest[0] = 5 as u8; + digest = std::hash::sha256(x); + assert(digest == result); +} + +fn must_be_zero(x: u8) { + assert(x == 0); +} + +fn test3 (x: u8) { + if x == 0 { + must_be_zero(x); + } +} + +fn test4() -> [u32; 4] { + let b: [u32; 4] = [1,2,3,4]; + b +} + +fn main(a: u32, mut c: [u32; 4], x: [u8; 5], result: pub [u8; 32]){ + // Regression test for issue #547 + // Warning: it must be kept at the start of main + let arr: [u8; 2] = [1, 2]; + if arr[0] != arr[1] { + for i in 0..1 { + assert(i != 2); + } + } + + //Issue reported in #421 + if a == c[0] { + assert(c[0] == 0); + } else { + if a == c[1] { + assert(c[1] == 0); + } else { + if a == c[2] { + assert(c[2] == 0); + } + } + } + + //Regression for to_le_bits() constant evaluation + // binary array representation of u8 1 + let as_bits_hardcode_1 = [1, 0]; + let mut c1 = 0; + for i in 0..2 { + let mut as_bits = (arr[i] as Field).to_le_bits(2); + c1 = c1 + as_bits[0] as Field; + + if i == 0 { + assert(arr[i] == 1);// 1 + for k in 0..2 { + assert(as_bits_hardcode_1[k] == as_bits[k]); + } + } + if i == 1 { + assert(arr[i] == 2);//2 + for k in 0..2 { + assert(as_bits_hardcode_1[k] != as_bits[k]); + } + } + } + assert(c1==1); + + //Regression for Issue #579 + let result1_true = test(true); + assert(result1_true.array_param[0] == 1); + let result1_false = test(false); + assert(result1_false.array_param[0] == 0); + + //Test case for short-circuit + let mut data = [0 as u32; 32]; + let mut ba = a; + for i in 0..32 { + let i_u32 = i as u32; + if i_u32 == a { + for j in 0..4 { + data[i + j] = c[4 - 1 - j]; + for k in 0..4 { + ba = ba +data[k]; + } + if ba == 4864 { + c[3]=ba; + } + } + } + } + assert(data[31] == 0); + assert(ba != 13); + //regression for short-circuit2 + if 35 == a { + assert(false); + } + bar(a as Field); + + if a == 3 { + c = test4(); + } + assert(c[1] != 2); + call_intrinsic(x, result); + + //Test case for conditional with arrays from function parameters + let b = sort([1,2,3,4]); + assert(b[0] == 1); + + if a == 0 { + must_be_zero(0); + c[0] = 3; + } else { + must_be_zero(1); + c[0] = 1; + c[1] = c[2] / a + 11 % a; + let f1 = a as Field; + assert(10/f1 != 0); + } + assert(c[0] == 3); + + let mut y = 0; + if a == 0 { + let digest = std::hash::sha256(x); + y = digest[0]; + } else { + y = 5; + } + assert(y == result[0]); + c = sort(c); + assert(c[0]==0); + + //test 1 + let mut x: u32 = 0; + if a == 0 { + c[0] = 12; + if a != 0 { + x = 6; + } else { + x = 2; + assert(x == 2); + } + } else { + x = 5; + assert(x == 5); + } + if c[0] == 0 { + x = 3; + } + assert(x == 2); + + //test2: loops! + x = 0; + x = a - a; + for i in 0..4 { + if c[i] == 0 { + x = i as u32 +2; + } + } + assert(x == 0); + + test3(1); + + if a == 0 { + c = test4(); + } else { + assert(c[1] != 2); + } + if false { + c[1] = 5; + } + assert(c[1] == 2); + + test5(4); + + // Regression for issue #661: + let mut c_661 :[u32;1]=[0]; + if a > 5 { + c_661 = issue_661_foo(issue_661_bar(c), a); + } else { + c_661 = issue_661_foo(issue_661_bar(c), x); + } + assert(c_661[0] < 20000); + + // Test case for function synchronisation + let mut c_sync = 0; + if a == 42 { + c_sync = foo2(); + } else { + c_sync = foo2() + foo2(); + } + assert(c_sync == 6); + + // Regression for predicate simplification + safe_inverse(0); +} + +fn test5(a : u32) { + if a > 1 { + let q = a / 2; + assert(q == 2); + } +} + + + +fn foo() { + let mut x = 1; + x /= 0; +} + +fn bar(x:Field) { + if x == 15 { + foo(); + } +} + + +struct MyStruct579 { + array_param: [u32; 2] +} + +impl MyStruct579 { + fn new(array_param: [u32; 2]) -> MyStruct579 { + MyStruct579 { + array_param: array_param + } + } +} + +fn test(flag: bool) -> MyStruct579 { + let mut my_struct = MyStruct579::new([0; 2]); + + if flag == true { + my_struct= MyStruct579::new([1; 2]); + } + my_struct +} + +fn issue_661_foo(array: [u32;4], b:u32) ->[u32;1] { + [array[0]+b] +} + +fn issue_661_bar(a : [u32;4]) ->[u32;4] { + let mut b:[u32;4] = [0;4]; + b[0]=a[0]+1; + b +} + +fn foo2() -> Field { + 3 +} + +fn safe_inverse(n: Field) -> Field +{ + if n == 0 { + 0 + } + else { + 1 / n + } +} diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr index 795fc02c35f..fed84f80545 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr @@ -5,6 +5,19 @@ fn main(x: u32) { assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); + multiple_values_entry_point(x); +} + +unconstrained fn returns_multiple_values(x : u32) -> (u32, u32, u32, u32) { + (x + 1, x + 2, x + 3, x + 4) +} + +unconstrained fn multiple_values_entry_point(x: u32) { + let (a, b, c, d) = returns_multiple_values(x); + assert(a == x + 1); + assert(b == x + 2); + assert(c == x + 3); + assert(d == x + 4); } unconstrained fn inner(x : u32) -> u32 { diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_fns_as_values/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_fns_as_values/src/main.nr index 5af542301ec..bae24b8c4b1 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_fns_as_values/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_fns_as_values/src/main.nr @@ -4,9 +4,12 @@ struct MyStruct { fn main(x: u32) { assert(wrapper(increment, x) == x + 1); + assert(wrapper(increment_acir, x) == x + 1); assert(wrapper(decrement, x) == x - 1); assert(wrapper_with_struct(MyStruct { operation: increment }, x) == x + 1); assert(wrapper_with_struct(MyStruct { operation: decrement }, x) == x - 1); + // https://github.com/noir-lang/noir/issues/1975 + assert(increment(x) == x + 1); } unconstrained fn wrapper(func: fn (u32) -> u32, param: u32) -> u32 { @@ -26,3 +29,8 @@ unconstrained fn wrapper_with_struct(my_struct: MyStruct, param: u32) -> u32 { func(param) } + + +fn increment_acir(x: u32) -> u32 { + x + 1 +} \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_loop/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_loop/src/main.nr index 7240264b2a8..e469248239e 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_loop/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_loop/src/main.nr @@ -3,6 +3,7 @@ // The features being tested is basic looping on brillig fn main(sum: u32){ assert(loop(4) == sum); + assert(plain_loop() == sum); } unconstrained fn loop(x: u32) -> u32 { @@ -12,3 +13,11 @@ unconstrained fn loop(x: u32) -> u32 { } sum } + +unconstrained fn plain_loop() -> u32 { + let mut sum = 0; + for i in 0..4 { + sum = sum + i; + } + sum +} diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/Nargo.toml new file mode 100644 index 00000000000..dc0c2f8917c --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.8.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/src/main.nr new file mode 100644 index 00000000000..d24a77851e4 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/let_stmt/src/main.nr @@ -0,0 +1,10 @@ +struct Foo { + a: u64, +} + +fn main() { + let _ = 42; + let _ = 42; + + let Foo { a: _ } = Foo { a: 42 }; +} diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/struct/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/struct/src/main.nr index 6d61393920d..a6d3eddd8d5 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/struct/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/struct/src/main.nr @@ -74,4 +74,6 @@ fn main(x: Field, y: Field) { let six = legs + eyes as Field; assert(six == 6); + + let Animal { legs: _, eyes: _ } = get_dog(); } diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/unit/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/unit/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/unit/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/unit/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/unit/src/main.nr new file mode 100644 index 00000000000..2cb1f7d7c66 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/unit/src/main.nr @@ -0,0 +1,14 @@ +fn main() { + let _a = (); + let _b: () = _a; + let _c: () = (); + let _d = f1(); + let _e: () = f2(); + let _f: () = f3(); + let _g = f4(); +} + +fn f1() {} +fn f2() { () } +fn f3() -> () {} +fn f4() -> () { () } diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 525c15af1e8..05e4cfd28e3 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -14,7 +14,7 @@ use noirc_frontend::hir::Context; use noirc_frontend::monomorphization::monomorphize; use noirc_frontend::node_interner::FuncId; use serde::{Deserialize, Serialize}; -use std::path::{Path, PathBuf}; +use std::path::Path; mod contract; mod program; @@ -67,7 +67,7 @@ pub type ErrorsAndWarnings = Vec; // with the restricted version which only uses one file pub fn compile_file( context: &mut Context, - root_file: PathBuf, + root_file: &Path, ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> { let crate_id = create_local_crate(context, root_file, CrateType::Binary); compile_main(context, crate_id, &CompileOptions::default()) @@ -79,26 +79,24 @@ pub fn compile_file( /// we have multiple binaries in one workspace /// A Fix would be for the driver instance to store the local crate id. // Granted that this is the only place which relies on the local crate being first -pub fn create_local_crate>( +pub fn create_local_crate( context: &mut Context, - root_file: P, + file_name: &Path, crate_type: CrateType, ) -> CrateId { - let dir_path = root_file.as_ref().to_path_buf(); - let root_file_id = context.file_manager.add_file(&dir_path).unwrap(); + let root_file_id = context.file_manager.add_file(file_name).unwrap(); context.crate_graph.add_crate_root(crate_type, root_file_id) } /// Creates a Non Local Crate. A Non Local Crate is any crate which is the not the crate that /// the compiler is compiling. -pub fn create_non_local_crate>( +pub fn create_non_local_crate( context: &mut Context, - root_file: P, + file_name: &Path, crate_type: CrateType, ) -> CrateId { - let dir_path = root_file.as_ref().to_path_buf(); - let root_file_id = context.file_manager.add_file(&dir_path).unwrap(); + let root_file_id = context.file_manager.add_file(file_name).unwrap(); // You can add any crate type to the crate graph // but you cannot depend on Binaries @@ -153,7 +151,7 @@ pub fn check_crate( // however, the `create_non_local_crate` panics if you add the stdlib as the first crate in the graph and other // parts of the code expect the `0` FileID to be the crate root. See also #1681 let std_crate_name = "std"; - let path_to_std_lib_file = PathBuf::from(std_crate_name).join("lib.nr"); + let path_to_std_lib_file = Path::new(std_crate_name).join("lib.nr"); let root_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); // You can add any crate type to the crate graph diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index e9215b81a2f..3beae3db64b 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1,7 +1,9 @@ use crate::brillig::brillig_gen::brillig_slice_ops::{ convert_array_or_vector_to_vector, slice_push_back_operation, }; -use crate::brillig::brillig_ir::{BrilligBinaryOp, BrilligContext}; +use crate::brillig::brillig_ir::{ + BrilligBinaryOp, BrilligContext, BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE, +}; use crate::ssa_refactor::ir::function::FunctionId; use crate::ssa_refactor::ir::instruction::Intrinsic; use crate::ssa_refactor::ir::{ @@ -285,45 +287,7 @@ impl<'block> BrilligBlock<'block> { } } Value::Function(func_id) => { - let argument_registers: Vec = arguments - .iter() - .flat_map(|arg| { - let arg = self.convert_ssa_value(*arg, dfg); - self.function_context.extract_registers(arg) - }) - .collect(); - let result_ids = dfg.instruction_results(instruction_id); - - // Create label for the function that will be called - let label_of_function_to_call = - FunctionContext::function_id_to_function_label(*func_id); - - let saved_registers = - self.brillig_context.pre_call_save_registers_prep_args(&argument_registers); - - // Call instruction, which will interpret above registers 0..num args - self.brillig_context.add_external_call_instruction(label_of_function_to_call); - - // Important: resolve after pre_call_save_registers_prep_args - // This ensures we don't save the results to registers unnecessarily. - let result_registers: Vec = result_ids - .iter() - .flat_map(|arg| { - let arg = self.function_context.create_variable( - self.brillig_context, - *arg, - dfg, - ); - self.function_context.extract_registers(arg) - }) - .collect(); - - assert!( - !saved_registers.iter().any(|x| result_registers.contains(x)), - "should not save registers used as function results" - ); - self.brillig_context - .post_call_prep_returns_load_registers(&result_registers, &saved_registers); + self.convert_ssa_function_call(*func_id, arguments, dfg, instruction_id); } Value::Intrinsic(Intrinsic::BlackBox(bb_func)) => { let function_arguments = @@ -430,6 +394,59 @@ impl<'block> BrilligBlock<'block> { }; } + fn convert_ssa_function_call( + &mut self, + func_id: FunctionId, + arguments: &[ValueId], + dfg: &DataFlowGraph, + instruction_id: InstructionId, + ) { + // Convert the arguments to registers casting those to the types of the receiving function + let argument_registers: Vec = arguments + .iter() + .flat_map(|argument_id| { + let variable_to_pass = self.convert_ssa_value(*argument_id, dfg); + self.function_context.extract_registers(variable_to_pass) + }) + .collect(); + + let result_ids = dfg.instruction_results(instruction_id); + + // Create label for the function that will be called + let label_of_function_to_call = FunctionContext::function_id_to_function_label(func_id); + + let saved_registers = + self.brillig_context.pre_call_save_registers_prep_args(&argument_registers); + + // Call instruction, which will interpret above registers 0..num args + self.brillig_context.add_external_call_instruction(label_of_function_to_call); + + // Important: resolve after pre_call_save_registers_prep_args + // This ensures we don't save the results to registers unnecessarily. + + // Allocate the registers for the variables where we are assigning the returns + let variables_assigned_to = vecmap(result_ids, |result_id| { + self.function_context.create_variable(self.brillig_context, *result_id, dfg) + }); + + // Collect the registers that should have been returned + let returned_registers: Vec = variables_assigned_to + .iter() + .flat_map(|returned_variable| { + self.function_context.extract_registers(*returned_variable) + }) + .collect(); + + assert!( + !saved_registers.iter().any(|x| returned_registers.contains(x)), + "should not save registers used as function results" + ); + + // puts the returns into the returned_registers and restores saved_registers + self.brillig_context + .post_call_prep_returns_load_registers(&returned_registers, &saved_registers); + } + /// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice /// With a specific value changed. fn convert_ssa_array_set( @@ -697,11 +714,28 @@ impl<'block> BrilligBlock<'block> { let binary_type = type_of_binary_operation(dfg[binary.lhs].get_type(), dfg[binary.rhs].get_type()); - let left = self.convert_ssa_register_value(binary.lhs, dfg); - let right = self.convert_ssa_register_value(binary.rhs, dfg); + let mut left = self.convert_ssa_register_value(binary.lhs, dfg); + let mut right = self.convert_ssa_register_value(binary.rhs, dfg); let brillig_binary_op = - convert_ssa_binary_op_to_brillig_binary_op(binary.operator, binary_type); + convert_ssa_binary_op_to_brillig_binary_op(binary.operator, &binary_type); + + // Some binary operations with fields are issued by the compiler, such as loop comparisons, cast those to the bit size here + // TODO Remove after fixing https://github.com/noir-lang/noir/issues/1979 + if let ( + BrilligBinaryOp::Integer { bit_size, .. }, + Type::Numeric(NumericType::NativeField), + ) = (&brillig_binary_op, &binary_type) + { + let new_lhs = self.brillig_context.allocate_register(); + let new_rhs = self.brillig_context.allocate_register(); + + self.brillig_context.cast_instruction(new_lhs, left, *bit_size); + self.brillig_context.cast_instruction(new_rhs, right, *bit_size); + + left = new_lhs; + right = new_rhs; + } self.brillig_context.binary_instruction(left, right, result_register, brillig_binary_op); } @@ -830,7 +864,7 @@ pub(crate) fn type_of_binary_operation(lhs_type: Type, rhs_type: Type) -> Type { /// - Brillig Binary Field Op, if it is a field type pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op( ssa_op: BinaryOp, - typ: Type, + typ: &Type, ) -> BrilligBinaryOp { // First get the bit size and whether its a signed integer, if it is a numeric type // if it is not,then we return None, indicating that @@ -845,18 +879,20 @@ pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op( }; fn binary_op_to_field_op(op: BinaryOp) -> BrilligBinaryOp { - let operation = match op { - BinaryOp::Add => BinaryFieldOp::Add, - BinaryOp::Sub => BinaryFieldOp::Sub, - BinaryOp::Mul => BinaryFieldOp::Mul, - BinaryOp::Div => BinaryFieldOp::Div, - BinaryOp::Eq => BinaryFieldOp::Equals, + match op { + BinaryOp::Add => BrilligBinaryOp::Field { op: BinaryFieldOp::Add }, + BinaryOp::Sub => BrilligBinaryOp::Field { op: BinaryFieldOp::Sub }, + BinaryOp::Mul => BrilligBinaryOp::Field { op: BinaryFieldOp::Mul }, + BinaryOp::Div => BrilligBinaryOp::Field { op: BinaryFieldOp::Div }, + BinaryOp::Eq => BrilligBinaryOp::Field { op: BinaryFieldOp::Equals }, + BinaryOp::Lt => BrilligBinaryOp::Integer { + op: BinaryIntOp::LessThan, + bit_size: BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE, + }, _ => unreachable!( "Field type cannot be used with {op}. This should have been caught by the frontend" ), - }; - - BrilligBinaryOp::Field { op: operation } + } } fn binary_op_to_int_op(op: BinaryOp, bit_size: u32, is_signed: bool) -> BrilligBinaryOp { @@ -888,7 +924,7 @@ pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op( // If bit size is available then it is a binary integer operation match bit_size_signedness { - Some((bit_size, is_signed)) => binary_op_to_int_op(ssa_op, bit_size, is_signed), + Some((bit_size, is_signed)) => binary_op_to_int_op(ssa_op, *bit_size, is_signed), None => binary_op_to_field_op(ssa_op), } } diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir.rs b/crates/noirc_evaluator/src/brillig/brillig_ir.rs index e662b7d2ea6..be9279111b9 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir.rs @@ -394,6 +394,7 @@ impl BrilligContext { .collect(); for (new_source, destination) in new_sources.iter().zip(destinations.iter()) { self.mov_instruction(*destination, *new_source); + self.deallocate_register(*new_source); } } @@ -821,9 +822,12 @@ impl BrilligContext { ) { // Allocate our result registers and write into them // We assume the return values of our call are held in 0..num results register indices - for (i, result_register) in result_registers.iter().enumerate() { - self.mov_instruction(*result_register, self.register(i)); - } + let (sources, destinations) = result_registers + .iter() + .enumerate() + .map(|(i, result_register)| (self.register(i), *result_register)) + .unzip(); + self.mov_registers_to_registers_instruction(sources, destinations); // Restore all the same registers we have, in exact reverse order. // Note that we have allocated some registers above, which we will not be handling here, diff --git a/crates/noirc_evaluator/src/ssa/ssa_gen.rs b/crates/noirc_evaluator/src/ssa/ssa_gen.rs index 11fc134215d..339fce7ea9d 100644 --- a/crates/noirc_evaluator/src/ssa/ssa_gen.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_gen.rs @@ -738,7 +738,7 @@ impl IrGenerator { //Parse a block of AST statements into ssa form fn ssa_gen_block(&mut self, block: &[Expression]) -> Result { - let mut last_value = Value::dummy(); + let mut last_value = Value::Node(self.context.zero()); for expr in block { last_value = self.ssa_gen_expression(expr)?; } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index a1fb66ab749..61c3386ccbd 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -156,7 +156,11 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the inverse of `var`. - pub(crate) fn inv_var(&mut self, var: AcirVar) -> Result { + pub(crate) fn inv_var( + &mut self, + var: AcirVar, + predicate: AcirVar, + ) -> Result { let var_data = &self.vars[&var]; if let AcirVarData::Const(constant) = var_data { // Note that this will return a 0 if the inverse is not available @@ -169,7 +173,7 @@ impl AcirContext { let field_type = AcirType::NumericType(NumericType::NativeField); let results = self.brillig( - None, + Some(predicate), inverse_code, vec![AcirValue::Var(var, field_type.clone())], vec![field_type], @@ -177,15 +181,28 @@ impl AcirContext { let inverted_var = Self::expect_one_var(results); let should_be_one = self.mul_var(inverted_var, var)?; - self.assert_eq_one(should_be_one)?; + self.maybe_eq_predicate(should_be_one, predicate)?; Ok(inverted_var) } - /// Constrains the lhs to be equal to the constant value `1` + // Constrains `var` to be equal to the constant value `1` pub(crate) fn assert_eq_one(&mut self, var: AcirVar) -> Result<(), AcirGenError> { - let one_var = self.add_constant(FieldElement::one()); - self.assert_eq_var(var, one_var) + let one = self.add_constant(FieldElement::one()); + self.assert_eq_var(var, one) + } + + // Constrains `var` to be equal to predicate if the predicate is true + // or to be equal to 0 if the predicate is false. + // + // Since we multiply `var` by the predicate, this is a no-op if the predicate is false + pub(crate) fn maybe_eq_predicate( + &mut self, + var: AcirVar, + predicate: AcirVar, + ) -> Result<(), AcirGenError> { + let pred_mul_var = self.mul_var(var, predicate)?; + self.assert_eq_var(pred_mul_var, predicate) } // Returns the variable from the results, assuming it is the only result @@ -296,6 +313,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, + predicate: AcirVar, ) -> Result { let numeric_type = match typ { AcirType::NumericType(numeric_type) => numeric_type, @@ -305,12 +323,12 @@ impl AcirContext { }; match numeric_type { NumericType::NativeField => { - let inv_rhs = self.inv_var(rhs)?; + let inv_rhs = self.inv_var(rhs, predicate)?; self.mul_var(lhs, inv_rhs) } NumericType::Unsigned { bit_size } => { let (quotient_var, _remainder_var) = - self.euclidean_division_var(lhs, rhs, bit_size)?; + self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(quotient_var) } NumericType::Signed { bit_size } => { @@ -430,17 +448,18 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, + predicate: AcirVar, ) -> Result<(AcirVar, AcirVar), AcirGenError> { - let predicate = Expression::one(); - let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; + let predicate_data = &self.vars[&predicate]; let lhs_expr = lhs_data.to_expression(); let rhs_expr = rhs_data.to_expression(); + let predicate_expr = predicate_data.to_expression(); let (quotient, remainder) = - self.acir_ir.euclidean_division(&lhs_expr, &rhs_expr, bit_size, &predicate)?; + self.acir_ir.euclidean_division(&lhs_expr, &rhs_expr, bit_size, &predicate_expr)?; let quotient_var = self.add_data(AcirVarData::Witness(quotient)); let remainder_var = self.add_data(AcirVarData::Witness(remainder)); @@ -485,8 +504,9 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, + predicate: AcirVar, ) -> Result { - let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size)?; + let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(remainder) } @@ -505,6 +525,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, + predicate: AcirVar, ) -> Result { let rhs_data = &self.vars[&rhs]; @@ -515,7 +536,7 @@ impl AcirContext { }; let two_pow_rhs_var = self.add_constant(two_pow_rhs); - self.div_var(lhs, two_pow_rhs_var, typ) + self.div_var(lhs, two_pow_rhs_var, typ, predicate) } /// Converts the `AcirVar` to a `Witness` if it hasn't been already, and appends it to the @@ -575,7 +596,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - predicate: Option, + predicate: AcirVar, ) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; @@ -586,10 +607,9 @@ impl AcirContext { // TODO: check what happens when we do (a as u8) >= (b as u32) // TODO: The frontend should shout in this case - let predicate = predicate.map(|acir_var| { - let predicate_data = &self.vars[&acir_var]; - predicate_data.to_expression().into_owned() - }); + let predicate_data = &self.vars[&predicate]; + let predicate = predicate_data.to_expression().into_owned(); + let is_greater_than_eq = self.acir_ir.more_than_eq_comparison(&lhs_expr, &rhs_expr, bit_size, predicate)?; @@ -607,7 +627,7 @@ impl AcirContext { ) -> Result { // Flip the result of calling more than equal method to // compute less than. - let comparison = self.more_than_eq_var(lhs, rhs, bit_size, Some(predicate))?; + let comparison = self.more_than_eq_var(lhs, rhs, bit_size, predicate)?; let one = self.add_constant(FieldElement::one()); self.sub_var(one, comparison) // comparison_negated @@ -858,6 +878,7 @@ impl AcirContext { &mut self, inputs: Vec, bit_size: u32, + predicate: AcirVar, ) -> Result, AcirGenError> { let len = inputs.len(); // Convert the inputs into expressions @@ -875,7 +896,7 @@ impl AcirContext { // Enforce the outputs to be sorted for i in 0..(outputs_var.len() - 1) { - self.less_than_constrain(outputs_var[i], outputs_var[i + 1], bit_size, None)?; + self.less_than_constrain(outputs_var[i], outputs_var[i + 1], bit_size, predicate)?; } Ok(outputs_var) @@ -887,10 +908,10 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - predicate: Option, + predicate: AcirVar, ) -> Result<(), AcirGenError> { let lhs_less_than_rhs = self.more_than_eq_var(rhs, lhs, bit_size, predicate)?; - self.assert_eq_one(lhs_less_than_rhs) + self.maybe_eq_predicate(lhs_less_than_rhs, predicate) } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 16e07c29730..5b05b1a0c1d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -428,11 +428,22 @@ impl GeneratedAcir { // When the predicate is 0, the equation always passes. // When the predicate is 1, the euclidean division needs to be // true. - let mut rhs_constraint = (rhs * &Expression::from(q_witness)).unwrap(); + let rhs_reduced: Expression = self.create_witness_for_expression(rhs).into(); + let mut rhs_constraint = (&rhs_reduced * &Expression::from(q_witness)) + .expect("rhs_reduced is expected to be a degree-1 witness"); rhs_constraint = &rhs_constraint + r_witness; - rhs_constraint = (&rhs_constraint * predicate).unwrap(); - let lhs_constraint = (lhs * predicate).unwrap(); - let div_euclidean = &lhs_constraint - &rhs_constraint; + + // Reduce the rhs_constraint to a witness + let rhs_constrain_reduced: Expression = + self.create_witness_for_expression(&rhs_constraint).into(); + // Reduce the lhs_constraint to a witness + let lhs_reduced: Expression = self.create_witness_for_expression(lhs).into(); + + let div_euclidean = &(&lhs_reduced * predicate).expect( + "lhs_reduced should be a degree-1 witness and predicate should be a degree-1 witness", + ) - &(&rhs_constrain_reduced * predicate).expect( + "rhs_reduced should be a degree-1 witness and predicate should be a degree-1 witness", + ); self.push_opcode(AcirOpcode::Arithmetic(div_euclidean)); @@ -711,7 +722,7 @@ impl GeneratedAcir { a: &Expression, b: &Expression, max_bits: u32, - predicate: Option, + predicate: Expression, ) -> Result { // Ensure that 2^{max_bits + 1} is less than the field size // @@ -737,7 +748,7 @@ impl GeneratedAcir { let (q_witness, r_witness) = self.quotient_directive( comparison_evaluation.clone(), two_max_bits.into(), - predicate, + Some(predicate), q_max_bits, r_max_bits, )?; diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 72194828f5c..0e665466d42 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -548,7 +548,12 @@ impl Context { BinaryOp::Add => self.acir_context.add_var(lhs, rhs), BinaryOp::Sub => self.acir_context.sub_var(lhs, rhs), BinaryOp::Mul => self.acir_context.mul_var(lhs, rhs), - BinaryOp::Div => self.acir_context.div_var(lhs, rhs, binary_type), + BinaryOp::Div => self.acir_context.div_var( + lhs, + rhs, + binary_type, + self.current_side_effects_enabled_var, + ), // Note: that this produces unnecessary constraints when // this Eq instruction is being used for a constrain statement BinaryOp::Eq => self.acir_context.eq_var(lhs, rhs), @@ -559,11 +564,21 @@ impl Context { self.current_side_effects_enabled_var, ), BinaryOp::Shl => self.acir_context.shift_left_var(lhs, rhs, binary_type), - BinaryOp::Shr => self.acir_context.shift_right_var(lhs, rhs, binary_type), + BinaryOp::Shr => self.acir_context.shift_right_var( + lhs, + rhs, + binary_type, + self.current_side_effects_enabled_var, + ), BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs, binary_type), BinaryOp::And => self.acir_context.and_var(lhs, rhs, binary_type), BinaryOp::Or => self.acir_context.or_var(lhs, rhs, binary_type), - BinaryOp::Mod => self.acir_context.modulo_var(lhs, rhs, bit_count), + BinaryOp::Mod => self.acir_context.modulo_var( + lhs, + rhs, + bit_count, + self.current_side_effects_enabled_var, + ), } } @@ -739,8 +754,10 @@ impl Context { } } // Generate the sorted output variables - let out_vars = - self.acir_context.sort(input_vars, bit_size).expect("Could not sort"); + let out_vars = self + .acir_context + .sort(input_vars, bit_size, self.current_side_effects_enabled_var) + .expect("Could not sort"); Ok(Self::convert_vars_to_values(out_vars, dfg, result_ids)) } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs index 76395ea74ab..aa4b561638e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs @@ -1,5 +1,7 @@ use std::collections::HashSet; +use iter_extended::vecmap; + use super::basic_block::BasicBlockId; use super::dfg::DataFlowGraph; use super::instruction::TerminatorInstruction; @@ -116,6 +118,12 @@ impl Function { } blocks } + + pub(crate) fn signature(&self) -> Signature { + let params = vecmap(self.parameters(), |param| self.dfg.type_of_value(*param)); + let returns = vecmap(self.returns(), |ret| self.dfg.type_of_value(*ret)); + Signature { params, returns } + } } impl std::fmt::Display for RuntimeType { @@ -133,7 +141,7 @@ impl std::fmt::Display for RuntimeType { /// within Call instructions. pub(crate) type FunctionId = Id; -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] pub(crate) struct Signature { pub(crate) params: Vec, pub(crate) returns: Vec, diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs index c31d0c58deb..4ef7625ec80 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs @@ -12,31 +12,15 @@ use iter_extended::vecmap; use crate::ssa_refactor::{ ir::{ basic_block::BasicBlockId, - function::{Function, FunctionId, RuntimeType}, + function::{Function, FunctionId, RuntimeType, Signature}, instruction::{BinaryOp, Instruction}, types::{NumericType, Type}, - value::Value, + value::{Value, ValueId}, }, ssa_builder::FunctionBuilder, ssa_gen::Ssa, }; -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -struct FunctionSignature { - parameters: Vec, - returns: Vec, - runtime: RuntimeType, -} - -impl FunctionSignature { - fn from(function: &Function) -> Self { - let parameters = vecmap(function.parameters(), |param| function.dfg.type_of_value(*param)); - let returns = vecmap(function.returns(), |ret| function.dfg.type_of_value(*ret)); - let runtime = function.runtime(); - Self { parameters, returns, runtime } - } -} - /// Represents an 'apply' function created by this pass to dispatch higher order functions to. /// Pseudocode of an `apply` function is given below: /// ```text @@ -64,8 +48,7 @@ struct ApplyFunction { #[derive(Debug, Clone)] struct DefunctionalizationContext { fn_to_runtime: HashMap, - variants: HashMap>, - apply_functions: HashMap, + apply_functions: HashMap, } impl Ssa { @@ -73,11 +56,11 @@ impl Ssa { // Find all functions used as value that share the same signature let variants = find_variants(&self); - let apply_functions = create_apply_functions(&mut self, &variants); + let apply_functions = create_apply_functions(&mut self, variants); let fn_to_runtime = self.functions.iter().map(|(func_id, func)| (*func_id, func.runtime())).collect(); - let context = DefunctionalizationContext { fn_to_runtime, variants, apply_functions }; + let context = DefunctionalizationContext { fn_to_runtime, apply_functions }; context.defunctionalize_all(&mut self); self @@ -94,7 +77,7 @@ impl DefunctionalizationContext { /// Defunctionalize a single function fn defunctionalize(&mut self, func: &mut Function) { - let mut target_function_ids = HashSet::new(); + let mut call_target_values = HashSet::new(); for block_id in func.reachable_blocks() { let block = &func.dfg[block_id]; @@ -114,21 +97,14 @@ impl DefunctionalizationContext { match func.dfg[target_func_id] { // If the target is a function used as value Value::Param { .. } | Value::Instruction { .. } => { - // Collect the argument types - let argument_types = vecmap(&arguments, |arg| func.dfg.type_of_value(*arg)); - - // Collect the result types - let result_types = - vecmap(func.dfg.instruction_results(instruction_id), |result| { - func.dfg.type_of_value(*result) - }); + let results = func.dfg.instruction_results(instruction_id); + let signature = Signature { + params: vecmap(&arguments, |param| func.dfg.type_of_value(*param)), + returns: vecmap(results, |result| func.dfg.type_of_value(*result)), + }; + // Find the correct apply function - let apply_function = self.get_apply_function(&FunctionSignature { - parameters: argument_types, - returns: result_types, - runtime: func.runtime(), - }); - target_function_ids.insert(apply_function.id); + let apply_function = self.get_apply_function(&signature); // Replace the instruction with a call to apply let apply_function_value_id = func.dfg.import_function(apply_function.id); @@ -136,10 +112,12 @@ impl DefunctionalizationContext { arguments.insert(0, target_func_id); } let func = apply_function_value_id; + call_target_values.insert(func); + replacement_instruction = Some(Instruction::Call { func, arguments }); } - Value::Function(id) => { - target_function_ids.insert(id); + Value::Function(..) => { + call_target_values.insert(target_func_id); } _ => {} } @@ -156,7 +134,7 @@ impl DefunctionalizationContext { match &func.dfg[value_id] { // If the value is a static function, transform it to the function id Value::Function(id) => { - if !target_function_ids.contains(id) { + if !call_target_values.contains(&value_id) { let new_value = func.dfg.make_constant(function_id_to_field(*id), Type::field()); func.dfg.set_value_from_id(value_id, new_value); @@ -173,72 +151,118 @@ impl DefunctionalizationContext { } /// Returns the apply function for the given signature - fn get_apply_function(&self, signature: &FunctionSignature) -> ApplyFunction { + fn get_apply_function(&self, signature: &Signature) -> ApplyFunction { *self.apply_functions.get(signature).expect("Could not find apply function") } } -/// Collects all functions used as a value by their signatures -fn find_variants(ssa: &Ssa) -> HashMap> { - let mut variants: HashMap> = HashMap::new(); - let mut functions_used_as_values = HashSet::new(); +/// Collects all functions used as values that can be called by their signatures +fn find_variants(ssa: &Ssa) -> HashMap> { + let mut dynamic_dispatches: HashSet = HashSet::new(); + let mut functions_as_values: HashSet = HashSet::new(); for function in ssa.functions.values() { - functions_used_as_values.extend(functions_as_values(function)); + functions_as_values.extend(find_functions_as_values(function)); + dynamic_dispatches.extend(find_dynamic_dispatches(function)); } - for function_id in functions_used_as_values { - let function = &ssa.functions[&function_id]; - let signature = FunctionSignature::from(function); - variants.entry(signature).or_default().push(function_id); + let mut signature_to_functions_as_value: HashMap> = HashMap::new(); + + for function_id in functions_as_values { + let signature = ssa.functions[&function_id].signature(); + signature_to_functions_as_value.entry(signature).or_default().push(function_id); + } + + let mut variants = HashMap::new(); + + for dispatch_signature in dynamic_dispatches { + let mut target_fns = vec![]; + for (target_signature, functions) in &signature_to_functions_as_value { + if &dispatch_signature == target_signature { + target_fns.extend(functions); + } + } + variants.insert(dispatch_signature, target_fns); } variants } /// Finds all literal functions used as values in the given function -fn functions_as_values(func: &Function) -> HashSet { - let mut literal_functions: HashSet<_> = func - .dfg - .values_iter() - .filter_map(|(id, _)| match func.dfg[id] { - Value::Function(id) => Some(id), - _ => None, - }) - .collect(); +fn find_functions_as_values(func: &Function) -> HashSet { + let mut functions_as_values: HashSet = HashSet::new(); + + let mut process_value = |value_id: ValueId| { + if let Value::Function(id) = func.dfg[value_id] { + functions_as_values.insert(id); + } + }; for block_id in func.reachable_blocks() { let block = &func.dfg[block_id]; for instruction_id in block.instructions() { let instruction = &func.dfg[*instruction_id]; - let target_value = match instruction { - Instruction::Call { func, .. } => func, + match instruction { + Instruction::Call { arguments, .. } => { + arguments.iter().for_each(|value_id| process_value(*value_id)); + } + Instruction::Store { value, .. } => { + process_value(*value); + } _ => continue, }; - let target_id = match func.dfg[*target_value] { - Value::Function(id) => id, + } + + block.unwrap_terminator().for_each_value(&mut process_value); + } + + functions_as_values +} + +/// Finds all dynamic dispatch signatures in the given function +fn find_dynamic_dispatches(func: &Function) -> HashSet { + let mut dispatches = HashSet::new(); + + for block_id in func.reachable_blocks() { + let block = &func.dfg[block_id]; + for instruction_id in block.instructions() { + let instruction = &func.dfg[*instruction_id]; + match instruction { + Instruction::Call { func: target, arguments } => { + if let Value::Param { .. } | Value::Instruction { .. } = &func.dfg[*target] { + let results = func.dfg.instruction_results(*instruction_id); + dispatches.insert(Signature { + params: vecmap(arguments, |param| func.dfg.type_of_value(*param)), + returns: vecmap(results, |result| func.dfg.type_of_value(*result)), + }); + } + } _ => continue, }; - literal_functions.remove(&target_id); } } - literal_functions + dispatches } fn create_apply_functions( ssa: &mut Ssa, - variants_map: &HashMap>, -) -> HashMap { + variants_map: HashMap>, +) -> HashMap { let mut apply_functions = HashMap::new(); - for (signature, variants) in variants_map.iter() { + for (signature, variants) in variants_map.into_iter() { + assert!( + !variants.is_empty(), + "ICE: at least one variant should exist for a dynamic call {:?}", + signature + ); let dispatches_to_multiple_functions = variants.len() > 1; + let id = if dispatches_to_multiple_functions { - create_apply_function(ssa, signature, variants) + create_apply_function(ssa, signature.clone(), variants) } else { variants[0] }; - apply_functions - .insert(signature.clone(), ApplyFunction { id, dispatches_to_multiple_functions }); + apply_functions.insert(signature, ApplyFunction { id, dispatches_to_multiple_functions }); } apply_functions } @@ -250,15 +274,14 @@ fn function_id_to_field(function_id: FunctionId) -> FieldElement { /// Creates an apply function for the given signature and variants fn create_apply_function( ssa: &mut Ssa, - signature: &FunctionSignature, - function_ids: &[FunctionId], + signature: Signature, + function_ids: Vec, ) -> FunctionId { assert!(!function_ids.is_empty()); ssa.add_fn(|id| { - let mut function_builder = FunctionBuilder::new("apply".to_string(), id, signature.runtime); + let mut function_builder = FunctionBuilder::new("apply".to_string(), id, RuntimeType::Acir); let target_id = function_builder.add_parameter(Type::field()); - let params_ids = - vecmap(signature.parameters.clone(), |typ| function_builder.add_parameter(typ)); + let params_ids = vecmap(signature.params, |typ| function_builder.add_parameter(typ)); let mut previous_target_block = None; for (index, function_id) in function_ids.iter().enumerate() { @@ -293,7 +316,7 @@ fn create_apply_function( let target_block = build_return_block( &mut function_builder, current_block, - signature.returns.clone(), + &signature.returns, previous_target_block, ); previous_target_block = Some(target_block); @@ -321,13 +344,13 @@ fn create_apply_function( fn build_return_block( builder: &mut FunctionBuilder, previous_block: BasicBlockId, - passed_types: Vec, + passed_types: &[Type], target: Option, ) -> BasicBlockId { let return_block = builder.insert_block(); builder.switch_to_block(return_block); - let params = vecmap(passed_types, |typ| builder.add_block_parameter(return_block, typ)); + let params = vecmap(passed_types, |typ| builder.add_block_parameter(return_block, typ.clone())); match target { None => builder.terminate_with_return(params), Some(target) => builder.terminate_with_jmp(target, params), diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index 3b6512a3c45..6ffbacf652c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -157,16 +157,14 @@ use crate::ssa_refactor::{ mod branch_analysis; impl Ssa { - /// Flattens the control flow graph of each function such that the function is left with a + /// Flattens the control flow graph of main such that the function is left with a /// single block containing all instructions and no more control-flow. /// /// This pass will modify any instructions with side effects in particular, often multiplying /// them by jump conditions to maintain correctness even when all branches of a jmpif are inlined. /// For more information, see the module-level comment at the top of this file. pub(crate) fn flatten_cfg(mut self) -> Ssa { - for function in self.functions.values_mut() { - flatten_function_cfg(function); - } + flatten_function_cfg(self.main_mut()); self } } diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index b1cd30a6777..e36f5b5d260 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -291,6 +291,7 @@ pub enum Literal { Bool(bool), Integer(FieldElement), Str(String), + Unit, } #[derive(Debug, PartialEq, Eq, Clone)] @@ -465,6 +466,7 @@ impl Display for Literal { Literal::Bool(boolean) => write!(f, "{}", if *boolean { "true" } else { "false" }), Literal::Integer(integer) => write!(f, "{}", integer.to_u128()), Literal::Str(string) => write!(f, "\"{string}\""), + Literal::Unit => write!(f, "()"), } } } diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index cd188f46392..6c9237bff48 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -14,12 +14,10 @@ use std::collections::HashMap; /// Helper object which groups together several useful context objects used /// during name resolution. Once name resolution is finished, only the /// def_interner is required for type inference and monomorphization. -#[derive(Default)] pub struct Context { pub def_interner: NodeInterner, pub crate_graph: CrateGraph, pub(crate) def_maps: HashMap, - // TODO(#1599): Remove default impl and move creation/control of the FileManager into places that construct Context pub file_manager: FileManager, /// Maps a given (contract) module id to the next available storage slot diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index ea0f341e983..506844824c8 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -160,11 +160,12 @@ impl<'a> Resolver<'a> { } for unused_var in unused_vars.iter() { - let definition_info = self.interner.definition(unused_var.id); - let name = &definition_info.name; - if name != ERROR_IDENT && !definition_info.is_global() { - let ident = Ident(Spanned::from(unused_var.location.span, name.to_owned())); - self.push_err(ResolverError::UnusedVariable { ident }); + if let Some(definition_info) = self.interner.try_definition(unused_var.id) { + let name = &definition_info.name; + if name != ERROR_IDENT && !definition_info.is_global() { + let ident = Ident(Spanned::from(unused_var.location.span, name.to_owned())); + self.push_err(ResolverError::UnusedVariable { ident }); + } } } } @@ -204,6 +205,8 @@ impl<'a> Resolver<'a> { warn_if_unused: bool, definition: DefinitionKind, ) -> HirIdent { + let allow_shadowing = allow_shadowing || &name == "_"; + if definition.is_global() { return self.add_global_variable_decl(name, definition); } @@ -883,6 +886,7 @@ impl<'a> Resolver<'a> { } Literal::Integer(integer) => HirLiteral::Integer(integer), Literal::Str(str) => HirLiteral::Str(str), + Literal::Unit => HirLiteral::Unit, }), ExpressionKind::Variable(path) => { // If the Path is being used as an Expression, then it is referring to a global from a separate module @@ -1280,14 +1284,15 @@ pub fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result< Err(ResolverError::MutableReferenceToArrayElement { span }) } HirExpression::Ident(ident) => { - let definition = interner.definition(ident.id); - if !definition.mutable { - let span = interner.expr_span(&rhs); - let variable = definition.name.clone(); - Err(ResolverError::MutableReferenceToImmutableVariable { span, variable }) - } else { - Ok(()) + if let Some(definition) = interner.try_definition(ident.id) { + if !definition.mutable { + return Err(ResolverError::MutableReferenceToImmutableVariable { + span: interner.expr_span(&rhs), + variable: definition.name.clone(), + }); + } } + Ok(()) } _ => Ok(()), } diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index 2ea9a33d191..1625c8a320f 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -86,6 +86,7 @@ impl<'interner> TypeChecker<'interner> { let len = Type::Constant(string.len() as u64); Type::String(Box::new(len)) } + HirLiteral::Unit => Type::Unit, } } HirExpression::Infix(infix_expr) => { diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index 52b6fc7dd2e..4354d4cc77b 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -128,15 +128,16 @@ impl<'interner> TypeChecker<'interner> { let typ = self.interner.id_type(ident.id).instantiate(self.interner).0; let typ = typ.follow_bindings(); - let definition = self.interner.definition(ident.id); - if !definition.mutable && !matches!(typ, Type::MutableReference(_)) { - self.errors.push(TypeCheckError::Unstructured { - msg: format!( - "Variable {} must be mutable to be assigned to", - definition.name - ), - span: ident.location.span, - }); + if let Some(definition) = self.interner.try_definition(ident.id) { + if !definition.mutable && !matches!(typ, Type::MutableReference(_)) { + self.errors.push(TypeCheckError::Unstructured { + msg: format!( + "Variable {} must be mutable to be assigned to", + definition.name + ), + span: ident.location.span, + }); + } } typ diff --git a/crates/noirc_frontend/src/hir_def/expr.rs b/crates/noirc_frontend/src/hir_def/expr.rs index b9ee6634cc7..63b7e421dc3 100644 --- a/crates/noirc_frontend/src/hir_def/expr.rs +++ b/crates/noirc_frontend/src/hir_def/expr.rs @@ -80,6 +80,7 @@ pub enum HirLiteral { Bool(bool), Integer(FieldElement), Str(String), + Unit, } #[derive(Debug, Clone)] diff --git a/crates/noirc_frontend/src/lexer/errors.rs b/crates/noirc_frontend/src/lexer/errors.rs index 189bfdb4bb2..0b6440dec44 100644 --- a/crates/noirc_frontend/src/lexer/errors.rs +++ b/crates/noirc_frontend/src/lexer/errors.rs @@ -8,7 +8,7 @@ use thiserror::Error; #[derive(Error, Clone, Debug, PartialEq, Eq)] pub enum LexerErrorKind { #[error("An unexpected character {:?} was found.", found)] - UnexpectedCharacter { span: Span, expected: String, found: char }, + UnexpectedCharacter { span: Span, expected: String, found: Option }, #[error("NotADoubleChar : {:?} is not a double char token", found)] NotADoubleChar { span: Span, found: Token }, #[error("InvalidIntegerLiteral : {:?} is not a integer", found)] @@ -19,6 +19,8 @@ pub enum LexerErrorKind { TooManyBits { span: Span, max: u32, got: u32 }, #[error("LogicalAnd used instead of bitwise and")] LogicalAnd { span: Span }, + #[error("Unterminated block comment")] + UnterminatedBlockComment { span: Span }, } impl LexerErrorKind { @@ -30,6 +32,7 @@ impl LexerErrorKind { LexerErrorKind::MalformedFuncAttribute { span, .. } => *span, LexerErrorKind::TooManyBits { span, .. } => *span, LexerErrorKind::LogicalAnd { span } => *span, + LexerErrorKind::UnterminatedBlockComment { span } => *span, } } @@ -39,11 +42,15 @@ impl LexerErrorKind { span, expected, found, - } => ( - "an unexpected character was found".to_string(), - format!(" expected {expected} , but got {found}"), - *span, - ), + } => { + let found: String = found.map(Into::into).unwrap_or_else(|| "".into()); + + ( + "an unexpected character was found".to_string(), + format!(" expected {expected} , but got {}", found), + *span, + ) + }, LexerErrorKind::NotADoubleChar { span, found } => ( format!("tried to parse {found} as double char"), format!( @@ -73,6 +80,7 @@ impl LexerErrorKind { "Try `&` instead, or use `if` only if you require short-circuiting".to_string(), *span, ), + LexerErrorKind::UnterminatedBlockComment { span } => ("unterminated block comment".to_string(), "Unterminated block comment".to_string(), *span), } } } diff --git a/crates/noirc_frontend/src/lexer/lexer.rs b/crates/noirc_frontend/src/lexer/lexer.rs index c2627f06143..30866be52ce 100644 --- a/crates/noirc_frontend/src/lexer/lexer.rs +++ b/crates/noirc_frontend/src/lexer/lexer.rs @@ -182,22 +182,6 @@ impl<'a> Lexer<'a> { } Ok(spanned_prev_token) } - Token::Underscore => { - let next_char = self.peek_char(); - let peeked_char = match next_char { - Some(peek_char) => peek_char, - None => return Ok(spanned_prev_token), - }; - - if peeked_char.is_ascii_alphabetic() { - // Okay to unwrap here because we already peeked to - // see that we have a character - let current_char = self.next_char().unwrap(); - return self.eat_word(current_char); - } - - Ok(spanned_prev_token) - } _ => Err(LexerErrorKind::NotADoubleChar { span: Span::single_char(self.position), found: prev_token, @@ -244,7 +228,7 @@ impl<'a> Lexer<'a> { '0'..='9' => self.eat_digit(initial_char), _ => Err(LexerErrorKind::UnexpectedCharacter { span: Span::single_char(self.position), - found: initial_char, + found: initial_char.into(), expected: "an alpha numeric character".to_owned(), }), } @@ -254,7 +238,7 @@ impl<'a> Lexer<'a> { if !self.peek_char_is('[') { return Err(LexerErrorKind::UnexpectedCharacter { span: Span::single_char(self.position), - found: self.next_char().unwrap(), + found: self.next_char(), expected: "[".to_owned(), }); } @@ -269,7 +253,7 @@ impl<'a> Lexer<'a> { return Err(LexerErrorKind::UnexpectedCharacter { span: Span::single_char(self.position), expected: "]".to_owned(), - found: self.next_char().unwrap(), + found: self.next_char(), }); } self.next_char(); @@ -339,6 +323,7 @@ impl<'a> Lexer<'a> { } fn parse_block_comment(&mut self) -> SpannedTokenResult { + let span = Span::new(self.position..self.position + 1); let mut depth = 1usize; while let Some(ch) = self.next_char() { @@ -352,6 +337,8 @@ impl<'a> Lexer<'a> { depth -= 1; // This block comment is closed, so for a construction like "/* */ */" + // there will be a successfully parsed block comment "/* */" + // and " */" will be processed separately. if depth == 0 { break; } @@ -360,7 +347,11 @@ impl<'a> Lexer<'a> { } } - self.next_token() + if depth == 0 { + self.next_token() + } else { + Err(LexerErrorKind::UnterminatedBlockComment { span }) + } } /// Skips white space. They are not significant in the source language @@ -427,6 +418,15 @@ fn test_single_double_char() { } } +#[test] +fn invalid_attribute() { + let input = "#"; + let mut lexer = Lexer::new(input); + + let token = lexer.next().unwrap(); + assert!(token.is_err()); +} + #[test] fn test_custom_gate_syntax() { let input = "#[foreign(sha256)]#[foreign(blake2s)]#[builtin(sum)]"; @@ -488,6 +488,16 @@ fn test_arithmetic_sugar() { } } +#[test] +fn unterminated_block_comment() { + let input = "/*/"; + + let mut lexer = Lexer::new(input); + let token = lexer.next().unwrap(); + + assert!(token.is_err()); +} + #[test] fn test_comment() { let input = "// hello diff --git a/crates/noirc_frontend/src/lexer/token.rs b/crates/noirc_frontend/src/lexer/token.rs index 90bc957600c..a58a9cbe249 100644 --- a/crates/noirc_frontend/src/lexer/token.rs +++ b/crates/noirc_frontend/src/lexer/token.rs @@ -80,8 +80,6 @@ pub enum Token { Semicolon, /// ! Bang, - /// _ - Underscore, /// = Assign, #[allow(clippy::upper_case_acronyms)] @@ -182,7 +180,6 @@ impl fmt::Display for Token { Token::Semicolon => write!(f, ";"), Token::Assign => write!(f, "="), Token::Bang => write!(f, "!"), - Token::Underscore => write!(f, "_"), Token::EOF => write!(f, "end of input"), Token::Invalid(c) => write!(f, "{c}"), } @@ -528,7 +525,6 @@ impl Keyword { "true" => return Some(Token::Bool(true)), "false" => return Some(Token::Bool(false)), - "_" => return Some(Token::Underscore), _ => return None, }; diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index d8dfb1550c0..c3b3ae44bd2 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -271,6 +271,7 @@ impl<'interner> Monomorphizer<'interner> { self.repeated_array(repeated_element, length) } }, + HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]), HirExpression::Block(block) => self.block(block.0), HirExpression::Prefix(prefix) => ast::Expression::Unary(ast::Unary { diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index f37daf45136..ce12cf5cfc3 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -478,10 +478,20 @@ impl NodeInterner { } } + /// Retrieves the definition where the given id was defined. + /// This will panic if given DefinitionId::dummy_id. Use try_definition for + /// any call with a possibly undefined variable. pub fn definition(&self, id: DefinitionId) -> &DefinitionInfo { &self.definitions[id.0] } + /// Tries to retrieve the given id's definition. + /// This function should be used during name resolution or type checking when we cannot be sure + /// all variables have corresponding definitions (in case of an error in the user's code). + pub fn try_definition(&self, id: DefinitionId) -> Option<&DefinitionInfo> { + self.definitions.get(id.0) + } + /// Returns the name of the definition /// /// This is needed as the Environment needs to map variable names to witness indices diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 3f9fe31f414..b44e4536844 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -35,9 +35,9 @@ use crate::parser::{force, ignore_then_commit, statement_recovery}; use crate::token::{Attribute, Keyword, Token, TokenKind}; use crate::{ BinaryOp, BinaryOpKind, BlockExpression, CompTime, ConstrainStatement, FunctionDefinition, - Ident, IfExpression, InfixExpression, LValue, Lambda, NoirFunction, NoirStruct, NoirTrait, - Path, PathKind, Pattern, Recoverable, TraitConstraint, TraitImpl, TraitImplItem, TraitItem, - TypeImpl, UnaryOp, UnresolvedTypeExpression, UseTree, UseTreeKind, + Ident, IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, NoirStruct, + NoirTrait, Path, PathKind, Pattern, Recoverable, TraitConstraint, TraitImpl, TraitImplItem, + TraitItem, TypeImpl, UnaryOp, UnresolvedTypeExpression, UseTree, UseTreeKind, }; use chumsky::prelude::*; @@ -788,6 +788,7 @@ fn parse_type_inner( string_type(), named_type(recursive_type_parser.clone()), array_type(recursive_type_parser.clone()), + recursive_type_parser.clone().delimited_by(just(Token::LeftParen), just(Token::RightParen)), tuple_type(recursive_type_parser.clone()), function_type(recursive_type_parser.clone()), mutable_reference_type(recursive_type_parser), @@ -893,7 +894,13 @@ where T: NoirParser, { let fields = type_parser.separated_by(just(Token::Comma)).allow_trailing(); - parenthesized(fields).map(UnresolvedType::Tuple) + parenthesized(fields).map(|fields| { + if fields.is_empty() { + UnresolvedType::Unit + } else { + UnresolvedType::Tuple(fields) + } + }) } fn function_type(type_parser: T) -> impl NoirParser @@ -1303,8 +1310,14 @@ fn tuple

(expr_parser: P) -> impl NoirParser where P: ExprParser, { - parenthesized(expression_list(expr_parser)) - .map_with_span(|elements, span| Expression::new(ExpressionKind::Tuple(elements), span)) + parenthesized(expression_list(expr_parser)).map_with_span(|elements, span| { + let kind = if elements.is_empty() { + ExpressionKind::Literal(Literal::Unit) + } else { + ExpressionKind::Tuple(elements) + }; + Expression::new(kind, span) + }) } fn field_name() -> impl NoirParser { @@ -1662,7 +1675,7 @@ mod test { // Let statements are not type checked here, so the parser will accept as // long as it is a type. Other statements such as Public are type checked // Because for now, they can only have one type - parse_all(declaration(expression()), vec!["let x = y", "let x : u8 = y"]); + parse_all(declaration(expression()), vec!["let _ = 42", "let x = y", "let x : u8 = y"]); } #[test] diff --git a/crates/wasm/Cargo.toml b/crates/wasm/Cargo.toml index 5c139d96164..2b834318168 100644 --- a/crates/wasm/Cargo.toml +++ b/crates/wasm/Cargo.toml @@ -13,6 +13,7 @@ crate-type = ["cdylib"] [dependencies] acvm.workspace = true +fm.workspace = true noirc_driver.workspace = true noirc_frontend.workspace = true wasm-bindgen.workspace = true @@ -27,4 +28,4 @@ gloo-utils = { version = "0.1", features = ["serde"] } getrandom = { version = "*", features = ["js"] } [build-dependencies] -build-data = "0.1.3" \ No newline at end of file +build-data = "0.1.3" diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index 5496920cf56..7321b4f31e4 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -1,4 +1,5 @@ use acvm::acir::circuit::Circuit; +use fm::FileManager; use gloo_utils::format::JsValueSerdeExt; use log::debug; use noirc_driver::{ @@ -6,11 +7,11 @@ use noirc_driver::{ propagate_dep, CompileOptions, CompiledContract, }; use noirc_frontend::{ - graph::{CrateName, CrateType}, + graph::{CrateGraph, CrateName, CrateType}, hir::Context, }; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::path::Path; use wasm_bindgen::prelude::*; #[derive(Debug, Serialize, Deserialize)] @@ -61,8 +62,8 @@ impl Default for WASMCompileOptions { } fn add_noir_lib(context: &mut Context, crate_name: &str) { - let path_to_lib = PathBuf::from(&crate_name).join("lib.nr"); - let library_crate = create_non_local_crate(context, path_to_lib, CrateType::Library); + let path_to_lib = Path::new(&crate_name).join("lib.nr"); + let library_crate = create_non_local_crate(context, &path_to_lib, CrateType::Library); propagate_dep(context, library_crate, &CrateName::new(crate_name).unwrap()); } @@ -80,9 +81,12 @@ pub fn compile(args: JsValue) -> JsValue { debug!("Compiler configuration {:?}", &options); - let mut context = Context::default(); + let root = Path::new("/"); + let fm = FileManager::new(root); + let graph = CrateGraph::default(); + let mut context = Context::new(fm, graph); - let path = PathBuf::from(&options.entry_point); + let path = Path::new(&options.entry_point); let crate_id = create_local_crate(&mut context, path, CrateType::Binary); for dependency in options.optional_dependencies_set {