Skip to content

Commit

Permalink
feat: Remove the concept of CrateType
Browse files Browse the repository at this point in the history
  • Loading branch information
phated committed Jul 6, 2023
1 parent 8895853 commit 60db9c6
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 87 deletions.
6 changes: 3 additions & 3 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use lsp_types::{
};
use noirc_driver::{check_crate, create_local_crate};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{graph::CrateType, hir::Context};
use noirc_frontend::hir::Context;
use serde_json::Value as JsonValue;
use tower::Service;

Expand Down Expand Up @@ -135,7 +135,7 @@ fn on_code_lens_request(
) -> impl Future<Output = Result<Option<Vec<CodeLens>>, ResponseError>> {
let file_path = &params.text_document.uri.to_file_path().unwrap();

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

// 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 @@ -220,7 +220,7 @@ fn on_did_save_text_document(
) -> ControlFlow<Result<(), async_lsp::Error>> {
let file_path = &params.text_document.uri.to_file_path().unwrap();

create_local_crate(&mut state.context, file_path, CrateType::Binary);
create_local_crate(&mut state.context, file_path);

let mut diagnostics = Vec::new();

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 @@ -119,7 +119,7 @@ pub fn prove_and_verify(program_dir: &Path, experimental_ssa: bool) -> bool {
mod tests {
use noirc_driver::{check_crate, create_local_crate};
use noirc_errors::reporter;
use noirc_frontend::{graph::CrateType, hir::Context};
use noirc_frontend::hir::Context;

use std::path::{Path, PathBuf};

Expand All @@ -130,7 +130,7 @@ mod tests {
/// This is used for tests.
fn file_compiles<P: AsRef<Path>>(root_file: P) -> bool {
let mut context = Context::default();
create_local_crate(&mut context, &root_file, CrateType::Binary);
create_local_crate(&mut context, &root_file);

let result = check_crate(&mut context, 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 @@ -7,7 +7,6 @@
//! This name was used because it sounds like `cargo` and
//! Noir Package Manager abbreviated is npm, which is already taken.
use noirc_frontend::graph::CrateType;
use std::{
fs::ReadDir,
path::{Path, PathBuf},
Expand Down Expand Up @@ -49,7 +48,7 @@ fn find_package_manifest(current_path: &Path) -> Result<PathBuf, InvalidPackageE
.ok_or_else(|| InvalidPackageError::MissingManifestFile(current_path.to_path_buf()))
}

fn lib_or_bin(current_path: &Path) -> Result<(PathBuf, CrateType), InvalidPackageError> {
fn lib_or_bin(current_path: &Path) -> Result<PathBuf, InvalidPackageError> {
// A library has a lib.nr and a binary has a main.nr
// You cannot have both.
let src_path = find_dir(current_path, "src")
Expand All @@ -59,8 +58,8 @@ fn lib_or_bin(current_path: &Path) -> Result<(PathBuf, CrateType), InvalidPackag
let bin_nr_path = find_file(&src_path, "main", "nr");
match (lib_nr_path, bin_nr_path) {
(Some(_), Some(_)) => Err(InvalidPackageError::ContainsMultipleCrates),
(None, Some(path)) => Ok((path, CrateType::Binary)),
(Some(path), None) => Ok((path, CrateType::Library)),
(None, Some(path)) => Ok(path),
(Some(path), None) => Ok(path),
(None, None) => Err(InvalidPackageError::ContainsZeroCrates),
}
}
Expand Down
21 changes: 7 additions & 14 deletions crates/nargo_cli/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ use std::{

use nargo::manifest::{Dependency, PackageManifest};
use noirc_driver::{add_dep, create_local_crate, create_non_local_crate};
use noirc_frontend::{
graph::{CrateId, CrateType},
hir::Context,
};
use noirc_frontend::{graph::CrateId, hir::Context};
use thiserror::Error;

use crate::{git::clone_git_repo, InvalidPackageError};
Expand Down Expand Up @@ -46,7 +43,6 @@ pub(crate) enum DependencyResolutionError {
#[derive(Debug, Clone)]
struct CachedDep {
entry_path: PathBuf,
crate_type: CrateType,
manifest: PackageManifest,
// Whether the dependency came from
// a remote dependency
Expand All @@ -66,12 +62,12 @@ pub(crate) fn resolve_root_manifest(
dir_path: &std::path::Path,
) -> Result<Context, DependencyResolutionError> {
let mut context = Context::default();
let (entry_path, crate_type) = super::lib_or_bin(dir_path)?;
let entry_path = super::lib_or_bin(dir_path)?;

let manifest_path = super::find_package_manifest(dir_path)?;
let manifest = super::manifest::parse(&manifest_path)?;

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

let pkg_root = manifest_path.parent().expect("Every manifest path has a parent.");
resolve_manifest(&mut context, crate_id, manifest, pkg_root)?;
Expand All @@ -97,15 +93,12 @@ fn resolve_manifest(
for (dep_pkg_name, pkg_src) in manifest.dependencies.iter() {
let (dir_path, dep_meta) = cache_dep(pkg_src, pkg_root)?;

let (entry_path, crate_type) = (&dep_meta.entry_path, &dep_meta.crate_type);

if crate_type == &CrateType::Binary {
let crate_id = create_non_local_crate(context, &dep_meta.entry_path);
if context.is_binary_crate(&crate_id) {
return Err(DependencyResolutionError::BinaryDependency {
dep_pkg_name: dep_pkg_name.to_string(),
});
}

let crate_id = create_non_local_crate(context, entry_path, *crate_type);
add_dep(context, parent_crate, crate_id, dep_pkg_name);

cached_packages.insert(dir_path, (crate_id, dep_meta));
Expand Down Expand Up @@ -136,10 +129,10 @@ fn cache_dep(
dir_path: &Path,
remote: bool,
) -> Result<CachedDep, DependencyResolutionError> {
let (entry_path, crate_type) = super::lib_or_bin(dir_path)?;
let entry_path = super::lib_or_bin(dir_path)?;
let manifest_path = super::find_package_manifest(dir_path)?;
let manifest = super::manifest::parse(manifest_path)?;
Ok(CachedDep { entry_path, crate_type, manifest, remote })
Ok(CachedDep { entry_path, manifest, remote })
}

match dep {
Expand Down
24 changes: 8 additions & 16 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use fm::{FileId, FileType};
use noirc_abi::FunctionSignature;
use noirc_errors::{CustomDiagnostic, FileDiagnostic};
use noirc_evaluator::{create_circuit, ssa_refactor::experimental_create_circuit};
use noirc_frontend::graph::{CrateId, CrateName, CrateType, LOCAL_CRATE};
use noirc_frontend::graph::{CrateId, CrateName, LOCAL_CRATE};
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
use noirc_frontend::hir::Context;
use noirc_frontend::monomorphization::monomorphize;
Expand Down Expand Up @@ -74,7 +74,7 @@ pub fn compile_file(
np_language: Language,
is_opcode_supported: &impl Fn(&Opcode) -> bool,
) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
create_local_crate(context, root_file, CrateType::Binary);
create_local_crate(context, root_file);
compile_main(context, np_language, is_opcode_supported, &CompileOptions::default())
}

Expand All @@ -84,15 +84,11 @@ pub fn compile_file(
/// we have multiple binaries in one workspace
/// A Fix would be for the driver instance to store the local crate id.
// Granted that this is the only place which relies on the local crate being first
pub fn create_local_crate<P: AsRef<Path>>(
context: &mut Context,
root_file: P,
crate_type: CrateType,
) -> CrateId {
pub fn create_local_crate<P: AsRef<Path>>(context: &mut Context, root_file: P) -> CrateId {
let dir_path = root_file.as_ref().to_path_buf();
let root_file_id = context.file_manager.add_file(&dir_path, FileType::Root).unwrap();

let crate_id = context.crate_graph.add_crate_root(crate_type, root_file_id);
let crate_id = context.crate_graph.add_crate_root(root_file_id);

assert!(crate_id == LOCAL_CRATE);

Expand All @@ -101,11 +97,7 @@ pub fn create_local_crate<P: AsRef<Path>>(

/// 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<P: AsRef<Path>>(
context: &mut Context,
root_file: P,
crate_type: CrateType,
) -> CrateId {
pub fn create_non_local_crate<P: AsRef<Path>>(context: &mut Context, root_file: P) -> CrateId {
let dir_path = root_file.as_ref().to_path_buf();
let root_file_id = context.file_manager.add_file(&dir_path, FileType::Root).unwrap();

Expand All @@ -114,7 +106,7 @@ pub fn create_non_local_crate<P: AsRef<Path>>(

// 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)
context.crate_graph.add_crate_root(root_file_id)
}

/// Adds a edge in the crate graph for two crates
Expand All @@ -123,7 +115,7 @@ pub fn add_dep(context: &mut Context, this_crate: CrateId, depends_on: CrateId,
.expect("crate name contains blacklisted characters, please remove");

// Cannot depend on a binary
if context.crate_graph.crate_type(depends_on) == CrateType::Binary {
if context.is_binary_crate(&depends_on) {
panic!("crates cannot depend on binaries. {crate_name:?} is a binary crate")
}

Expand Down Expand Up @@ -164,7 +156,7 @@ pub fn check_crate(
// parts of the code expect the `0` FileID to be the crate root. See also #1681
let std_crate_name = "std";
let path_to_std_lib_file = PathBuf::from(std_crate_name).join("lib.nr");
let std_crate = create_non_local_crate(context, path_to_std_lib_file, CrateType::Library);
let std_crate = create_non_local_crate(context, path_to_std_lib_file);
propagate_dep(context, std_crate, &CrateName::new(std_crate_name).unwrap());

let mut errors = vec![];
Expand Down
11 changes: 4 additions & 7 deletions crates/noirc_driver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ use acvm::Language;
use noirc_driver::{
add_dep, compile_main, create_local_crate, create_non_local_crate, CompileOptions,
};
use noirc_frontend::{
graph::{CrateType, LOCAL_CRATE},
hir::Context,
};
use noirc_frontend::{graph::LOCAL_CRATE, hir::Context};
fn main() {
const EXTERNAL_DIR: &str = "dep_b/lib.nr";
const EXTERNAL_DIR2: &str = "dep_a/lib.nr";
Expand All @@ -14,11 +11,11 @@ fn main() {
let mut context = Context::default();

// Add local crate to dep graph
create_local_crate(&mut context, ROOT_DIR_MAIN, CrateType::Binary);
create_local_crate(&mut context, ROOT_DIR_MAIN);

// Add libraries into Driver
let crate_id1 = create_non_local_crate(&mut context, EXTERNAL_DIR2, CrateType::Library);
let crate_id2 = create_non_local_crate(&mut context, EXTERNAL_DIR, CrateType::Library);
let crate_id1 = create_non_local_crate(&mut context, EXTERNAL_DIR2);
let crate_id2 = create_non_local_crate(&mut context, EXTERNAL_DIR);

// Add dependencies as package
add_dep(&mut context, LOCAL_CRATE, crate_id1, "coo4");
Expand Down
38 changes: 13 additions & 25 deletions crates/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,9 @@ pub struct CrateGraph {
/// and we do not want names that differ by a hyphen
pub const CHARACTER_BLACK_LIST: [char; 1] = ['-'];

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum CrateType {
Library,
Binary,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CrateData {
pub root_file_id: FileId,
pub crate_type: CrateType,
pub dependencies: Vec<Dependency>,
}

Expand All @@ -80,7 +73,7 @@ impl Dependency {
}

impl CrateGraph {
pub fn add_crate_root(&mut self, crate_type: CrateType, file_id: FileId) -> CrateId {
pub fn add_crate_root(&mut self, file_id: FileId) -> CrateId {
let mut roots_with_file_id =
self.arena.iter().filter(|(_, crate_data)| crate_data.root_file_id == file_id);

Expand All @@ -89,17 +82,12 @@ impl CrateGraph {
return *file_id.0;
}

let data = CrateData { root_file_id: file_id, crate_type, dependencies: Vec::new() };
let data = CrateData { root_file_id: file_id, dependencies: Vec::new() };
let crate_id = CrateId(self.arena.len());
let prev = self.arena.insert(crate_id, data);
assert!(prev.is_none());
crate_id
}

pub fn crate_type(&self, crate_id: CrateId) -> CrateType {
self.arena.get(&crate_id).unwrap().crate_type
}

pub fn iter_keys(&self) -> impl Iterator<Item = CrateId> + '_ {
self.arena.keys().copied()
}
Expand Down Expand Up @@ -191,7 +179,7 @@ pub struct CyclicDependenciesError {
mod tests {
use std::path::PathBuf;

use super::{CrateGraph, CrateName, CrateType, FileId};
use super::{CrateGraph, CrateName, FileId};

fn dummy_file_ids(n: usize) -> Vec<FileId> {
use fm::{FileMap, FILE_EXTENSION};
Expand All @@ -213,9 +201,9 @@ mod tests {
let file_ids = dummy_file_ids(3);

let mut graph = CrateGraph::default();
let crate1 = graph.add_crate_root(CrateType::Library, file_ids[0]);
let crate2 = graph.add_crate_root(CrateType::Library, file_ids[1]);
let crate3 = graph.add_crate_root(CrateType::Library, file_ids[2]);
let crate1 = graph.add_crate_root(file_ids[0]);
let crate2 = graph.add_crate_root(file_ids[1]);
let crate3 = graph.add_crate_root(file_ids[2]);

assert!(graph.add_dep(crate1, CrateName::new("crate2").unwrap(), crate2).is_ok());
assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok());
Expand All @@ -229,9 +217,9 @@ mod tests {
let file_id_1 = file_ids[1];
let file_id_2 = file_ids[2];
let mut graph = CrateGraph::default();
let crate1 = graph.add_crate_root(CrateType::Library, file_id_0);
let crate2 = graph.add_crate_root(CrateType::Library, file_id_1);
let crate3 = graph.add_crate_root(CrateType::Library, file_id_2);
let crate1 = graph.add_crate_root(file_id_0);
let crate2 = graph.add_crate_root(file_id_1);
let crate3 = graph.add_crate_root(file_id_2);
assert!(graph.add_dep(crate1, CrateName::new("crate2").unwrap(), crate2).is_ok());
assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok());
}
Expand All @@ -242,12 +230,12 @@ mod tests {
let file_id_1 = file_ids[1];
let file_id_2 = file_ids[2];
let mut graph = CrateGraph::default();
let _crate1 = graph.add_crate_root(CrateType::Library, file_id_0);
let _crate2 = graph.add_crate_root(CrateType::Library, file_id_1);
let _crate1 = graph.add_crate_root(file_id_0);
let _crate2 = graph.add_crate_root(file_id_1);

// Adding the same file, so the crate should be the same.
let crate3 = graph.add_crate_root(CrateType::Library, file_id_2);
let crate3_2 = graph.add_crate_root(CrateType::Library, file_id_2);
let crate3 = graph.add_crate_root(file_id_2);
let crate3_2 = graph.add_crate_root(file_id_2);
assert_eq!(crate3, crate3_2);
}
}
15 changes: 7 additions & 8 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub mod resolution;
pub mod scope;
pub mod type_check;

use crate::graph::{CrateGraph, CrateId, CrateType};
use crate::graph::{CrateGraph, CrateId};
use crate::hir_def::function::FuncMeta;
use crate::node_interner::{FuncId, NodeInterner};
use def_map::{Contract, CrateDefMap};
Expand Down Expand Up @@ -66,19 +66,18 @@ impl Context {
/// - Expects check_crate to be called beforehand
/// - Panics if no main function is found
pub fn get_main_function(&self, crate_id: &CrateId) -> Option<FuncId> {
// Find the local crate, one should always be present
let local_crate = self.def_map(crate_id).unwrap();

// Check the crate type
// We don't panic here to allow users to `evaluate` libraries which will do nothing
if self.crate_graph[*crate_id].crate_type == CrateType::Binary {
// All Binaries should have a main function
if let Some(local_crate) = self.def_map(crate_id) {
local_crate.main_function()
} else {
None
}
}

pub fn is_binary_crate(&self, crate_id: &CrateId) -> bool {
// Currently, anything with a `main` function is a binary crate
matches!(self.get_main_function(crate_id), Some(_))
}

/// Returns a list of all functions in the current crate marked with #[test]
/// whose names contain the given pattern string. An empty pattern string
/// will return all functions marked with #[test].
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_frontend/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::path::PathBuf;

use fm::{FileManager, FileType};
use hir::{def_map::CrateDefMap, Context};
use noirc_frontend::graph::{CrateGraph, CrateType};
use noirc_frontend::graph::CrateGraph;
use noirc_frontend::hir::{self, def_map::ModuleDefId};

// XXX: This is another sandbox test
Expand All @@ -24,7 +24,7 @@ fn main() {
// CrateGraph
let mut crate_graph = CrateGraph::default();
// Initiate crate with root file
let crate_id = crate_graph.add_crate_root(CrateType::Library, root_file_id);
let crate_id = crate_graph.add_crate_root(root_file_id);

// initiate context with file manager and crate graph
let mut context = Context::new(fm, crate_graph);
Expand Down
Loading

0 comments on commit 60db9c6

Please sign in to comment.