Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Push FileManager population up to nargo_cli #3844

Merged
merged 37 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4472e4a
make Context have a Cow of FileManager
kevaundray Dec 17, 2023
1bf00ac
prepare_package now takes a reference to FileManager
kevaundray Dec 17, 2023
8147bce
annotate noir_wasm context with 'static
kevaundray Dec 17, 2023
9d4ef7e
compile_ methods take a reference to file manager
kevaundray Dec 17, 2023
4a95500
modify lsp methods to now take a reference to file manager
kevaundray Dec 17, 2023
7fc4779
modify cmd modules to accomodate the reference to file manager
kevaundray Dec 17, 2023
76eb80a
add a method to add stdlib source into file manager
kevaundray Dec 17, 2023
d0877d6
add lifetime
kevaundray Dec 17, 2023
77a3cf5
clippy fix
kevaundray Dec 17, 2023
24277fa
fmt
kevaundray Dec 17, 2023
7552dcc
no longer add stdlib in prepare_crate
kevaundray Dec 17, 2023
cf79724
add `file_manager_with_stdlib` method
kevaundray Dec 17, 2023
7ee031f
noir_wasm: modify to now use file_manager_with_stdlib
kevaundray Dec 17, 2023
1d79e1e
modify all methods to use `file_manager_with_stdlib`
kevaundray Dec 17, 2023
ddf47fe
move `file_manager_with_stdlib` to noirc_driver
kevaundray Dec 17, 2023
dea3e5a
modify code to match
kevaundray Dec 17, 2023
25428ab
add comment
kevaundray Dec 17, 2023
1c010e7
modify comments
kevaundray Dec 17, 2023
c0dd164
typo in comment
kevaundray Dec 17, 2023
3d7e450
add note on assuming stdlib is already added
kevaundray Dec 17, 2023
f016ab0
Merge remote-tracking branch 'origin/master' into kw/make-file-manage…
kevaundray Dec 18, 2023
e37224c
merge fix
kevaundray Dec 18, 2023
8c0edd3
fix merge
kevaundray Dec 18, 2023
159886e
Merge remote-tracking branch 'origin/master' into kw/make-file-manage…
kevaundray Dec 18, 2023
debf5e7
fix merge
kevaundray Dec 18, 2023
c93e81b
Update tooling/nargo/src/ops/compile.rs
kevaundray Dec 19, 2023
5f16f73
Update compiler/wasm/src/compile_new.rs
kevaundray Dec 19, 2023
aa9c612
Update compiler/noirc_driver/src/lib.rs
kevaundray Dec 19, 2023
85542d7
Update compiler/noirc_frontend/src/hir/mod.rs
kevaundray Dec 19, 2023
8afcf9e
Merge branch 'master' into kw/make-file-manager-reference
kevaundray Dec 19, 2023
4d5d130
chore: inline default crate graphs
TomAFrench Dec 19, 2023
0846e69
use workspace root dir
kevaundray Dec 19, 2023
115cdef
use one file manager in fmt_cmd
kevaundray Dec 19, 2023
58d591d
resolve todo
kevaundray Dec 19, 2023
e17918f
fix todo
kevaundray Dec 19, 2023
e89c0c3
fmt
kevaundray Dec 19, 2023
1e9464c
remove extra imports
kevaundray Dec 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl From<&PathBuf> for PathString {
PathString::from(pb.to_owned())
}
}
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct FileMap {
files: SimpleFiles<PathString, String>,
name_to_id: HashMap<PathString, FileId>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{
};

pub const FILE_EXTENSION: &str = "nr";

#[derive(Clone)]
pub struct FileManager {
root: PathBuf,
file_map: FileMap,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId {
// to manually add it here.
let stdlib_paths_with_source = stdlib::stdlib_paths_with_source();
for (path, source) in stdlib_paths_with_source {
context.file_manager.add_file_with_source_canonical_path(Path::new(&path), source);
context.file_manager.to_mut().add_file_with_source_canonical_path(Path::new(&path), source);
}

let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr");
Expand Down
25 changes: 20 additions & 5 deletions compiler/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,21 @@ use crate::node_interner::{FuncId, NodeInterner, StructId};
use def_map::{Contract, CrateDefMap};
use fm::FileManager;
use noirc_errors::Location;
use std::borrow::Cow;
use std::collections::BTreeMap;

use self::def_map::TestFunction;

/// 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 {
pub struct Context<'file_manager> {
pub def_interner: NodeInterner,
pub crate_graph: CrateGraph,
pub(crate) def_maps: BTreeMap<CrateId, CrateDefMap>,
pub file_manager: FileManager,
// TODO: This requires FileManager to be clone, perhaps Context should
// TODO always have a reference to the file manager since it doesn't ever add to it
pub file_manager: Cow<'file_manager, FileManager>,

/// 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.
Expand All @@ -35,14 +38,26 @@ pub enum FunctionNameMatch<'a> {
Contains(&'a str),
}

impl Context {
pub fn new(file_manager: FileManager, crate_graph: CrateGraph) -> Context {
impl Context<'_> {
pub fn new(file_manager: FileManager, crate_graph: CrateGraph) -> Context<'static> {
Context {
def_interner: NodeInterner::default(),
def_maps: BTreeMap::new(),
visited_files: BTreeMap::new(),
crate_graph,
file_manager,
file_manager: Cow::Owned(file_manager),
}
}
pub fn from_ref_file_manager(
file_manager: &FileManager,
crate_graph: CrateGraph,
) -> Context<'_> {
Context {
def_interner: NodeInterner::default(),
def_maps: BTreeMap::new(),
visited_files: BTreeMap::new(),
crate_graph,
file_manager: Cow::Borrowed(file_manager),
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/wasm/src/compile_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use wasm_bindgen::prelude::wasm_bindgen;
/// then the impl block is not picked up in javascript.
#[wasm_bindgen]
pub struct CompilerContext {
context: Context,
context: Context<'static>,
}

#[wasm_bindgen(js_name = "CrateId")]
Expand Down
10 changes: 8 additions & 2 deletions tooling/lsp/src/notifications/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::ops::ControlFlow;
use std::path::Path;

use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use nargo::prepare_package;
use fm::FileManager;
use nargo::{insert_all_files_for_package_into_file_manager, prepare_package};
use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
Expand Down Expand Up @@ -100,10 +102,14 @@ pub(super) fn on_did_save_text_document(
}
};

let mut workspace_file_manager = FileManager::new(Path::new(""));

let diagnostics: Vec<_> = workspace
.into_iter()
.flat_map(|package| -> Vec<Diagnostic> {
let (mut context, crate_id) = prepare_package(package);
insert_all_files_for_package_into_file_manager(package, &mut workspace_file_manager);

let (mut context, crate_id) = prepare_package(&workspace_file_manager, package);

let file_diagnostics = match check_crate(&mut context, crate_id, false, false) {
Ok(((), warnings)) => warnings,
Expand Down
9 changes: 8 additions & 1 deletion tooling/lsp/src/requests/goto_definition.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use std::future::{self, Future};
use std::path::Path;

use crate::{types::GotoDefinitionResult, LspState};
use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use fm::codespan_files::Error;
use fm::FileManager;
use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse, Location};
use lsp_types::{Position, Url};
use nargo::insert_all_files_for_package_into_file_manager;
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;

Expand Down Expand Up @@ -51,8 +54,12 @@ fn on_goto_definition_inner(

let mut definition_position = None;

let mut workspace_file_manager = FileManager::new(Path::new(""));

for package in &workspace {
let (mut context, crate_id) = nargo::prepare_package(package);
insert_all_files_for_package_into_file_manager(package, &mut workspace_file_manager);

let (mut context, crate_id) = nargo::prepare_package(&workspace_file_manager, package);

// We ignore the warnings and errors produced by compilation while resolving the definition
let _ = noirc_driver::check_crate(&mut context, crate_id, false, false);
Expand Down
11 changes: 9 additions & 2 deletions tooling/lsp/src/requests/profile_run.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::{
collections::{BTreeMap, HashMap},
future::{self, Future},
path::Path,
};

use acvm::Language;
use async_lsp::{ErrorCode, ResponseError};
use nargo::artifacts::debug::DebugArtifact;
use nargo::{artifacts::debug::DebugArtifact, insert_all_files_for_package_into_file_manager};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING};
use noirc_errors::{debug_info::OpCodesCount, Location};
Expand All @@ -14,7 +15,7 @@ use crate::{
types::{NargoProfileRunParams, NargoProfileRunResult},
LspState,
};
use fm::FileId;
use fm::{FileId, FileManager};

pub(crate) fn on_profile_run_request(
state: &mut LspState,
Expand Down Expand Up @@ -48,9 +49,14 @@ fn on_profile_run_request_inner(
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut workspace_file_manager = FileManager::new(Path::new(""));

// Since we filtered on crate name, this should be the only item in the iterator
match workspace.into_iter().next() {
Some(_package) => {
// TODO: We should not be adding files for the entire workspace because
// TODO: we are filtering out some packages
insert_all_files_for_package_into_file_manager(_package, &mut workspace_file_manager);
let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace
.into_iter()
.filter(|package| !package.is_library())
Expand All @@ -60,6 +66,7 @@ fn on_profile_run_request_inner(
let np_language = Language::PLONKCSat { width: 3 };

let (compiled_programs, compiled_contracts) = nargo::ops::compile_workspace(
&workspace_file_manager,
&workspace,
&binary_packages,
&contract_packages,
Expand Down
13 changes: 11 additions & 2 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
use std::future::{self, Future};
use std::{
future::{self, Future},
path::Path,
};

use async_lsp::{ErrorCode, ResponseError};
use fm::FileManager;
use nargo::{
insert_all_files_for_package_into_file_manager,
ops::{run_test, TestStatus},
prepare_package,
};
Expand Down Expand Up @@ -47,10 +52,14 @@ fn on_test_run_request_inner(
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut workspace_file_manager = FileManager::new(Path::new(""));

// 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(package);
insert_all_files_for_package_into_file_manager(package, &mut workspace_file_manager);

let (mut context, crate_id) = prepare_package(&workspace_file_manager, package);
if check_crate(&mut context, crate_id, false, false).is_err() {
let result = NargoTestRunResult {
id: params.id.clone(),
Expand Down
13 changes: 10 additions & 3 deletions tooling/lsp/src/requests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
use std::future::{self, Future};
use std::{
future::{self, Future},
path::Path,
};

use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use fm::FileManager;
use lsp_types::{LogMessageParams, MessageType};
use nargo::prepare_package;
use nargo::{insert_all_files_for_package_into_file_manager, prepare_package};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING};

Expand Down Expand Up @@ -50,10 +54,13 @@ fn on_tests_request_inner(
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut workspace_file_manager = FileManager::new(Path::new(""));

let package_tests: Vec<_> = workspace
.into_iter()
.filter_map(|package| {
let (mut context, crate_id) = prepare_package(package);
insert_all_files_for_package_into_file_manager(package, &mut workspace_file_manager);
let (mut context, crate_id) = prepare_package(&workspace_file_manager, 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);
Expand Down
10 changes: 5 additions & 5 deletions tooling/nargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ fn insert_all_files_for_packages_dependencies_into_file_manager(
}
}

pub fn prepare_package(package: &Package) -> (Context, CrateId) {
let mut fm = FileManager::new(&package.root_dir);
insert_all_files_for_package_into_file_manager(package, &mut fm);

pub fn prepare_package<'file_manager>(
file_manager: &'file_manager FileManager,
package: &Package,
) -> (Context<'file_manager>, CrateId) {
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);
let mut context = Context::from_ref_file_manager(file_manager, graph);

let crate_id = prepare_crate(&mut context, &package.entry_path);

Expand Down
43 changes: 24 additions & 19 deletions tooling/nargo/src/ops/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,41 +14,44 @@ use rayon::prelude::*;
///
/// This function will return an error if there are any compilations errors reported.
pub fn compile_workspace(
file_manager: &FileManager,
workspace: &Workspace,
binary_packages: &[Package],
contract_packages: &[Package],
np_language: Language,
compile_options: &CompileOptions,
) -> Result<(Vec<CompiledProgram>, Vec<CompiledContract>), CompileError> {
// TODO: We want to remove this returning of file managers and just have one file manager
// Compile all of the packages in parallel.
let program_results: Vec<(FileManager, CompilationResult<CompiledProgram>)> = binary_packages
let program_results: Vec<CompilationResult<CompiledProgram>> = binary_packages
.par_iter()
.map(|package| {
compile_program(file_manager, workspace, package, compile_options, np_language)
})
.collect();
let contract_results: Vec<CompilationResult<CompiledContract>> = contract_packages
.par_iter()
.map(|package| compile_program(workspace, package, compile_options, np_language))
.map(|package| compile_contract(file_manager, package, compile_options, np_language))
.collect();
let contract_results: Vec<(FileManager, CompilationResult<CompiledContract>)> =
contract_packages
.par_iter()
.map(|package| compile_contract(package, compile_options, np_language))
.collect();

// Report any warnings/errors which were encountered during compilation.
let compiled_programs: Vec<CompiledProgram> = program_results
.into_iter()
.map(|(file_manager, compilation_result)| {
.map(|compilation_result| {
report_errors(
compilation_result,
&file_manager,
file_manager,
compile_options.deny_warnings,
compile_options.silence_warnings,
)
})
.collect::<Result<_, _>>()?;
let compiled_contracts: Vec<CompiledContract> = contract_results
.into_iter()
.map(|(file_manager, compilation_result)| {
.map(|compilation_result| {
report_errors(
compilation_result,
&file_manager,
file_manager,
compile_options.deny_warnings,
compile_options.silence_warnings,
)
Expand All @@ -59,12 +62,13 @@ pub fn compile_workspace(
}

pub fn compile_program(
file_manager: &FileManager,
workspace: &Workspace,
package: &Package,
compile_options: &CompileOptions,
np_language: Language,
) -> (FileManager, CompilationResult<CompiledProgram>) {
let (mut context, crate_id) = prepare_package(package);
) -> CompilationResult<CompiledProgram> {
let (mut context, crate_id) = prepare_package(file_manager, package);

let program_artifact_path = workspace.package_build_path(package);
let mut debug_artifact_path = program_artifact_path.clone();
Expand All @@ -74,33 +78,34 @@ pub fn compile_program(
match noirc_driver::compile_main(&mut context, crate_id, compile_options, None, true) {
Ok(program_and_warnings) => program_and_warnings,
Err(errors) => {
return (context.file_manager, Err(errors));
return Err(errors);
}
};

// Apply backend specific optimizations.
let optimized_program = crate::ops::optimize_program(program, np_language);

(context.file_manager, Ok((optimized_program, warnings)))
Ok((optimized_program, warnings))
}

fn compile_contract(
file_manager: &FileManager,
package: &Package,
compile_options: &CompileOptions,
np_language: Language,
) -> (FileManager, CompilationResult<CompiledContract>) {
let (mut context, crate_id) = prepare_package(package);
) -> CompilationResult<CompiledContract> {
let (mut context, crate_id) = prepare_package(file_manager, package);
let (contract, warnings) =
match noirc_driver::compile_contract(&mut context, crate_id, compile_options) {
Ok(contracts_and_warnings) => contracts_and_warnings,
Err(errors) => {
return (context.file_manager, Err(errors));
return Err(errors);
}
};

let optimized_contract = crate::ops::optimize_contract(contract, np_language);

(context.file_manager, Ok((optimized_contract, warnings)))
Ok((optimized_contract, warnings))
}

pub(crate) fn report_errors<T>(
Expand Down
Loading