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(noirc_driver): Unify crate preparation #2119

Merged
merged 1 commit into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use lsp_types::{
InitializeParams, InitializeResult, InitializedParams, Position, PublishDiagnosticsParams,
Range, ServerCapabilities, TextDocumentSyncOptions,
};
use noirc_driver::{check_crate, create_local_crate};
use noirc_driver::{check_crate, prepare_crate};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{
graph::{CrateGraph, CrateType},
Expand Down Expand Up @@ -190,7 +190,7 @@ fn on_code_lens_request(
}
};

let crate_id = create_local_crate(&mut context, file_path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, file_path, CrateType::Binary);

// We ignore the warnings and errors produced by compilation for producing codelenses
// because we can still get the test functions even if compilation fails
Expand Down Expand Up @@ -283,7 +283,7 @@ fn on_did_save_text_document(
}
};

let crate_id = create_local_crate(&mut context, file_path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, file_path, CrateType::Binary);

let mut diagnostics = Vec::new();

Expand Down
7 changes: 3 additions & 4 deletions crates/lsp/src/lib_hacky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use lsp_types::{
InitializedParams, Position, PublishDiagnosticsParams, Range, ServerCapabilities,
TextDocumentSyncOptions,
};
use noirc_driver::{check_crate, create_local_crate, create_non_local_crate, propagate_dep};
use noirc_driver::{check_crate, prepare_crate, propagate_dep};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{
graph::{CrateGraph, CrateId, CrateType},
Expand Down Expand Up @@ -286,7 +286,7 @@ fn create_context_at_path(
}
let nargo_toml_path = find_nearest_parent_file(&file_path, &["Nargo.toml"]);

let current_crate_id = create_local_crate(&mut context, &file_path, CrateType::Binary);
let current_crate_id = prepare_crate(&mut context, &file_path, CrateType::Binary);

// TODO(AD): undo hacky dependency resolution
if let Some(nargo_toml_path) = nargo_toml_path {
Expand All @@ -297,8 +297,7 @@ fn create_context_at_path(
.parent()
.unwrap() // TODO
.join(PathBuf::from(&dependency_path).join("src").join("lib.nr"));
let library_crate =
create_non_local_crate(&mut context, &path_to_lib, CrateType::Library);
let library_crate = prepare_crate(&mut context, &path_to_lib, CrateType::Library);
propagate_dep(&mut context, library_crate, &crate_name.parse().unwrap());
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub fn start_cli() -> eyre::Result<()> {
#[cfg(test)]
mod tests {
use fm::FileManager;
use noirc_driver::{check_crate, create_local_crate};
use noirc_driver::{check_crate, prepare_crate};
use noirc_errors::reporter;
use noirc_frontend::{
graph::{CrateGraph, CrateType},
Expand All @@ -110,7 +110,7 @@ mod tests {
let fm = FileManager::new(root_dir);
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);
let crate_id = create_local_crate(&mut context, root_file, CrateType::Binary);
let crate_id = prepare_crate(&mut context, root_file, CrateType::Binary);

let result = check_crate(&mut context, crate_id, false);
let success = result.is_ok();
Expand Down
7 changes: 3 additions & 4 deletions crates/nargo_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use fm::FileManager;
use nargo::package::{Dependency, Package};
use noirc_driver::{add_dep, create_local_crate, create_non_local_crate};
use noirc_driver::{add_dep, prepare_crate};
use noirc_frontend::{
graph::{CrateGraph, CrateId, CrateName, CrateType},
hir::Context,
Expand Down Expand Up @@ -107,8 +107,7 @@ fn prepare_dependencies(
for (dep_name, dep) in dependencies.into_iter() {
match dep {
Dependency::Remote { package } | Dependency::Local { package } => {
let crate_id =
create_non_local_crate(context, &package.entry_path, package.crate_type);
let crate_id = prepare_crate(context, &package.entry_path, package.crate_type);
add_dep(context, parent_crate, crate_id, dep_name);
prepare_dependencies(context, crate_id, package.dependencies.to_owned());
}
Expand All @@ -121,7 +120,7 @@ fn prepare_package(package: &Package) -> (Context, CrateId) {
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);

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

prepare_dependencies(&mut context, crate_id, package.dependencies.to_owned());

Expand Down
29 changes: 3 additions & 26 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,40 +52,17 @@ pub fn compile_file(
context: &mut Context,
root_file: &Path,
) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
let crate_id = create_local_crate(context, root_file, CrateType::Binary);
let crate_id = prepare_crate(context, root_file, CrateType::Binary);
compile_main(context, crate_id, &CompileOptions::default())
}

/// Adds the File with the local crate root to the file system
/// and adds the local crate to the graph
/// XXX: This may pose a problem with workspaces, where you can change the local crate and where
/// we have multiple binaries in one workspace
/// A Fix would be for the driver instance to store the local crate id.
// Granted that this is the only place which relies on the local crate being first
pub fn create_local_crate(
context: &mut Context,
file_name: &Path,
crate_type: CrateType,
) -> CrateId {
/// Adds the file from the file system at `Path` to the crate graph
pub fn prepare_crate(context: &mut Context, file_name: &Path, crate_type: CrateType) -> CrateId {
let root_file_id = context.file_manager.add_file(file_name).unwrap();

context.crate_graph.add_crate_root(crate_type, root_file_id)
}

/// Creates a Non Local Crate. A Non Local Crate is any crate which is the not the crate that
/// the compiler is compiling.
pub fn create_non_local_crate(
context: &mut Context,
file_name: &Path,
crate_type: CrateType,
) -> CrateId {
let root_file_id = context.file_manager.add_file(file_name).unwrap();

// You can add any crate type to the crate graph
// but you cannot depend on Binaries
context.crate_graph.add_crate_root(crate_type, root_file_id)
}

/// Adds a edge in the crate graph for two crates
pub fn add_dep(
context: &mut Context,
Expand Down
8 changes: 4 additions & 4 deletions crates/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use fm::FileManager;
use gloo_utils::format::JsValueSerdeExt;
use log::debug;
use noirc_driver::{
check_crate, compile_contracts, compile_no_check, create_local_crate, create_non_local_crate,
propagate_dep, CompileOptions, CompiledContract,
check_crate, compile_contracts, compile_no_check, prepare_crate, propagate_dep, CompileOptions,
CompiledContract,
};
use noirc_frontend::{
graph::{CrateGraph, CrateType},
Expand Down Expand Up @@ -63,7 +63,7 @@ impl Default for WASMCompileOptions {

fn add_noir_lib(context: &mut Context, crate_name: &str) {
let path_to_lib = Path::new(&crate_name).join("lib.nr");
let library_crate = create_non_local_crate(context, &path_to_lib, CrateType::Library);
let library_crate = prepare_crate(context, &path_to_lib, CrateType::Library);

propagate_dep(context, library_crate, &crate_name.parse().unwrap());
}
Expand All @@ -87,7 +87,7 @@ pub fn compile(args: JsValue) -> JsValue {
let mut context = Context::new(fm, graph);

let path = Path::new(&options.entry_point);
let crate_id = create_local_crate(&mut context, path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, path, CrateType::Binary);

for dependency in options.optional_dependencies_set {
add_noir_lib(&mut context, dependency.as_str());
Expand Down