diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index 95aae571fbb..cab74bd5560 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -171,7 +171,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(); @@ -254,11 +254,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, }; diff --git a/crates/lsp/src/lib_hacky.rs b/crates/lsp/src/lib_hacky.rs index 9d0e4c0c2dc..36feee27348 100644 --- a/crates/lsp/src/lib_hacky.rs +++ b/crates/lsp/src/lib_hacky.rs @@ -78,7 +78,7 @@ pub 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 driver, false, false); + let _ = check_crate(&mut driver, current_crate_id, false, false); let fm = &driver.file_manager; let files = fm.as_simple_files(); @@ -197,9 +197,9 @@ pub fn on_did_save_text_document( params: DidSaveTextDocumentParams, ) -> ControlFlow> { let actual_path = params.text_document.uri.to_file_path().unwrap(); - let (mut context, _) = create_context_at_path(actual_path.clone()); + let (mut context, crate_id) = create_context_at_path(actual_path.clone()); - 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, }; diff --git a/crates/nargo_cli/build.rs b/crates/nargo_cli/build.rs index e453f51777c..aa3b651660a 100644 --- a/crates/nargo_cli/build.rs +++ b/crates/nargo_cli/build.rs @@ -102,7 +102,7 @@ fn prove_and_verify_{test_sub_dir}_{test_name}() {{ if {experimental_ssa} {{ cmd.arg("--experimental-ssa"); }}; - + if {should_fail} {{ cmd.assert().failure(); diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index 9b3f4e459e4..9d071731207 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -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; @@ -36,15 +36,16 @@ fn check_from_path( program_dir: &Path, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut context = resolve_root_manifest(program_dir, None)?; + let (mut context, crate_id) = resolve_root_manifest(program_dir, None)?; 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. // @@ -217,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) } diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index d6d3deda55f..fbaecb606a1 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -57,9 +57,9 @@ pub(crate) fn run( // 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, None)?; + let (mut context, crate_id) = resolve_root_manifest(&config.program_dir, None)?; - let result = compile_contracts(&mut context, &args.compile_options); + let result = compile_contracts(&mut context, crate_id, &args.compile_options); let contracts = report_errors(result, &context, args.compile_options.deny_warnings)?; // TODO(#1389): I wonder if it is incorrect for nargo-core to know anything about contracts. @@ -124,8 +124,8 @@ pub(crate) fn compile_circuit( program_dir: &Path, compile_options: &CompileOptions, ) -> Result<(CompiledProgram, Context), CliError> { - let mut context = resolve_root_manifest(program_dir, package)?; - let result = compile_main(&mut context, compile_options); + let (mut context, crate_id) = resolve_root_manifest(program_dir, package)?; + let result = compile_main(&mut context, crate_id, compile_options); let mut program = report_errors(result, &context, compile_options.deny_warnings)?; // Apply backend specific optimizations. diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index d77ca55f81a..2edf36b1166 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -101,9 +101,9 @@ mod tests { /// This is used for tests. fn file_compiles>(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 { diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index dcc4cce5734..afb4481f5c5 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -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::{ @@ -40,18 +40,19 @@ fn run_tests( test_name: &str, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut context = resolve_root_manifest(program_dir, None)?; + let (mut context, crate_id) = resolve_root_manifest(program_dir, None)?; check_crate_and_report_errors( &mut context, + crate_id, compile_options.deny_warnings, compile_options.experimental_ssa, )?; - let test_functions = match context.crate_graph.crate_type(LOCAL_CRATE) { + let test_functions = match context.crate_graph.crate_type(crate_id) { noirc_frontend::graph::CrateType::Workspace => { context.get_all_test_functions_in_workspace_matching(test_name) } - _ => context.get_all_test_functions_in_crate_matching(&LOCAL_CRATE, test_name), + _ => context.get_all_test_functions_in_crate_matching(&crate_id, test_name), }; println!("Running {} test functions...", test_functions.len()); diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index f5cc640ff5f..9598031b193 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -87,33 +87,33 @@ struct CachedDep { pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, package: Option, -) -> Result { +) -> Result<(Context, CrateId), DependencyResolutionError> { let mut context = Context::default(); let manifest_path = super::find_package_manifest(dir_path)?; let manifest = super::manifest::parse(&manifest_path)?; - match manifest { - Manifest::Package(inner) => { + let crate_id = match manifest { + Manifest::Package(package) => { let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; let crate_id = create_local_crate(&mut context, entry_path, crate_type); let pkg_root = manifest_path.parent().expect("Every manifest path has a parent."); - resolve_package_manifest(&mut context, crate_id, inner, pkg_root)?; - } - Manifest::Workspace(workspace) => { - resolve_workspace_manifest( - &mut context, - package, - manifest_path, - dir_path, - workspace.config, - )?; + resolve_package_manifest(&mut context, crate_id, package, pkg_root)?; + + crate_id } + Manifest::Workspace(workspace) => resolve_workspace_manifest( + &mut context, + package, + manifest_path, + dir_path, + workspace.config, + )?, }; - Ok(context) + Ok((context, crate_id)) } // Resolves a config file by recursively resolving the dependencies in the config @@ -169,7 +169,7 @@ fn resolve_workspace_manifest( manifest_path: PathBuf, dir_path: &Path, workspace: WorkspaceConfig, -) -> Result<(), DependencyResolutionError> { +) -> Result { let members = workspace.members; let mut packages = HashMap::new(); @@ -222,14 +222,14 @@ fn resolve_workspace_manifest( .ok_or_else(|| DependencyResolutionError::PackageNotFound(crate_name(local_package)))?; let (entry_path, _crate_type) = super::lib_or_bin(local_crate)?; - create_local_crate(context, entry_path, CrateType::Workspace); + let crate_id = create_local_crate(context, entry_path, CrateType::Workspace); for (_, package_path) in packages.drain() { let (entry_path, crate_type) = super::lib_or_bin(package_path)?; create_non_local_crate(context, entry_path, crate_type); } - Ok(()) + Ok(crate_id) } /// If the dependency is remote, download the dependency diff --git a/crates/nargo_cli/tests/test_data/config.toml b/crates/nargo_cli/tests/test_data/config.toml index b191daf23db..bc0ac152271 100644 --- a/crates/nargo_cli/tests/test_data/config.toml +++ b/crates/nargo_cli/tests/test_data/config.toml @@ -4,4 +4,4 @@ exclude = ["bit_shifts_runtime", "comptime_fail", "sha2_blocks", "sha2_byte"] # List of tests (as their directory name in test_data) expecting to fail: if the test pass, we report an error. -fail = ["range_fail"] +fail = ["range_fail", "dep_impl_primitive"] diff --git a/crates/nargo_cli/tests/test_data/dep_impl_primitive/Nargo.toml b/crates/nargo_cli/tests/test_data/dep_impl_primitive/Nargo.toml new file mode 100644 index 00000000000..080785a4a63 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/dep_impl_primitive/Nargo.toml @@ -0,0 +1,6 @@ +[package] +authors = [""] +compiler_version = "0.8.0" + +[dependencies] +bad_impl = { path = "../../test_libraries/bad_impl" } diff --git a/crates/nargo_cli/tests/test_data/dep_impl_primitive/Prover.toml b/crates/nargo_cli/tests/test_data/dep_impl_primitive/Prover.toml new file mode 100644 index 00000000000..7d4290a117a --- /dev/null +++ b/crates/nargo_cli/tests/test_data/dep_impl_primitive/Prover.toml @@ -0,0 +1 @@ +x = 1 diff --git a/crates/nargo_cli/tests/test_data/dep_impl_primitive/src/main.nr b/crates/nargo_cli/tests/test_data/dep_impl_primitive/src/main.nr new file mode 100644 index 00000000000..ff9a40da18e --- /dev/null +++ b/crates/nargo_cli/tests/test_data/dep_impl_primitive/src/main.nr @@ -0,0 +1,5 @@ +use dep::bad_impl; + +fn main(x : Field) { + x.something(); +} diff --git a/crates/nargo_cli/tests/test_libraries/bad_impl/Nargo.toml b/crates/nargo_cli/tests/test_libraries/bad_impl/Nargo.toml new file mode 100644 index 00000000000..ffaca6d92ea --- /dev/null +++ b/crates/nargo_cli/tests/test_libraries/bad_impl/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.7.1" + +[dependencies] diff --git a/crates/nargo_cli/tests/test_libraries/bad_impl/src/lib.nr b/crates/nargo_cli/tests/test_libraries/bad_impl/src/lib.nr new file mode 100644 index 00000000000..a96a6cbf91f --- /dev/null +++ b/crates/nargo_cli/tests/test_libraries/bad_impl/src/lib.nr @@ -0,0 +1,5 @@ +impl Field { + fn something(self) -> Field { + self + } +} diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index b4d0e636e20..525c15af1e8 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -8,7 +8,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; @@ -69,8 +69,8 @@ pub fn compile_file( context: &mut Context, root_file: PathBuf, ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> { - create_local_crate(context, root_file, CrateType::Binary); - compile_main(context, &CompileOptions::default()) + let crate_id = create_local_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 @@ -87,11 +87,7 @@ pub fn create_local_crate>( 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 @@ -104,9 +100,6 @@ pub fn create_non_local_crate>( 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) @@ -151,6 +144,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 { @@ -160,20 +154,24 @@ 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![]; - match context.crate_graph.crate_type(LOCAL_CRATE) { + match context.crate_graph.crate_type(crate_id) { CrateType::Workspace => { let keys: Vec<_> = context.crate_graph.iter_keys().collect(); // avoid borrow checker for crate_id in keys { CrateDefMap::collect_defs(crate_id, context, &mut errors); } } - _ => CrateDefMap::collect_defs(LOCAL_CRATE, context, &mut errors), + _ => CrateDefMap::collect_defs(crate_id, context, &mut errors), } if has_errors(&errors, deny_warnings) { @@ -183,8 +181,11 @@ pub fn check_crate( } } -pub fn compute_function_signature(context: &Context) -> Option { - let main_function = context.get_main_function(&LOCAL_CRATE)?; +pub fn compute_function_signature( + context: &Context, + crate_id: &CrateId, +) -> Option { + let main_function = context.get_main_function(crate_id)?; let func_meta = context.def_interner.function_meta(&main_function); @@ -197,11 +198,12 @@ pub fn compute_function_signature(context: &Context) -> Option 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 { @@ -225,11 +227,12 @@ 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, options: &CompileOptions, ) -> Result<(Vec, 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; diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index f87a9b3347d..7a95ff324de 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/crates/noirc_frontend/src/graph/mod.rs @@ -8,16 +8,19 @@ 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) + } + + pub fn is_stdlib(&self) -> bool { + matches!(self, CrateId::Stdlib(_)) } } @@ -50,7 +53,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 + } + } } } @@ -98,7 +105,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 diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index b0022e26924..d5fc0251d6e 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -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; @@ -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 && !crate_id.is_stdlib() { let span = *span; let error = DefCollectorErrorKind::NonStructTypeInImpl { span }; errors.push(error.into_file_diagnostic(unresolved.file_id)); diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index 1218bbe5d0d..5496920cf56 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -89,12 +89,13 @@ 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, &options.compile_options) - .expect("Contract compilation failed") - .0; + let compiled_contracts = + compile_contracts(&mut context, crate_id, &options.compile_options) + .expect("Contract compilation failed") + .0; let optimized_contracts: Vec = compiled_contracts.into_iter().map(optimize_contract).collect();