fix: Differentiate stdlib CrateId from others (#1895)

* chore: Add failing test

* fix: Differentiate stdlib `CrateId` from others

feat: Drop LOCAL_CRATE constant

* add is_stdlib helper

* remove unused panic section

---------

Co-authored-by: TomAFrench <[email protected]>

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,
     };

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<Result<(), async_lsp::Error>> {
     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,
     };

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

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<B: Backend>(
     program_dir: &Path,
     compile_options: &CompileOptions,
 ) -> Result<(), CliError<B>> {
-    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)
 }

crates/nargo_cli/src/cli/compile_cmd.rs

@@ -57,9 +57,9 @@ 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, 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<B: Backend>(
     program_dir: &Path,
     compile_options: &CompileOptions,
 ) -> Result<(CompiledProgram, Context), CliError<B>> {
-    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.

crates/nargo_cli/src/cli/mod.rs

@@ -101,9 +101,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 {

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<B: Backend>(
     test_name: &str,
     compile_options: &CompileOptions,
 ) -> Result<(), CliError<B>> {
-    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());

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<String>,
-) -> Result<Context, DependencyResolutionError> {
+) -> 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<CrateId, DependencyResolutionError> {
     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

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"]

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" }

crates/nargo_cli/tests/test_data/dep_impl_primitive/Prover.toml

@@ -0,0 +1 @@
+x = 1

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();
+}

crates/nargo_cli/tests/test_libraries/bad_impl/Nargo.toml

@@ -0,0 +1,5 @@
+[package]
+authors = [""]
+compiler_version = "0.7.1"
+
+[dependencies]

crates/nargo_cli/tests/test_libraries/bad_impl/src/lib.nr

@@ -0,0 +1,5 @@
+impl Field {
+    fn something(self) -> Field {
+        self
+    }
+}

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<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
@@ -104,9 +100,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)
@@ -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<Warnings, ErrorsAndWarnings> {
@@ -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<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);
 
@@ -197,11 +198,12 @@ 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,
     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 {
@@ -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<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;