From f20bbf5c55ce55d3c429c4d824110fe419896a5f Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Jan 2024 12:49:58 +0000 Subject: [PATCH 01/10] feat: Extract parsing to its own pass and do it in parallel --- compiler/fm/src/file_map.rs | 4 +++ .../noirc_driver/tests/stdlib_warnings.rs | 11 +++--- .../src/hir/def_collector/dc_mod.rs | 4 +-- .../noirc_frontend/src/hir/def_map/mod.rs | 2 +- compiler/noirc_frontend/src/hir/mod.rs | 28 ++++++++++++--- compiler/noirc_frontend/src/tests.rs | 2 +- compiler/wasm/src/compile.rs | 23 +++++++++--- compiler/wasm/src/compile_new.rs | 21 ++++++++--- tooling/lsp/src/lib.rs | 7 ++-- tooling/lsp/src/notifications/mod.rs | 6 ++-- tooling/lsp/src/requests/goto_declaration.rs | 6 ++-- tooling/lsp/src/requests/goto_definition.rs | 6 ++-- tooling/lsp/src/requests/profile_run.rs | 6 +++- tooling/lsp/src/requests/test_run.rs | 6 ++-- tooling/lsp/src/requests/tests.rs | 6 ++-- tooling/nargo/src/lib.rs | 24 ++++++++++--- tooling/nargo/src/ops/compile.rs | 21 ++++++++--- tooling/nargo_cli/src/cli/check_cmd.rs | 10 +++--- .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 16 +++++++-- tooling/nargo_cli/src/cli/compile_cmd.rs | 36 +++++++++++++++---- tooling/nargo_cli/src/cli/dap_cmd.rs | 4 ++- tooling/nargo_cli/src/cli/debug_cmd.rs | 4 ++- tooling/nargo_cli/src/cli/execute_cmd.rs | 4 ++- tooling/nargo_cli/src/cli/export_cmd.rs | 8 +++-- tooling/nargo_cli/src/cli/info_cmd.rs | 4 ++- tooling/nargo_cli/src/cli/prove_cmd.rs | 4 ++- tooling/nargo_cli/src/cli/test_cmd.rs | 13 +++++-- tooling/nargo_cli/src/cli/verify_cmd.rs | 4 ++- 28 files changed, 220 insertions(+), 70 deletions(-) diff --git a/compiler/fm/src/file_map.rs b/compiler/fm/src/file_map.rs index c4d7002a082..50412d352ec 100644 --- a/compiler/fm/src/file_map.rs +++ b/compiler/fm/src/file_map.rs @@ -75,6 +75,10 @@ impl FileMap { pub fn get_file_id(&self, file_name: &PathString) -> Option { self.name_to_id.get(file_name).cloned() } + + pub fn all_file_ids(&self) -> impl Iterator { + self.name_to_id.values() + } } impl Default for FileMap { fn default() -> Self { diff --git a/compiler/noirc_driver/tests/stdlib_warnings.rs b/compiler/noirc_driver/tests/stdlib_warnings.rs index e9153ec2f99..b2e6e8db3bf 100644 --- a/compiler/noirc_driver/tests/stdlib_warnings.rs +++ b/compiler/noirc_driver/tests/stdlib_warnings.rs @@ -1,22 +1,23 @@ -use std::path::Path; +use std::{collections::HashMap, path::Path}; use noirc_driver::{file_manager_with_stdlib, prepare_crate, ErrorsAndWarnings}; -use noirc_frontend::hir::Context; +use noirc_frontend::hir::{def_map::parse_file, Context}; #[test] fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings> { // We use a minimal source file so that if stdlib produces warnings then we can expect these warnings to _always_ // be emitted. - let source = "fn main() {}"; + let source: &str = "fn main() {}"; let root = Path::new(""); let file_name = Path::new("main.nr"); let mut file_manager = file_manager_with_stdlib(root); - file_manager.add_file_with_source(file_name, source.to_owned()).expect( + let file_id = file_manager.add_file_with_source(file_name, source.to_owned()).expect( "Adding source buffer to file manager should never fail when file manager is empty", ); + let parsed_source = parse_file(&file_manager, file_id); - let mut context = Context::new(file_manager); + let mut context = Context::new(file_manager, HashMap::from([(file_id, parsed_source)])); let root_crate_id = prepare_crate(&mut context, file_name); let ((), warnings) = noirc_driver::check_crate(&mut context, root_crate_id, false, false)?; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 2e6eb3992ff..3cd60c33b8b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -20,7 +20,7 @@ use super::{ }, errors::{DefCollectorErrorKind, DuplicateType}, }; -use crate::hir::def_map::{parse_file, LocalModuleId, ModuleData, ModuleId}; +use crate::hir::def_map::{LocalModuleId, ModuleData, ModuleId}; use crate::hir::resolution::import::ImportDirective; use crate::hir::Context; @@ -555,7 +555,7 @@ impl<'a> ModCollector<'a> { context.visited_files.insert(child_file_id, location); // Parse the AST for the module we just found and then recursively look for it's defs - let (ast, parsing_errors) = parse_file(&context.file_manager, child_file_id); + let (ast, parsing_errors) = context.parsed_file_results(child_file_id); let ast = ast.into_sorted(); errors.extend( diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index d60ceffa9af..8c985e88e0b 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -87,7 +87,7 @@ impl CrateDefMap { // First parse the root file. let root_file_id = context.crate_graph[crate_id].root_file_id; - let (ast, parsing_errors) = parse_file(&context.file_manager, root_file_id); + let (ast, parsing_errors) = context.parsed_file_results(root_file_id); let mut ast = ast.into_sorted(); for macro_processor in ¯o_processors { diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index c62f357167f..cab923be34d 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -7,18 +7,22 @@ pub mod type_check; use crate::graph::{CrateGraph, CrateId}; use crate::hir_def::function::FuncMeta; use crate::node_interner::{FuncId, NodeInterner, StructId}; +use crate::parser::ParserError; +use crate::ParsedModule; use def_map::{Contract, CrateDefMap}; use fm::FileManager; use noirc_errors::Location; use std::borrow::Cow; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use self::def_map::TestFunction; +pub type ParsedFiles = 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. -pub struct Context<'file_manager> { +pub struct Context<'file_manager, 'parsed_files> { pub def_interner: NodeInterner, pub crate_graph: CrateGraph, pub(crate) def_maps: BTreeMap, @@ -30,6 +34,8 @@ pub struct Context<'file_manager> { /// A map of each file that already has been visited from a prior `mod foo;` declaration. /// This is used to issue an error if a second `mod foo;` is declared to the same file. pub visited_files: BTreeMap, + + pub parsed_files: Cow<'parsed_files, ParsedFiles>, } #[derive(Debug, Copy, Clone)] @@ -39,27 +45,39 @@ pub enum FunctionNameMatch<'a> { Contains(&'a str), } -impl Context<'_> { - pub fn new(file_manager: FileManager) -> Context<'static> { +impl<'file_manager, 'parsed_files> Context<'file_manager, 'parsed_files> { + pub fn new(file_manager: FileManager, parsed_files: ParsedFiles) -> Context<'static, 'static> { Context { def_interner: NodeInterner::default(), def_maps: BTreeMap::new(), visited_files: BTreeMap::new(), crate_graph: CrateGraph::default(), file_manager: Cow::Owned(file_manager), + parsed_files: Cow::Owned(parsed_files), } } - pub fn from_ref_file_manager(file_manager: &FileManager) -> Context<'_> { + pub fn from_ref_file_manager( + file_manager: &'file_manager FileManager, + parsed_files: &'parsed_files ParsedFiles, + ) -> Context<'file_manager, 'parsed_files> { Context { def_interner: NodeInterner::default(), def_maps: BTreeMap::new(), visited_files: BTreeMap::new(), crate_graph: CrateGraph::default(), file_manager: Cow::Borrowed(file_manager), + parsed_files: Cow::Borrowed(parsed_files), } } + pub fn parsed_file_results(&self, file_id: fm::FileId) -> (ParsedModule, Vec) { + // TODO: we could make it parse the file if it is not in the cache + // + // TODO: Check if we can return a reference here instead of cloning. + self.parsed_files.get(&file_id).expect("noir file wasn't parsed").clone() + } + /// Returns the CrateDefMap for a given CrateId. /// It is perfectly valid for the compiler to look /// up a CrateDefMap and it is not available. diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index a56c3a7755f..9ccbddab9ec 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -52,7 +52,7 @@ mod test { ) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { let root = std::path::Path::new("/"); let fm = FileManager::new(root); - let mut context = Context::new(fm); + let mut context = Context::new(fm, Default::default()); context.def_interner.populate_dummy_operator_traits(); let root_file_id = FileId::dummy(); let root_crate_id = context.crate_graph.add_crate_root(root_file_id); diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 151dde2eea6..26895c92583 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -13,7 +13,7 @@ use noirc_driver::{ }; use noirc_frontend::{ graph::{CrateId, CrateName}, - hir::Context, + hir::{def_map::parse_file, Context}, }; use serde::Deserialize; use std::{collections::HashMap, path::Path}; @@ -169,8 +169,13 @@ pub fn compile( }; let fm = file_manager_with_source_map(file_source_map); + let parsed_files = fm + .as_file_map() + .all_file_ids() + .map(|&file_id| (file_id, parse_file(&fm, file_id))) + .collect(); - let mut context = Context::new(fm); + let mut context = Context::new(fm, parsed_files); let path = Path::new(&entry_point); let crate_id = prepare_crate(&mut context, path); @@ -303,19 +308,27 @@ pub(crate) fn generate_contract_artifact(contract: CompiledContract) -> CompileR #[cfg(test)] mod test { use noirc_driver::prepare_crate; - use noirc_frontend::{graph::CrateName, hir::Context}; + use noirc_frontend::{ + graph::CrateName, + hir::{def_map::parse_file, Context}, + }; use crate::compile::PathToFileSourceMap; use super::{file_manager_with_source_map, process_dependency_graph, DependencyGraph}; use std::{collections::HashMap, path::Path}; - fn setup_test_context(source_map: PathToFileSourceMap) -> Context<'static> { + fn setup_test_context(source_map: PathToFileSourceMap) -> Context<'static, 'static> { let mut fm = file_manager_with_source_map(source_map); // Add this due to us calling prepare_crate on "/main.nr" below fm.add_file_with_source(Path::new("/main.nr"), "fn foo() {}".to_string()); + let parsed_files = fm + .as_file_map() + .all_file_ids() + .map(|&file_id| (file_id, parse_file(&fm, file_id))) + .collect(); - let mut context = Context::new(fm); + let mut context = Context::new(fm, parsed_files); prepare_crate(&mut context, Path::new("/main.nr")); context diff --git a/compiler/wasm/src/compile_new.rs b/compiler/wasm/src/compile_new.rs index 3cb20bd0b5c..c86f0662653 100644 --- a/compiler/wasm/src/compile_new.rs +++ b/compiler/wasm/src/compile_new.rs @@ -6,6 +6,7 @@ use crate::errors::{CompileError, JsCompileError}; use noirc_driver::{ add_dep, compile_contract, compile_main, prepare_crate, prepare_dependency, CompileOptions, }; +use noirc_frontend::hir::def_map::parse_file; use noirc_frontend::{ graph::{CrateId, CrateName}, hir::Context, @@ -20,7 +21,7 @@ use wasm_bindgen::prelude::wasm_bindgen; pub struct CompilerContext { // `wasm_bindgen` currently doesn't allow lifetime parameters on structs so we must use a `'static` lifetime. // `Context` must then own the `FileManager` to satisfy this lifetime. - context: Context<'static>, + context: Context<'static, 'static>, } #[wasm_bindgen(js_name = "CrateId")] @@ -34,7 +35,14 @@ impl CompilerContext { console_error_panic_hook::set_once(); let fm = file_manager_with_source_map(source_map); - CompilerContext { context: Context::new(fm) } + + let parsed_files = fm + .as_file_map() + .all_file_ids() + .map(|&file_id| (file_id, parse_file(&fm, file_id))) + .collect(); + + CompilerContext { context: Context::new(fm, parsed_files) } } #[cfg(test)] @@ -229,7 +237,7 @@ pub fn compile_( #[cfg(test)] mod test { use noirc_driver::prepare_crate; - use noirc_frontend::hir::Context; + use noirc_frontend::hir::{def_map::parse_file, Context}; use crate::compile::{file_manager_with_source_map, PathToFileSourceMap}; @@ -241,8 +249,13 @@ mod test { let mut fm = file_manager_with_source_map(source_map); // Add this due to us calling prepare_crate on "/main.nr" below fm.add_file_with_source(Path::new("/main.nr"), "fn foo() {}".to_string()); + let parsed_files = fm + .as_file_map() + .all_file_ids() + .map(|&file_id| (file_id, parse_file(&fm, file_id))) + .collect(); - let mut context = Context::new(fm); + let mut context = Context::new(fm, parsed_files); prepare_crate(&mut context, Path::new("/main.nr")); CompilerContext { context } diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 1099ad60269..c0b944b2c73 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -19,7 +19,7 @@ use async_lsp::{ }; use fm::codespan_files as files; use lsp_types::CodeLens; -use nargo::workspace::Workspace; +use nargo::{parse_all, workspace::Workspace}; use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{file_manager_with_stdlib, prepare_crate, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::{ @@ -225,15 +225,16 @@ pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result (Context<'static>, CrateId) { +fn prepare_source(source: String) -> (Context<'static, 'static>, CrateId) { let root = Path::new(""); let file_name = Path::new("main.nr"); let mut file_manager = file_manager_with_stdlib(root); file_manager.add_file_with_source(file_name, source).expect( "Adding source buffer to file manager should never fail when file manager is empty", ); + let parsed_files = parse_all(&file_manager); - let mut context = Context::new(file_manager); + let mut context = Context::new(file_manager, parsed_files); let root_crate_id = prepare_crate(&mut context, file_name); (context, root_crate_id) diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 0cd86803efa..769cfbdfed7 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -1,7 +1,7 @@ use std::ops::ControlFlow; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; -use nargo::{insert_all_files_for_workspace_into_file_manager, prepare_package}; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all, prepare_package}; use noirc_driver::{check_crate, file_manager_with_stdlib}; use noirc_errors::{DiagnosticKind, FileDiagnostic}; @@ -130,11 +130,13 @@ fn process_noir_document( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let diagnostics: Vec<_> = workspace .into_iter() .flat_map(|package| -> Vec { - let (mut context, crate_id) = prepare_package(&workspace_file_manager, package); + let (mut context, crate_id) = + prepare_package(&workspace_file_manager, &parsed_files, package); let file_diagnostics = match check_crate(&mut context, crate_id, false, false) { Ok(((), warnings)) => warnings, diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 6e3664804f6..59636192d87 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -7,7 +7,7 @@ use async_lsp::{ErrorCode, ResponseError}; use lsp_types::request::{GotoDeclarationParams, GotoDeclarationResponse}; -use nargo::insert_all_files_for_workspace_into_file_manager; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use noirc_driver::file_manager_with_stdlib; use super::{position_to_byte_index, to_lsp_location}; @@ -36,8 +36,10 @@ fn on_goto_definition_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); - let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, package); + let (mut context, crate_id) = + nargo::prepare_package(&workspace_file_manager, &parsed_files, package); let interner; if let Some(def_interner) = _state.cached_definitions.get(&package_root_path) { diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 277bbf013f9..fa043365236 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -5,7 +5,7 @@ use crate::{types::GotoDefinitionResult, LspState}; use async_lsp::{ErrorCode, ResponseError}; use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse}; -use nargo::insert_all_files_for_workspace_into_file_manager; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use noirc_driver::file_manager_with_stdlib; use super::{position_to_byte_index, to_lsp_location}; @@ -34,8 +34,10 @@ fn on_goto_definition_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); - let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, package); + let (mut context, crate_id) = + nargo::prepare_package(&workspace_file_manager, &parsed_files, package); let interner; if let Some(def_interner) = _state.cached_definitions.get(&package_root_path) { diff --git a/tooling/lsp/src/requests/profile_run.rs b/tooling/lsp/src/requests/profile_run.rs index 6664475a68c..65e6a936e20 100644 --- a/tooling/lsp/src/requests/profile_run.rs +++ b/tooling/lsp/src/requests/profile_run.rs @@ -5,7 +5,9 @@ use std::{ use acvm::ExpressionWidth; use async_lsp::{ErrorCode, ResponseError}; -use nargo::{artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager}; +use nargo::{ + artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager, parse_all, +}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ file_manager_with_stdlib, CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING, @@ -52,6 +54,7 @@ fn on_profile_run_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); // Since we filtered on crate name, this should be the only item in the iterator match workspace.into_iter().next() { @@ -66,6 +69,7 @@ fn on_profile_run_request_inner( let (compiled_programs, compiled_contracts) = nargo::ops::compile_workspace( &workspace_file_manager, + &parsed_files, &workspace, &binary_packages, &contract_packages, diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index c2181d7839d..8da4d74a654 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -4,7 +4,7 @@ use async_lsp::{ErrorCode, ResponseError}; use nargo::{ insert_all_files_for_workspace_into_file_manager, ops::{run_test, TestStatus}, - prepare_package, + parse_all, prepare_package, }; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ @@ -52,11 +52,13 @@ fn on_test_run_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); // Since we filtered on crate name, this should be the only item in the iterator match workspace.into_iter().next() { Some(package) => { - let (mut context, crate_id) = prepare_package(&workspace_file_manager, package); + let (mut context, crate_id) = + prepare_package(&workspace_file_manager, &parsed_files, package); if check_crate(&mut context, crate_id, false, false).is_err() { let result = NargoTestRunResult { id: params.id.clone(), diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index 0f717b9fb9e..c9af1443cef 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -2,7 +2,7 @@ use std::future::{self, Future}; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; use lsp_types::{LogMessageParams, MessageType}; -use nargo::{insert_all_files_for_workspace_into_file_manager, prepare_package}; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all, prepare_package}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{check_crate, file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING}; @@ -52,11 +52,13 @@ fn on_tests_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files: std::collections::HashMap)> = parse_all(&workspace_file_manager); let package_tests: Vec<_> = workspace .into_iter() .filter_map(|package| { - let (mut context, crate_id) = prepare_package(&workspace_file_manager, package); + let (mut context, crate_id) = + prepare_package(&workspace_file_manager, &parsed_files, package); // We ignore the warnings and errors produced by compilation for producing tests // because we can still get the test functions even if compilation fails let _ = check_crate(&mut context, crate_id, false, false); diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 62ff4325a23..731135bec65 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -20,9 +20,10 @@ use fm::FileManager; use noirc_driver::{add_dep, prepare_crate, prepare_dependency}; use noirc_frontend::{ graph::{CrateId, CrateName}, - hir::Context, + hir::{def_map::parse_file, Context, ParsedFiles}, }; use package::{Dependency, Package}; +use rayon::prelude::*; pub use self::errors::NargoError; @@ -95,11 +96,26 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } } -pub fn prepare_package<'file_manager>( +pub fn parse_all(file_manager: &FileManager) -> ParsedFiles { + let file_ids: Vec<_> = file_manager.as_file_map().all_file_ids().collect(); + file_ids + .into_par_iter() + .filter(|&&file_id| { + let file_path = file_manager.path(file_id).expect("expected file to exist"); + let file_extension = + file_path.extension().expect("expected all file paths to have an extension"); + file_extension == "nr" + }) + .map(|&file_id| (file_id, parse_file(file_manager, file_id))) + .collect() +} + +pub fn prepare_package<'file_manager, 'parsed_files>( file_manager: &'file_manager FileManager, + parsed_files: &'parsed_files ParsedFiles, package: &Package, -) -> (Context<'file_manager>, CrateId) { - let mut context = Context::from_ref_file_manager(file_manager); +) -> (Context<'file_manager, 'parsed_files>, CrateId) { + let mut context = Context::from_ref_file_manager(file_manager, parsed_files); let crate_id = prepare_crate(&mut context, &package.entry_path); diff --git a/tooling/nargo/src/ops/compile.rs b/tooling/nargo/src/ops/compile.rs index 043e2a367a5..c048dbd288a 100644 --- a/tooling/nargo/src/ops/compile.rs +++ b/tooling/nargo/src/ops/compile.rs @@ -1,6 +1,7 @@ use acvm::ExpressionWidth; use fm::FileManager; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract, CompiledProgram}; +use noirc_frontend::hir::ParsedFiles; use crate::errors::CompileError; use crate::prepare_package; @@ -15,6 +16,7 @@ use rayon::prelude::*; /// This function will return an error if there are any compilations errors reported. pub fn compile_workspace( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, binary_packages: &[Package], contract_packages: &[Package], @@ -25,12 +27,21 @@ pub fn compile_workspace( let program_results: Vec> = binary_packages .par_iter() .map(|package| { - compile_program(file_manager, workspace, package, compile_options, expression_width) + compile_program( + file_manager, + parsed_files, + workspace, + package, + compile_options, + expression_width, + ) }) .collect(); let contract_results: Vec> = contract_packages .par_iter() - .map(|package| compile_contract(file_manager, package, compile_options, expression_width)) + .map(|package| { + compile_contract(file_manager, parsed_files, package, compile_options, expression_width) + }) .collect(); // Report any warnings/errors which were encountered during compilation. @@ -62,12 +73,13 @@ pub fn compile_workspace( pub fn compile_program( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, package: &Package, compile_options: &CompileOptions, expression_width: ExpressionWidth, ) -> CompilationResult { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); let program_artifact_path = workspace.package_build_path(package); let mut debug_artifact_path = program_artifact_path.clone(); @@ -84,11 +96,12 @@ pub fn compile_program( fn compile_contract( file_manager: &FileManager, + parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, expression_width: ExpressionWidth, ) -> CompilationResult { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); let (contract, warnings) = match noirc_driver::compile_contract(&mut context, crate_id, compile_options) { Ok(contracts_and_warnings) => contracts_and_warnings, diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index e2db492fe9c..a8b9dbdeeb2 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -6,7 +6,7 @@ use fm::FileManager; use iter_extended::btree_map; use nargo::{ errors::CompileError, insert_all_files_for_workspace_into_file_manager, package::Package, - prepare_package, + parse_all, prepare_package, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME}; @@ -16,7 +16,7 @@ use noirc_driver::{ }; use noirc_frontend::{ graph::{CrateId, CrateName}, - hir::Context, + hir::{Context, ParsedFiles}, }; use super::fs::write_to_file; @@ -54,9 +54,10 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); for package in &workspace { - check_package(&workspace_file_manager, package, &args.compile_options)?; + check_package(&workspace_file_manager, &parsed_files, package, &args.compile_options)?; println!("[{}] Constraint system successfully built!", package.name); } Ok(()) @@ -64,10 +65,11 @@ pub(crate) fn run( fn check_package( file_manager: &FileManager, + parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, ) -> Result<(), CompileError> { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); check_crate_and_report_errors( &mut context, crate_id, diff --git a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs index 1eb8153ce9b..02cbdb8b914 100644 --- a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -9,12 +9,13 @@ use crate::errors::CliError; use acvm::ExpressionWidth; use clap::Args; use fm::FileManager; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::package::Package; use nargo::workspace::Workspace; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::graph::CrateName; +use noirc_frontend::hir::ParsedFiles; /// Generates a Solidity verifier smart contract for the program #[derive(Debug, Clone, Args)] @@ -48,11 +49,13 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let expression_width = backend.get_backend_info()?; for package in &workspace { let smart_contract_string = smart_contract_for_package( &workspace_file_manager, + &parsed_files, &workspace, backend, package, @@ -73,14 +76,21 @@ pub(crate) fn run( fn smart_contract_for_package( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, backend: &Backend, package: &Package, compile_options: &CompileOptions, expression_width: ExpressionWidth, ) -> Result { - let program = - compile_bin_package(file_manager, workspace, package, compile_options, expression_width)?; + let program = compile_bin_package( + file_manager, + parsed_files, + workspace, + package, + compile_options, + expression_width, + )?; Ok(backend.eth_contract(&program.circuit)?) } diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 8bc35080fd7..374628a3c3f 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -7,10 +7,10 @@ use nargo::artifacts::contract::{ContractArtifact, ContractFunctionArtifact}; use nargo::artifacts::debug::DebugArtifact; use nargo::artifacts::program::ProgramArtifact; use nargo::errors::CompileError; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::package::Package; use nargo::prepare_package; use nargo::workspace::Workspace; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::file_manager_with_stdlib; use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; @@ -18,6 +18,7 @@ use noirc_driver::{CompilationResult, CompileOptions, CompiledContract, Compiled use noirc_frontend::graph::CrateName; use clap::Args; +use noirc_frontend::hir::ParsedFiles; use crate::backends::Backend; use crate::errors::CliError; @@ -64,6 +65,7 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace .into_iter() @@ -74,6 +76,7 @@ pub(crate) fn run( let expression_width = backend.get_backend_info_or_default(); let (_, compiled_contracts) = compile_workspace( &workspace_file_manager, + &parsed_files, &workspace, &binary_packages, &contract_packages, @@ -91,6 +94,7 @@ pub(crate) fn run( pub(super) fn compile_workspace( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, binary_packages: &[Package], contract_packages: &[Package], @@ -101,12 +105,21 @@ pub(super) fn compile_workspace( let program_results: Vec> = binary_packages .par_iter() .map(|package| { - compile_program(file_manager, workspace, package, compile_options, expression_width) + compile_program( + file_manager, + parsed_files, + workspace, + package, + compile_options, + expression_width, + ) }) .collect(); let contract_results: Vec> = contract_packages .par_iter() - .map(|package| compile_contract(file_manager, package, compile_options, expression_width)) + .map(|package| { + compile_contract(file_manager, parsed_files, package, compile_options, expression_width) + }) .collect(); // Report any warnings/errors which were encountered during compilation. @@ -138,6 +151,7 @@ pub(super) fn compile_workspace( pub(crate) fn compile_bin_package( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, package: &Package, compile_options: &CompileOptions, @@ -147,8 +161,14 @@ pub(crate) fn compile_bin_package( return Err(CompileError::LibraryCrate(package.name.clone()).into()); } - let compilation_result = - compile_program(file_manager, workspace, package, compile_options, expression_width); + let compilation_result = compile_program( + file_manager, + parsed_files, + workspace, + package, + compile_options, + expression_width, + ); let program = report_errors( compilation_result, @@ -162,12 +182,13 @@ pub(crate) fn compile_bin_package( fn compile_program( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, package: &Package, compile_options: &CompileOptions, expression_width: ExpressionWidth, ) -> CompilationResult { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); let program_artifact_path = workspace.package_build_path(package); let mut debug_artifact_path = program_artifact_path.clone(); @@ -206,11 +227,12 @@ fn compile_program( fn compile_contract( file_manager: &FileManager, + parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, expression_width: ExpressionWidth, ) -> CompilationResult { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); let (contract, warnings) = match noirc_driver::compile_contract(&mut context, crate_id, compile_options) { Ok(contracts_and_warnings) => contracts_and_warnings, diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index 29e696ea608..fe418a81ffd 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -2,8 +2,8 @@ use acvm::acir::native_types::WitnessMap; use backend_interface::Backend; use clap::Args; use nargo::constants::PROVER_INPUT_FILE; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::workspace::Workspace; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::Format; use noirc_driver::{ @@ -70,9 +70,11 @@ fn load_and_compile_project( let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let compiled_program = compile_bin_package( &workspace_file_manager, + &parsed_files, &workspace, package, &CompileOptions::default(), diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index f78a683aa8f..7207513c366 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -6,8 +6,8 @@ use clap::Args; use nargo::artifacts::debug::DebugArtifact; use nargo::constants::PROVER_INPUT_FILE; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::package::Package; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::InputMap; @@ -57,6 +57,7 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let Some(package) = workspace.into_iter().find(|p| p.is_binary()) else { println!( @@ -67,6 +68,7 @@ pub(crate) fn run( let compiled_program = compile_bin_package( &workspace_file_manager, + &parsed_files, &workspace, package, &args.compile_options, diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 7f695c42fa4..aa230613eaf 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -5,9 +5,9 @@ use clap::Args; use nargo::artifacts::debug::DebugArtifact; use nargo::constants::PROVER_INPUT_FILE; use nargo::errors::try_to_diagnose_runtime_error; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::ops::DefaultForeignCallExecutor; use nargo::package::Package; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::InputMap; @@ -66,11 +66,13 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let expression_width = backend.get_backend_info_or_default(); for package in &workspace { let compiled_program = compile_bin_package( &workspace_file_manager, + &parsed_files, &workspace, package, &args.compile_options, diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index ac3e93e09b7..feaa55857e5 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -1,13 +1,14 @@ use nargo::errors::CompileError; use noirc_errors::FileDiagnostic; +use noirc_frontend::hir::ParsedFiles; use rayon::prelude::*; use fm::FileManager; use iter_extended::try_vecmap; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::package::Package; use nargo::prepare_package; use nargo::workspace::Workspace; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ compile_no_check, file_manager_with_stdlib, CompileOptions, CompiledProgram, @@ -60,6 +61,7 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let library_packages: Vec<_> = workspace.into_iter().filter(|package| package.is_library()).collect(); @@ -69,6 +71,7 @@ pub(crate) fn run( .map(|package| { compile_exported_functions( &workspace_file_manager, + &parsed_files, &workspace, package, &args.compile_options, @@ -79,11 +82,12 @@ pub(crate) fn run( fn compile_exported_functions( file_manager: &FileManager, + parsed_files: &ParsedFiles, workspace: &Workspace, package: &Package, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); check_crate_and_report_errors( &mut context, crate_id, diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index f983a19c0fd..5f7dcbfbb6e 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -6,7 +6,7 @@ use clap::Args; use iter_extended::vecmap; use nargo::{ artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager, - package::Package, + package::Package, parse_all, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ @@ -67,6 +67,7 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace .into_iter() @@ -77,6 +78,7 @@ pub(crate) fn run( let expression_width = backend.get_backend_info_or_default(); let (compiled_programs, compiled_contracts) = compile_workspace( &workspace_file_manager, + &parsed_files, &workspace, &binary_packages, &contract_packages, diff --git a/tooling/nargo_cli/src/cli/prove_cmd.rs b/tooling/nargo_cli/src/cli/prove_cmd.rs index 167ab541bc5..fb565b691b6 100644 --- a/tooling/nargo_cli/src/cli/prove_cmd.rs +++ b/tooling/nargo_cli/src/cli/prove_cmd.rs @@ -1,8 +1,8 @@ use clap::Args; use nargo::constants::{PROVER_INPUT_FILE, VERIFIER_INPUT_FILE}; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::package::Package; use nargo::workspace::Workspace; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::Format; use noirc_driver::{ @@ -66,11 +66,13 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let expression_width = backend.get_backend_info()?; for package in &workspace { let program = compile_bin_package( &workspace_file_manager, + &parsed_files, &workspace, package, &args.compile_options, diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 69f03b49cbd..5db842609e5 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -8,11 +8,14 @@ use nargo::{ insert_all_files_for_workspace_into_file_manager, ops::{run_test, TestStatus}, package::Package, - prepare_package, + parse_all, prepare_package, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; -use noirc_frontend::{graph::CrateName, hir::FunctionNameMatch}; +use noirc_frontend::{ + graph::CrateName, + hir::{FunctionNameMatch, ParsedFiles}, +}; use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; use crate::{backends::Backend, cli::check_cmd::check_crate_and_report_errors, errors::CliError}; @@ -66,6 +69,7 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let pattern = match &args.test_name { Some(name) => { @@ -84,6 +88,7 @@ pub(crate) fn run( // TODO: We should run the whole suite even if there are failures in a package run_tests( &workspace_file_manager, + &parsed_files, &blackbox_solver, package, pattern, @@ -96,8 +101,10 @@ pub(crate) fn run( Ok(()) } +#[allow(clippy::too_many_arguments)] fn run_tests( file_manager: &FileManager, + parsed_files: &ParsedFiles, blackbox_solver: &S, package: &Package, fn_name: FunctionNameMatch, @@ -105,7 +112,7 @@ fn run_tests( foreign_call_resolver_url: Option<&str>, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let (mut context, crate_id) = prepare_package(file_manager, package); + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); check_crate_and_report_errors( &mut context, crate_id, diff --git a/tooling/nargo_cli/src/cli/verify_cmd.rs b/tooling/nargo_cli/src/cli/verify_cmd.rs index 86d5e774cbe..3d6aeb2e4d5 100644 --- a/tooling/nargo_cli/src/cli/verify_cmd.rs +++ b/tooling/nargo_cli/src/cli/verify_cmd.rs @@ -7,9 +7,9 @@ use crate::{backends::Backend, errors::CliError}; use clap::Args; use nargo::constants::{PROOF_EXT, VERIFIER_INPUT_FILE}; -use nargo::insert_all_files_for_workspace_into_file_manager; use nargo::package::Package; use nargo::workspace::Workspace; +use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::Format; use noirc_driver::{ @@ -53,11 +53,13 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let expression_width = backend.get_backend_info()?; for package in &workspace { let program = compile_bin_package( &workspace_file_manager, + &parsed_files, &workspace, package, &args.compile_options, From c80ddfed900da080e9d7a2d56e60f67895bd0f44 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Jan 2024 13:27:34 +0000 Subject: [PATCH 02/10] refactor: extract to fns and add comments --- .../noirc_driver/tests/stdlib_warnings.rs | 2 +- compiler/noirc_frontend/src/hir/mod.rs | 10 +++---- compiler/wasm/src/compile.rs | 28 ++++++++----------- compiler/wasm/src/compile_new.rs | 20 ++++--------- tooling/lsp/src/requests/tests.rs | 5 +++- 5 files changed, 26 insertions(+), 39 deletions(-) diff --git a/compiler/noirc_driver/tests/stdlib_warnings.rs b/compiler/noirc_driver/tests/stdlib_warnings.rs index b2e6e8db3bf..1cf7713684c 100644 --- a/compiler/noirc_driver/tests/stdlib_warnings.rs +++ b/compiler/noirc_driver/tests/stdlib_warnings.rs @@ -7,7 +7,7 @@ use noirc_frontend::hir::{def_map::parse_file, Context}; fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings> { // We use a minimal source file so that if stdlib produces warnings then we can expect these warnings to _always_ // be emitted. - let source: &str = "fn main() {}"; + let source = "fn main() {}"; let root = Path::new(""); let file_name = Path::new("main.nr"); diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index cab923be34d..2124b5281f4 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -35,6 +35,9 @@ pub struct Context<'file_manager, 'parsed_files> { /// This is used to issue an error if a second `mod foo;` is declared to the same file. pub visited_files: BTreeMap, + // A map of all parsed files. + // Same as the file manager, we take ownership of the parsed files in the WASM context. + // Parsed files is also read only. pub parsed_files: Cow<'parsed_files, ParsedFiles>, } @@ -45,7 +48,7 @@ pub enum FunctionNameMatch<'a> { Contains(&'a str), } -impl<'file_manager, 'parsed_files> Context<'file_manager, 'parsed_files> { +impl Context<'_, '_> { pub fn new(file_manager: FileManager, parsed_files: ParsedFiles) -> Context<'static, 'static> { Context { def_interner: NodeInterner::default(), @@ -57,7 +60,7 @@ impl<'file_manager, 'parsed_files> Context<'file_manager, 'parsed_files> { } } - pub fn from_ref_file_manager( + pub fn from_ref_file_manager<'file_manager, 'parsed_files>( file_manager: &'file_manager FileManager, parsed_files: &'parsed_files ParsedFiles, ) -> Context<'file_manager, 'parsed_files> { @@ -72,9 +75,6 @@ impl<'file_manager, 'parsed_files> Context<'file_manager, 'parsed_files> { } pub fn parsed_file_results(&self, file_id: fm::FileId) -> (ParsedModule, Vec) { - // TODO: we could make it parse the file if it is not in the cache - // - // TODO: Check if we can return a reference here instead of cloning. self.parsed_files.get(&file_id).expect("noir file wasn't parsed").clone() } diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 26895c92583..3336ad77f8c 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -13,7 +13,7 @@ use noirc_driver::{ }; use noirc_frontend::{ graph::{CrateId, CrateName}, - hir::{def_map::parse_file, Context}, + hir::{def_map::parse_file, Context, ParsedFiles}, }; use serde::Deserialize; use std::{collections::HashMap, path::Path}; @@ -147,6 +147,10 @@ impl PathToFileSourceMap { } } +pub(crate) fn parse_all(fm: &FileManager) -> ParsedFiles { + fm.as_file_map().all_file_ids().map(|&file_id| (file_id, parse_file(fm, file_id))).collect() +} + pub enum CompileResult { Contract { contract: ContractArtifact, debug: DebugArtifact }, Program { program: ProgramArtifact, debug: DebugArtifact }, @@ -169,12 +173,7 @@ pub fn compile( }; let fm = file_manager_with_source_map(file_source_map); - let parsed_files = fm - .as_file_map() - .all_file_ids() - .map(|&file_id| (file_id, parse_file(&fm, file_id))) - .collect(); - + let parsed_files = parse_all(&fm); let mut context = Context::new(fm, parsed_files); let path = Path::new(&entry_point); @@ -308,25 +307,20 @@ pub(crate) fn generate_contract_artifact(contract: CompiledContract) -> CompileR #[cfg(test)] mod test { use noirc_driver::prepare_crate; - use noirc_frontend::{ - graph::CrateName, - hir::{def_map::parse_file, Context}, - }; + use noirc_frontend::{graph::CrateName, hir::Context}; use crate::compile::PathToFileSourceMap; - use super::{file_manager_with_source_map, process_dependency_graph, DependencyGraph}; + use super::{ + file_manager_with_source_map, parse_all, process_dependency_graph, DependencyGraph, + }; use std::{collections::HashMap, path::Path}; fn setup_test_context(source_map: PathToFileSourceMap) -> Context<'static, 'static> { let mut fm = file_manager_with_source_map(source_map); // Add this due to us calling prepare_crate on "/main.nr" below fm.add_file_with_source(Path::new("/main.nr"), "fn foo() {}".to_string()); - let parsed_files = fm - .as_file_map() - .all_file_ids() - .map(|&file_id| (file_id, parse_file(&fm, file_id))) - .collect(); + let parsed_files = parse_all(&fm); let mut context = Context::new(fm, parsed_files); prepare_crate(&mut context, Path::new("/main.nr")); diff --git a/compiler/wasm/src/compile_new.rs b/compiler/wasm/src/compile_new.rs index c86f0662653..6476f6d29bc 100644 --- a/compiler/wasm/src/compile_new.rs +++ b/compiler/wasm/src/compile_new.rs @@ -1,12 +1,11 @@ use crate::compile::{ - file_manager_with_source_map, generate_contract_artifact, generate_program_artifact, + file_manager_with_source_map, generate_contract_artifact, generate_program_artifact, parse_all, JsCompileResult, PathToFileSourceMap, }; use crate::errors::{CompileError, JsCompileError}; use noirc_driver::{ add_dep, compile_contract, compile_main, prepare_crate, prepare_dependency, CompileOptions, }; -use noirc_frontend::hir::def_map::parse_file; use noirc_frontend::{ graph::{CrateId, CrateName}, hir::Context, @@ -35,12 +34,7 @@ impl CompilerContext { console_error_panic_hook::set_once(); let fm = file_manager_with_source_map(source_map); - - let parsed_files = fm - .as_file_map() - .all_file_ids() - .map(|&file_id| (file_id, parse_file(&fm, file_id))) - .collect(); + let parsed_files = parse_all(&fm); CompilerContext { context: Context::new(fm, parsed_files) } } @@ -237,9 +231,9 @@ pub fn compile_( #[cfg(test)] mod test { use noirc_driver::prepare_crate; - use noirc_frontend::hir::{def_map::parse_file, Context}; + use noirc_frontend::hir::Context; - use crate::compile::{file_manager_with_source_map, PathToFileSourceMap}; + use crate::compile::{file_manager_with_source_map, parse_all, PathToFileSourceMap}; use std::path::Path; @@ -249,11 +243,7 @@ mod test { let mut fm = file_manager_with_source_map(source_map); // Add this due to us calling prepare_crate on "/main.nr" below fm.add_file_with_source(Path::new("/main.nr"), "fn foo() {}".to_string()); - let parsed_files = fm - .as_file_map() - .all_file_ids() - .map(|&file_id| (file_id, parse_file(&fm, file_id))) - .collect(); + let parsed_files = parse_all(&fm); let mut context = Context::new(fm, parsed_files); prepare_crate(&mut context, Path::new("/main.nr")); diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index c9af1443cef..75eb5ae9906 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -52,7 +52,10 @@ fn on_tests_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files: std::collections::HashMap)> = parse_all(&workspace_file_manager); + let parsed_files: std::collections::HashMap< + fm::FileId, + (noirc_frontend::ParsedModule, Vec), + > = parse_all(&workspace_file_manager); let package_tests: Vec<_> = workspace .into_iter() From 682b05a9aa48428712ccb4035648295ae75db3c9 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Jan 2024 13:51:26 +0000 Subject: [PATCH 03/10] test: fix test --- compiler/noirc_driver/tests/stdlib_warnings.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_driver/tests/stdlib_warnings.rs b/compiler/noirc_driver/tests/stdlib_warnings.rs index 1cf7713684c..069344103b9 100644 --- a/compiler/noirc_driver/tests/stdlib_warnings.rs +++ b/compiler/noirc_driver/tests/stdlib_warnings.rs @@ -12,12 +12,16 @@ fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings> let root = Path::new(""); let file_name = Path::new("main.nr"); let mut file_manager = file_manager_with_stdlib(root); - let file_id = file_manager.add_file_with_source(file_name, source.to_owned()).expect( + file_manager.add_file_with_source(file_name, source.to_owned()).expect( "Adding source buffer to file manager should never fail when file manager is empty", ); - let parsed_source = parse_file(&file_manager, file_id); + let parsed_files = file_manager + .as_file_map() + .all_file_ids() + .map(|&file_id| (file_id, parse_file(&file_manager, file_id))) + .collect(); - let mut context = Context::new(file_manager, HashMap::from([(file_id, parsed_source)])); + let mut context = Context::new(file_manager, parsed_files); let root_crate_id = prepare_crate(&mut context, file_name); let ((), warnings) = noirc_driver::check_crate(&mut context, root_crate_id, false, false)?; From 59f4112e1aa6fa0a0e377f9145b6aaab3de98031 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Jan 2024 13:55:42 +0000 Subject: [PATCH 04/10] clippy --- compiler/noirc_driver/tests/stdlib_warnings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_driver/tests/stdlib_warnings.rs b/compiler/noirc_driver/tests/stdlib_warnings.rs index 069344103b9..6f437621123 100644 --- a/compiler/noirc_driver/tests/stdlib_warnings.rs +++ b/compiler/noirc_driver/tests/stdlib_warnings.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, path::Path}; +use std::path::Path; use noirc_driver::{file_manager_with_stdlib, prepare_crate, ErrorsAndWarnings}; use noirc_frontend::hir::{def_map::parse_file, Context}; From 85c59ed32192e39582013fb0396d64e42e8a6bf2 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Jan 2024 14:18:13 +0000 Subject: [PATCH 05/10] feat: use par_bridge --- tooling/nargo/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 731135bec65..0fdff8b202f 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -97,9 +97,10 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } pub fn parse_all(file_manager: &FileManager) -> ParsedFiles { - let file_ids: Vec<_> = file_manager.as_file_map().all_file_ids().collect(); - file_ids - .into_par_iter() + file_manager + .as_file_map() + .all_file_ids() + .par_bridge() .filter(|&&file_id| { let file_path = file_manager.path(file_id).expect("expected file to exist"); let file_extension = From e5671aaa3df6d67618ee97cee0db7405684cacfc Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Jan 2024 14:20:53 +0000 Subject: [PATCH 06/10] style: remove explicity type --- tooling/lsp/src/requests/tests.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index 75eb5ae9906..20098685a90 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -52,10 +52,7 @@ fn on_tests_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files: std::collections::HashMap< - fm::FileId, - (noirc_frontend::ParsedModule, Vec), - > = parse_all(&workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); let package_tests: Vec<_> = workspace .into_iter() From 125b98518af21e48b8ad92f344fa6edae6172d0f Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 18 Jan 2024 10:30:38 +0000 Subject: [PATCH 07/10] feat: Cached LSP parsing --- Cargo.lock | 2 + tooling/lsp/Cargo.toml | 2 + tooling/lsp/src/lib.rs | 97 +++++++++++++++---- tooling/lsp/src/notifications/mod.rs | 9 +- tooling/lsp/src/requests/code_lens_request.rs | 2 +- tooling/lsp/src/requests/goto_declaration.rs | 10 +- tooling/lsp/src/requests/goto_definition.rs | 10 +- tooling/lsp/src/requests/profile_run.rs | 9 +- tooling/lsp/src/requests/test_run.rs | 7 +- tooling/lsp/src/requests/tests.rs | 6 +- 10 files changed, 110 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 438d74b1b2d..1a4f2f05e13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2841,6 +2841,7 @@ dependencies = [ "async-lsp", "codespan-lsp", "fm", + "fxhash", "lsp-types 0.94.1", "nargo", "nargo_fmt", @@ -2848,6 +2849,7 @@ dependencies = [ "noirc_driver", "noirc_errors", "noirc_frontend", + "rayon", "serde", "serde_json", "serde_with", diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index 6371bcbac19..750e85694e2 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -25,6 +25,8 @@ async-lsp = { workspace = true, features = ["omni-trait"] } serde_with = "3.2.0" thiserror.workspace = true fm.workspace = true +rayon = "1.8.0" +fxhash.workspace = true [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies] wasm-bindgen.workspace = true diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index c0b944b2c73..1c115cee48c 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -17,16 +17,20 @@ use async_lsp::{ router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, LspService, ResponseError, }; -use fm::codespan_files as files; +use fm::{codespan_files as files, FileManager}; +use fxhash::FxHashSet; use lsp_types::CodeLens; -use nargo::{parse_all, workspace::Workspace}; +use nargo::workspace::Workspace; use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{file_manager_with_stdlib, prepare_crate, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::{ graph::{CrateId, CrateName}, - hir::{Context, FunctionNameMatch}, + hir::{def_map::parse_file, Context, FunctionNameMatch, ParsedFiles}, node_interner::NodeInterner, + parser::ParserError, + ParsedModule, }; +use rayon::prelude::*; use notifications::{ on_did_change_configuration, on_did_change_text_document, on_did_close_text_document, @@ -64,6 +68,7 @@ pub struct LspState { input_files: HashMap, cached_lenses: HashMap>, cached_definitions: HashMap, + cached_parsed_files: HashMap))>, } impl LspState { @@ -76,6 +81,7 @@ impl LspState { cached_lenses: HashMap::new(), cached_definitions: HashMap::new(), open_documents_count: 0, + cached_parsed_files: HashMap::new(), } } } @@ -225,14 +231,14 @@ pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result (Context<'static, 'static>, CrateId) { +fn prepare_source(source: String, state: &mut LspState) -> (Context<'static, 'static>, CrateId) { let root = Path::new(""); let file_name = Path::new("main.nr"); let mut file_manager = file_manager_with_stdlib(root); file_manager.add_file_with_source(file_name, source).expect( "Adding source buffer to file manager should never fail when file manager is empty", ); - let parsed_files = parse_all(&file_manager); + let parsed_files = parse_diff(&file_manager, state); let mut context = Context::new(file_manager, parsed_files); let root_crate_id = prepare_crate(&mut context, file_name); @@ -240,18 +246,73 @@ fn prepare_source(source: String) -> (Context<'static, 'static>, CrateId) { (context, root_crate_id) } -#[test] -fn prepare_package_from_source_string() { - let source = r#" - fn main() { - let x = 1; - let y = 2; - let z = x + y; - } - "#; +fn parse_diff(file_manager: &FileManager, state: &mut LspState) -> ParsedFiles { + let noir_file_hashes: Vec<_> = file_manager + .as_file_map() + .all_file_ids() + .par_bridge() + .filter_map(|&file_id| { + let file_path = file_manager.path(file_id).expect("expected file to exist"); + let file_extension = + file_path.extension().expect("expected all file paths to have an extension"); + if file_extension == "nr" { + Some(( + file_id, + file_path.to_path_buf(), + fxhash::hash(file_manager.fetch_file(file_id).expect("file must exist")), + )) + } else { + None + } + }) + .collect(); + + let cache_hits: Vec<_> = noir_file_hashes + .par_iter() + .filter_map(|(file_id, file_path, current_hash)| { + let cached_version = state.cached_parsed_files.get(file_path); + if let Some((hash, cached_parsing)) = cached_version { + if hash == current_hash { + return Some((*file_id, cached_parsing.clone())); + } + } + None + }) + .collect(); - let (mut context, crate_id) = crate::prepare_source(source.to_string()); - let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false); - let main_func_id = context.get_main_function(&crate_id); - assert!(main_func_id.is_some()); + let cache_hits_ids: FxHashSet<_> = cache_hits.iter().map(|(file_id, _)| *file_id).collect(); + + let cache_misses: Vec<_> = noir_file_hashes + .into_par_iter() + .filter(|(id, _, _)| !cache_hits_ids.contains(id)) + .map(|(file_id, path, hash)| (file_id, path, hash, parse_file(file_manager, file_id))) + .collect(); + + cache_misses.iter().for_each(|(_, path, hash, parse_results)| { + state.cached_parsed_files.insert(path.clone(), (*hash, parse_results.clone())); + }); + + cache_misses + .into_iter() + .map(|(id, _, _, parse_results)| (id, parse_results)) + .chain(cache_hits.into_iter()) + .collect() } + +// #[test] +// fn prepare_package_from_source_string() { +// let source = r#" +// fn main() { +// let x = 1; +// let y = 2; +// let z = x + y; +// } +// "#; + +// let state = LspState::default(); + +// let (mut context, crate_id) = crate::prepare_source(source.to_string(), &mut state); +// let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false); +// let main_func_id = context.get_main_function(&crate_id); +// assert!(main_func_id.is_some()); +// } diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 769cfbdfed7..355bb7832c4 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -1,7 +1,7 @@ use std::ops::ControlFlow; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all, prepare_package}; +use nargo::{insert_all_files_for_workspace_into_file_manager, prepare_package}; use noirc_driver::{check_crate, file_manager_with_stdlib}; use noirc_errors::{DiagnosticKind, FileDiagnostic}; @@ -13,7 +13,7 @@ use crate::types::{ }; use crate::{ - byte_span_to_range, get_package_tests_in_crate, prepare_source, + byte_span_to_range, get_package_tests_in_crate, parse_diff, prepare_source, resolve_workspace_for_source_path, LspState, }; @@ -55,7 +55,7 @@ pub(super) fn on_did_change_text_document( let text = params.content_changes.into_iter().next().unwrap().text; state.input_files.insert(params.text_document.uri.to_string(), text.clone()); - let (mut context, crate_id) = prepare_source(text); + let (mut context, crate_id) = prepare_source(text, state); let _ = check_crate(&mut context, crate_id, false, false); let workspace = match resolve_workspace_for_source_path( @@ -130,7 +130,8 @@ fn process_noir_document( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + + let parsed_files = parse_diff(&workspace_file_manager, state); let diagnostics: Vec<_> = workspace .into_iter() diff --git a/tooling/lsp/src/requests/code_lens_request.rs b/tooling/lsp/src/requests/code_lens_request.rs index b16c19457f0..893ba33d845 100644 --- a/tooling/lsp/src/requests/code_lens_request.rs +++ b/tooling/lsp/src/requests/code_lens_request.rs @@ -64,7 +64,7 @@ fn on_code_lens_request_inner( let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap(); let package = workspace.members.first().unwrap(); - let (mut context, crate_id) = prepare_source(source_string); + let (mut context, crate_id) = prepare_source(source_string, state); // We ignore the warnings and errors produced by compilation for producing code lenses // because we can still get the test functions even if compilation fails let _ = check_crate(&mut context, crate_id, false, false); diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 59636192d87..8e6d519b895 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -1,13 +1,13 @@ use std::future::{self, Future}; -use crate::resolve_workspace_for_source_path; use crate::types::GotoDeclarationResult; use crate::LspState; +use crate::{parse_diff, resolve_workspace_for_source_path}; use async_lsp::{ErrorCode, ResponseError}; use lsp_types::request::{GotoDeclarationParams, GotoDeclarationResponse}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; +use nargo::insert_all_files_for_workspace_into_file_manager; use noirc_driver::file_manager_with_stdlib; use super::{position_to_byte_index, to_lsp_location}; @@ -21,7 +21,7 @@ pub(crate) fn on_goto_declaration_request( } fn on_goto_definition_inner( - _state: &mut LspState, + state: &mut LspState, params: GotoDeclarationParams, ) -> Result { let file_path = @@ -36,13 +36,13 @@ fn on_goto_definition_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let parsed_files = parse_diff(&workspace_file_manager, state); let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, &parsed_files, package); let interner; - if let Some(def_interner) = _state.cached_definitions.get(&package_root_path) { + if let Some(def_interner) = state.cached_definitions.get(&package_root_path) { interner = def_interner; } else { // We ignore the warnings and errors produced by compilation while resolving the definition diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index fa043365236..a4538897aef 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -1,11 +1,11 @@ use std::future::{self, Future}; -use crate::resolve_workspace_for_source_path; +use crate::{parse_diff, resolve_workspace_for_source_path}; use crate::{types::GotoDefinitionResult, LspState}; use async_lsp::{ErrorCode, ResponseError}; use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; +use nargo::insert_all_files_for_workspace_into_file_manager; use noirc_driver::file_manager_with_stdlib; use super::{position_to_byte_index, to_lsp_location}; @@ -19,7 +19,7 @@ pub(crate) fn on_goto_definition_request( } fn on_goto_definition_inner( - _state: &mut LspState, + state: &mut LspState, params: GotoDefinitionParams, ) -> Result { let file_path = @@ -34,13 +34,13 @@ fn on_goto_definition_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let parsed_files = parse_diff(&workspace_file_manager, state); let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, &parsed_files, package); let interner; - if let Some(def_interner) = _state.cached_definitions.get(&package_root_path) { + if let Some(def_interner) = state.cached_definitions.get(&package_root_path) { interner = def_interner; } else { // We ignore the warnings and errors produced by compilation while resolving the definition diff --git a/tooling/lsp/src/requests/profile_run.rs b/tooling/lsp/src/requests/profile_run.rs index 65e6a936e20..ff4c4270520 100644 --- a/tooling/lsp/src/requests/profile_run.rs +++ b/tooling/lsp/src/requests/profile_run.rs @@ -5,9 +5,7 @@ use std::{ use acvm::ExpressionWidth; use async_lsp::{ErrorCode, ResponseError}; -use nargo::{ - artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager, parse_all, -}; +use nargo::{artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ file_manager_with_stdlib, CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING, @@ -15,6 +13,7 @@ use noirc_driver::{ use noirc_errors::{debug_info::OpCodesCount, Location}; use crate::{ + parse_diff, types::{NargoProfileRunParams, NargoProfileRunResult}, LspState, }; @@ -28,7 +27,7 @@ pub(crate) fn on_profile_run_request( } fn on_profile_run_request_inner( - state: &LspState, + state: &mut LspState, params: NargoProfileRunParams, ) -> Result { let root_path = state.root_path.as_deref().ok_or_else(|| { @@ -54,7 +53,7 @@ fn on_profile_run_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let parsed_files = parse_diff(&workspace_file_manager, state); // Since we filtered on crate name, this should be the only item in the iterator match workspace.into_iter().next() { diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 8da4d74a654..135090d7ed9 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -4,7 +4,7 @@ use async_lsp::{ErrorCode, ResponseError}; use nargo::{ insert_all_files_for_workspace_into_file_manager, ops::{run_test, TestStatus}, - parse_all, prepare_package, + prepare_package, }; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ @@ -13,6 +13,7 @@ use noirc_driver::{ use noirc_frontend::hir::FunctionNameMatch; use crate::{ + parse_diff, types::{NargoTestRunParams, NargoTestRunResult}, LspState, }; @@ -25,7 +26,7 @@ pub(crate) fn on_test_run_request( } fn on_test_run_request_inner( - state: &LspState, + state: &mut LspState, params: NargoTestRunParams, ) -> Result { let root_path = state.root_path.as_deref().ok_or_else(|| { @@ -52,7 +53,7 @@ fn on_test_run_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let parsed_files = parse_diff(&workspace_file_manager, state); // Since we filtered on crate name, this should be the only item in the iterator match workspace.into_iter().next() { diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index 20098685a90..5b78fcc65c3 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -2,12 +2,12 @@ use std::future::{self, Future}; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; use lsp_types::{LogMessageParams, MessageType}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all, prepare_package}; +use nargo::{insert_all_files_for_workspace_into_file_manager, prepare_package}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{check_crate, file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING}; use crate::{ - get_package_tests_in_crate, + get_package_tests_in_crate, parse_diff, types::{NargoPackageTests, NargoTestsParams, NargoTestsResult}, LspState, }; @@ -52,7 +52,7 @@ fn on_tests_request_inner( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let parsed_files = parse_diff(&workspace_file_manager, state); let package_tests: Vec<_> = workspace .into_iter() From 271ac482efed78e04bcd47ceea83003e376a3f03 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 18 Jan 2024 13:37:14 +0000 Subject: [PATCH 08/10] feat: add parsing cache option --- tooling/lsp/src/lib.rs | 106 +++++++++++++++++--------------- tooling/lsp/src/requests/mod.rs | 14 ++++- 2 files changed, 68 insertions(+), 52 deletions(-) diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 1c115cee48c..68de6428dbd 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -20,7 +20,7 @@ use async_lsp::{ use fm::{codespan_files as files, FileManager}; use fxhash::FxHashSet; use lsp_types::CodeLens; -use nargo::workspace::Workspace; +use nargo::{parse_all, workspace::Workspace}; use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{file_manager_with_stdlib, prepare_crate, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::{ @@ -69,6 +69,7 @@ pub struct LspState { cached_lenses: HashMap>, cached_definitions: HashMap, cached_parsed_files: HashMap))>, + parsing_cache_enabled: bool, } impl LspState { @@ -82,6 +83,7 @@ impl LspState { cached_definitions: HashMap::new(), open_documents_count: 0, cached_parsed_files: HashMap::new(), + parsing_cache_enabled: true, } } } @@ -247,56 +249,60 @@ fn prepare_source(source: String, state: &mut LspState) -> (Context<'static, 'st } fn parse_diff(file_manager: &FileManager, state: &mut LspState) -> ParsedFiles { - let noir_file_hashes: Vec<_> = file_manager - .as_file_map() - .all_file_ids() - .par_bridge() - .filter_map(|&file_id| { - let file_path = file_manager.path(file_id).expect("expected file to exist"); - let file_extension = - file_path.extension().expect("expected all file paths to have an extension"); - if file_extension == "nr" { - Some(( - file_id, - file_path.to_path_buf(), - fxhash::hash(file_manager.fetch_file(file_id).expect("file must exist")), - )) - } else { - None - } - }) - .collect(); - - let cache_hits: Vec<_> = noir_file_hashes - .par_iter() - .filter_map(|(file_id, file_path, current_hash)| { - let cached_version = state.cached_parsed_files.get(file_path); - if let Some((hash, cached_parsing)) = cached_version { - if hash == current_hash { - return Some((*file_id, cached_parsing.clone())); + if state.parsing_cache_enabled { + let noir_file_hashes: Vec<_> = file_manager + .as_file_map() + .all_file_ids() + .par_bridge() + .filter_map(|&file_id| { + let file_path = file_manager.path(file_id).expect("expected file to exist"); + let file_extension = + file_path.extension().expect("expected all file paths to have an extension"); + if file_extension == "nr" { + Some(( + file_id, + file_path.to_path_buf(), + fxhash::hash(file_manager.fetch_file(file_id).expect("file must exist")), + )) + } else { + None } - } - None - }) - .collect(); - - let cache_hits_ids: FxHashSet<_> = cache_hits.iter().map(|(file_id, _)| *file_id).collect(); - - let cache_misses: Vec<_> = noir_file_hashes - .into_par_iter() - .filter(|(id, _, _)| !cache_hits_ids.contains(id)) - .map(|(file_id, path, hash)| (file_id, path, hash, parse_file(file_manager, file_id))) - .collect(); - - cache_misses.iter().for_each(|(_, path, hash, parse_results)| { - state.cached_parsed_files.insert(path.clone(), (*hash, parse_results.clone())); - }); - - cache_misses - .into_iter() - .map(|(id, _, _, parse_results)| (id, parse_results)) - .chain(cache_hits.into_iter()) - .collect() + }) + .collect(); + + let cache_hits: Vec<_> = noir_file_hashes + .par_iter() + .filter_map(|(file_id, file_path, current_hash)| { + let cached_version = state.cached_parsed_files.get(file_path); + if let Some((hash, cached_parsing)) = cached_version { + if hash == current_hash { + return Some((*file_id, cached_parsing.clone())); + } + } + None + }) + .collect(); + + let cache_hits_ids: FxHashSet<_> = cache_hits.iter().map(|(file_id, _)| *file_id).collect(); + + let cache_misses: Vec<_> = noir_file_hashes + .into_par_iter() + .filter(|(id, _, _)| !cache_hits_ids.contains(id)) + .map(|(file_id, path, hash)| (file_id, path, hash, parse_file(file_manager, file_id))) + .collect(); + + cache_misses.iter().for_each(|(_, path, hash, parse_results)| { + state.cached_parsed_files.insert(path.clone(), (*hash, parse_results.clone())); + }); + + cache_misses + .into_iter() + .map(|(id, _, _, parse_results)| (id, parse_results)) + .chain(cache_hits.into_iter()) + .collect() + } else { + parse_all(file_manager) + } } // #[test] diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 9a4738e1985..18c2cfd24ae 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -46,15 +46,25 @@ struct LspInitializationOptions { /// By default this will be set to true (enabled). #[serde(rename = "enableCodeLens", default = "default_enable_code_lens")] enable_code_lens: bool, + + #[serde(rename = "enableParsingCache", default = "default_enable_parsing_cache")] + enable_parsing_cache: bool, } fn default_enable_code_lens() -> bool { true } +fn default_enable_parsing_cache() -> bool { + true +} + impl Default for LspInitializationOptions { fn default() -> Self { - Self { enable_code_lens: default_enable_code_lens() } + Self { + enable_code_lens: default_enable_code_lens(), + enable_parsing_cache: default_enable_parsing_cache(), + } } } @@ -63,11 +73,11 @@ pub(crate) fn on_initialize( params: InitializeParams, ) -> impl Future> { state.root_path = params.root_uri.and_then(|root_uri| root_uri.to_file_path().ok()); - let initialization_options: LspInitializationOptions = params .initialization_options .and_then(|value| serde_json::from_value(value).ok()) .unwrap_or_default(); + state.parsing_cache_enabled = initialization_options.enable_parsing_cache; async move { let text_document_sync = TextDocumentSyncCapability::Kind(TextDocumentSyncKind::FULL); From 3e9641c5ffe0d39ad001e36d63698003d4d9ec62 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 18 Jan 2024 13:41:19 +0000 Subject: [PATCH 09/10] clippy --- tooling/lsp/src/notifications/mod.rs | 3 +-- tooling/lsp/src/requests/goto_declaration.rs | 2 +- tooling/lsp/src/requests/goto_definition.rs | 2 +- tooling/lsp/src/requests/profile_run.rs | 4 +--- tooling/lsp/src/requests/test_run.rs | 2 +- tooling/lsp/src/requests/tests.rs | 2 +- 6 files changed, 6 insertions(+), 9 deletions(-) diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 23b94b99df1..355bb7832c4 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -1,7 +1,7 @@ use std::ops::ControlFlow; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all, prepare_package}; +use nargo::{insert_all_files_for_workspace_into_file_manager, prepare_package}; use noirc_driver::{check_crate, file_manager_with_stdlib}; use noirc_errors::{DiagnosticKind, FileDiagnostic}; @@ -130,7 +130,6 @@ fn process_noir_document( let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); let parsed_files = parse_diff(&workspace_file_manager, state); diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index d80f25e9666..8e6d519b895 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -7,7 +7,7 @@ use async_lsp::{ErrorCode, ResponseError}; use lsp_types::request::{GotoDeclarationParams, GotoDeclarationResponse}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; +use nargo::insert_all_files_for_workspace_into_file_manager; use noirc_driver::file_manager_with_stdlib; use super::{position_to_byte_index, to_lsp_location}; diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index de559f550ce..88bb667f2e8 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -6,7 +6,7 @@ use async_lsp::{ErrorCode, ResponseError}; use lsp_types::request::GotoTypeDefinitionParams; use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; +use nargo::insert_all_files_for_workspace_into_file_manager; use noirc_driver::file_manager_with_stdlib; use super::{position_to_byte_index, to_lsp_location}; diff --git a/tooling/lsp/src/requests/profile_run.rs b/tooling/lsp/src/requests/profile_run.rs index 36f16bf1133..ff4c4270520 100644 --- a/tooling/lsp/src/requests/profile_run.rs +++ b/tooling/lsp/src/requests/profile_run.rs @@ -5,9 +5,7 @@ use std::{ use acvm::ExpressionWidth; use async_lsp::{ErrorCode, ResponseError}; -use nargo::{ - artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager, parse_all, -}; +use nargo::{artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ file_manager_with_stdlib, CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING, diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index a768b3a8692..135090d7ed9 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -4,7 +4,7 @@ use async_lsp::{ErrorCode, ResponseError}; use nargo::{ insert_all_files_for_workspace_into_file_manager, ops::{run_test, TestStatus}, - parse_all, prepare_package, + prepare_package, }; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index b5aba5bcaa1..5b78fcc65c3 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -2,7 +2,7 @@ use std::future::{self, Future}; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; use lsp_types::{LogMessageParams, MessageType}; -use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all, prepare_package}; +use nargo::{insert_all_files_for_workspace_into_file_manager, prepare_package}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{check_crate, file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING}; From 22b22e4ba034ca3a80242750a283bde5ac0b2b1f Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 18 Jan 2024 17:02:20 +0000 Subject: [PATCH 10/10] test: restore commented out test --- tooling/lsp/src/lib.rs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index b230b1bdbb4..b64fc474b0b 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -307,20 +307,21 @@ fn parse_diff(file_manager: &FileManager, state: &mut LspState) -> ParsedFiles { } } -// #[test] -// fn prepare_package_from_source_string() { -// let source = r#" -// fn main() { -// let x = 1; -// let y = 2; -// let z = x + y; -// } -// "#; - -// let state = LspState::default(); - -// let (mut context, crate_id) = crate::prepare_source(source.to_string(), &mut state); -// let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false); -// let main_func_id = context.get_main_function(&crate_id); -// assert!(main_func_id.is_some()); -// } +#[test] +fn prepare_package_from_source_string() { + let source = r#" + fn main() { + let x = 1; + let y = 2; + let z = x + y; + } + "#; + + let client = ClientSocket::new_closed(); + let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver); + + let (mut context, crate_id) = crate::prepare_source(source.to_string(), &mut state); + let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false); + let main_func_id = context.get_main_function(&crate_id); + assert!(main_func_id.is_some()); +}