Skip to content

Commit

Permalink
fix: Differentiate stdlib CrateId from others
Browse files Browse the repository at this point in the history
feat: Drop LOCAL_CRATE constant
  • Loading branch information
phated committed Jul 7, 2023
1 parent cfb83f3 commit f8ceba7
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 59 deletions.
6 changes: 3 additions & 3 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ fn on_code_lens_request(

// We ignore the warnings and errors produced by compilation for producing codelenses
// because we can still get the test functions even if compilation fails
let _ = check_crate(&mut context, false, false);
let _ = check_crate(&mut context, crate_id, false, false);

let fm = &context.file_manager;
let files = fm.as_simple_files();
Expand Down Expand Up @@ -222,11 +222,11 @@ fn on_did_save_text_document(

let mut context = Context::default();

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

let mut diagnostics = Vec::new();

let file_diagnostics = match check_crate(&mut context, false, false) {
let file_diagnostics = match check_crate(&mut context, crate_id, false, false) {
Ok(warnings) => warnings,
Err(errors_and_warnings) => errors_and_warnings,
};
Expand Down
17 changes: 12 additions & 5 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use iter_extended::btree_map;
use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME};
use noirc_driver::{check_crate, compute_function_signature, CompileOptions};
use noirc_errors::reporter::ReportedErrors;
use noirc_frontend::hir::Context;
use noirc_frontend::{graph::CrateId, hir::Context};
use std::path::{Path, PathBuf};

use super::fs::write_to_file;
Expand Down Expand Up @@ -36,11 +36,16 @@ fn check_from_path<B: Backend>(
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings, compile_options.experimental_ssa)?;
let (mut context, crate_id) = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(
&mut context,
crate_id,
compile_options.deny_warnings,
compile_options.experimental_ssa,
)?;

// XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
if let Some((parameters, return_type)) = compute_function_signature(&context) {
if let Some((parameters, return_type)) = compute_function_signature(&context, &crate_id) {
// XXX: The root config should return an enum to determine if we are looking for .json or .toml
// For now it is hard-coded to be toml.
//
Expand Down Expand Up @@ -213,9 +218,11 @@ d2 = ["", "", ""]
/// and errors found.
pub(crate) fn check_crate_and_report_errors(
context: &mut Context,
crate_id: CrateId,
deny_warnings: bool,
enable_slices: bool,
) -> Result<(), ReportedErrors> {
let result = check_crate(context, deny_warnings, enable_slices).map(|warnings| ((), warnings));
let result =
check_crate(context, crate_id, deny_warnings, enable_slices).map(|warnings| ((), warnings));
super::compile_cmd::report_errors(result, context, deny_warnings)
}
6 changes: 4 additions & 2 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,11 @@ pub(crate) fn run<B: Backend>(

// If contracts is set we're compiling every function in a 'contract' rather than just 'main'.
if args.contracts {
let mut context = resolve_root_manifest(&config.program_dir)?;
let (mut context, crate_id) = resolve_root_manifest(&config.program_dir)?;

let result = compile_contracts(
&mut context,
crate_id,
backend.np_language(),
&|op| backend.supports_opcode(op),
&args.compile_options,
Expand Down Expand Up @@ -123,9 +124,10 @@ pub(crate) fn compile_circuit<B: Backend>(
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<CompiledProgram, CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
let (mut context, crate_id) = resolve_root_manifest(program_dir)?;
let result = compile_main(
&mut context,
crate_id,
backend.np_language(),
&|op| backend.supports_opcode(op),
compile_options,
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 @@ -130,9 +130,9 @@ 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);
let crate_id = create_local_crate(&mut context, &root_file, CrateType::Binary);

let result = check_crate(&mut context, false, false);
let result = check_crate(&mut context, crate_id, false, false);
let success = result.is_ok();

let errors = match result {
Expand Down
15 changes: 10 additions & 5 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use acvm::{acir::native_types::WitnessMap, Backend};
use clap::Args;
use nargo::ops::execute_circuit;
use noirc_driver::{compile_no_check, CompileOptions};
use noirc_frontend::{graph::LOCAL_CRATE, hir::Context, node_interner::FuncId};
use noirc_frontend::{hir::Context, node_interner::FuncId};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

use crate::{
Expand Down Expand Up @@ -40,10 +40,15 @@ fn run_tests<B: Backend>(
test_name: &str,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings, compile_options.experimental_ssa)?;

let test_functions = context.get_all_test_functions_in_crate_matching(&LOCAL_CRATE, test_name);
let (mut context, crate_id) = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(
&mut context,
crate_id,
compile_options.deny_warnings,
compile_options.experimental_ssa,
)?;

let test_functions = context.get_all_test_functions_in_crate_matching(&crate_id, test_name);
println!("Running {} test functions...", test_functions.len());
let mut failing = 0;

Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ struct CachedDep {
/// XXX: Need to handle when a local package changes!
pub(crate) fn resolve_root_manifest(
dir_path: &std::path::Path,
) -> Result<Context, DependencyResolutionError> {
) -> Result<(Context, CrateId), DependencyResolutionError> {
let mut context = Context::default();
let (entry_path, crate_type) = super::lib_or_bin(dir_path)?;

Expand All @@ -76,7 +76,7 @@ pub(crate) fn resolve_root_manifest(
let pkg_root = manifest_path.parent().expect("Every manifest path has a parent.");
resolve_manifest(&mut context, crate_id, manifest, pkg_root)?;

Ok(context)
Ok((context, crate_id))
}

// Resolves a config file by recursively resolving the dependencies in the config
Expand Down
41 changes: 22 additions & 19 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;
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, CrateType};
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
use noirc_frontend::hir::Context;
use noirc_frontend::monomorphization::monomorphize;
Expand Down Expand Up @@ -74,8 +74,8 @@ 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);
compile_main(context, np_language, is_opcode_supported, &CompileOptions::default())
let crate_id = create_local_crate(context, root_file, CrateType::Binary);
compile_main(context, crate_id, np_language, is_opcode_supported, &CompileOptions::default())
}

/// Adds the File with the local crate root to the file system
Expand All @@ -92,11 +92,7 @@ pub fn create_local_crate<P: AsRef<Path>>(
let dir_path = root_file.as_ref().to_path_buf();
let root_file_id = context.file_manager.add_file(&dir_path).unwrap();

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

assert!(crate_id == LOCAL_CRATE);

LOCAL_CRATE
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
Expand All @@ -109,9 +105,6 @@ pub fn create_non_local_crate<P: AsRef<Path>>(
let dir_path = root_file.as_ref().to_path_buf();
let root_file_id = context.file_manager.add_file(&dir_path).unwrap();

// The first crate is always the local crate
assert!(context.crate_graph.number_of_crates() != 0);

// 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)
Expand Down Expand Up @@ -156,6 +149,7 @@ pub fn propagate_dep(
/// On error, this returns a non-empty vector of warnings and error messages, with at least one error.
pub fn check_crate(
context: &mut Context,
crate_id: CrateId,
deny_warnings: bool,
enable_slices: bool,
) -> Result<Warnings, ErrorsAndWarnings> {
Expand All @@ -165,13 +159,17 @@ 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 root_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap();

// You can add any crate type to the crate graph
// but you cannot depend on Binaries
let std_crate = context.crate_graph.add_stdlib(CrateType::Library, root_file_id);
propagate_dep(context, std_crate, &CrateName::new(std_crate_name).unwrap());

context.def_interner.enable_slices = enable_slices;

let mut errors = vec![];
CrateDefMap::collect_defs(LOCAL_CRATE, context, &mut errors);
CrateDefMap::collect_defs(crate_id, context, &mut errors);

if has_errors(&errors, deny_warnings) {
Err(errors)
Expand All @@ -180,8 +178,11 @@ pub fn check_crate(
}
}

pub fn compute_function_signature(context: &Context) -> Option<FunctionSignature> {
let main_function = context.get_main_function(&LOCAL_CRATE)?;
pub fn compute_function_signature(
context: &Context,
crate_id: &CrateId,
) -> Option<FunctionSignature> {
let main_function = context.get_main_function(crate_id)?;

let func_meta = context.def_interner.function_meta(&main_function);

Expand All @@ -194,13 +195,14 @@ pub fn compute_function_signature(context: &Context) -> Option<FunctionSignature
/// On error this returns the non-empty list of warnings and errors.
pub fn compile_main(
context: &mut Context,
crate_id: CrateId,
np_language: Language,
is_opcode_supported: &impl Fn(&Opcode) -> bool,
options: &CompileOptions,
) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;
let warnings = check_crate(context, crate_id, options.deny_warnings, options.experimental_ssa)?;

let main = match context.get_main_function(&LOCAL_CRATE) {
let main = match context.get_main_function(&crate_id) {
Some(m) => m,
None => {
let err = FileDiagnostic {
Expand All @@ -225,13 +227,14 @@ pub fn compile_main(
/// Run the frontend to check the crate for errors then compile all contracts if there were none
pub fn compile_contracts(
context: &mut Context,
crate_id: CrateId,
np_language: Language,
is_opcode_supported: &impl Fn(&Opcode) -> bool,
options: &CompileOptions,
) -> Result<(Vec<CompiledContract>, Warnings), ErrorsAndWarnings> {
let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;
let warnings = check_crate(context, crate_id, options.deny_warnings, options.experimental_ssa)?;

let contracts = context.get_all_contracts(&LOCAL_CRATE);
let contracts = context.get_all_contracts(&crate_id);
let mut compiled_contracts = vec![];
let mut errors = warnings;

Expand Down
12 changes: 5 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::CrateType, hir::Context};
fn main() {
const EXTERNAL_DIR: &str = "dep_b/lib.nr";
const EXTERNAL_DIR2: &str = "dep_a/lib.nr";
Expand All @@ -14,18 +11,19 @@ fn main() {
let mut context = Context::default();

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

// 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);

// Add dependencies as package
add_dep(&mut context, LOCAL_CRATE, crate_id1, "coo4");
add_dep(&mut context, LOCAL_CRATE, crate_id2, "coo3");
add_dep(&mut context, local_crate_id, crate_id1, "coo4");
add_dep(&mut context, local_crate_id, crate_id2, "coo3");

compile_main(
&mut context,
local_crate_id,
Language::R1CS,
#[allow(deprecated)]
&acvm::pwg::default_is_opcode_supported(Language::R1CS),
Expand Down
35 changes: 27 additions & 8 deletions crates/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@ use fm::FileId;
use rustc_hash::{FxHashMap, FxHashSet};
use smol_str::SmolStr;

/// The local crate is the crate being compiled.
/// The caller should ensure that this crate has a CrateId(0).
pub const LOCAL_CRATE: CrateId = CrateId(0);

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct CrateId(usize);
pub enum CrateId {
Crate(usize),
Stdlib(usize),
}

impl CrateId {
pub fn dummy_id() -> CrateId {
CrateId(std::usize::MAX)
CrateId::Crate(std::usize::MAX)
}
}

Expand Down Expand Up @@ -50,7 +49,11 @@ pub struct CrateGraph {

impl CrateGraph {
pub fn is_last_crate(&self, crate_id: CrateId) -> bool {
(self.arena.len() - 1) == crate_id.0
match crate_id {
CrateId::Crate(crate_id) | CrateId::Stdlib(crate_id) => {
(self.arena.len() - 1) == crate_id
}
}
}
}

Expand Down Expand Up @@ -97,7 +100,23 @@ impl CrateGraph {
}

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

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

let next_file_id = roots_with_file_id.next();
if let Some(file_id) = next_file_id {
return *file_id.0;
}

let data = CrateData { root_file_id: file_id, crate_type, dependencies: Vec::new() };
let crate_id = CrateId::Stdlib(self.arena.len());
let prev = self.arena.insert(crate_id, data);
assert!(prev.is_none());
crate_id
Expand Down
8 changes: 3 additions & 5 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::dc_mod::collect_defs;
use super::errors::DefCollectorErrorKind;
use crate::graph::{CrateId, LOCAL_CRATE};
use crate::graph::CrateId;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
use crate::hir::resolution::errors::ResolverError;
use crate::hir::resolution::resolver::Resolver;
Expand Down Expand Up @@ -237,10 +237,8 @@ fn collect_impls(
errors.push(err.into_file_diagnostic(unresolved.file_id));
}
}
// Prohibit defining impls for primitive types if we're in the local crate.
// We should really prevent it for all crates that aren't the noir stdlib but
// there is no way of checking if the current crate is the stdlib currently.
} else if typ != Type::Error && crate_id == LOCAL_CRATE {
// Prohibit defining impls for primitive types if we're not in the stdlib
} else if typ != Type::Error && matches!(crate_id, CrateId::Crate(_)) {
let span = *span;
let error = DefCollectorErrorKind::NonStructTypeInImpl { span };
errors.push(error.into_file_diagnostic(unresolved.file_id));
Expand Down
3 changes: 2 additions & 1 deletion crates/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ pub fn compile(args: JsValue) -> JsValue {
add_noir_lib(&mut context, dependency.as_str());
}

check_crate(&mut context, false, false).expect("Crate check failed");
check_crate(&mut context, crate_id, false, false).expect("Crate check failed");

if options.contracts {
let compiled_contracts = compile_contracts(
&mut context,
crate_id,
language,
#[allow(deprecated)]
&acvm::pwg::default_is_opcode_supported(language),
Expand Down

0 comments on commit f8ceba7

Please sign in to comment.