From 0af1f6f17bf9b1850ca61e1e69a7a19cddf19452 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 15 May 2024 21:37:11 -0400 Subject: [PATCH 01/41] wip --- compiler/fm/src/file_map.rs | 3 +- .../src/hir/def_collector/dc_crate.rs | 6 +++ .../src/hir/def_collector/dc_mod.rs | 39 +++++++++++++++++++ .../src/hir/resolution/import.rs | 18 +++++++++ .../mod_nr_entrypoint/Nargo.toml | 7 ++++ .../mod_nr_entrypoint/src/baz.nr | 3 ++ .../mod_nr_entrypoint/src/foo/bar.nr | 3 ++ .../mod_nr_entrypoint/src/foo/mod.nr | 3 ++ .../mod_nr_entrypoint/src/main.nr | 8 ++++ 9 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/Nargo.toml create mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr create mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/bar.nr create mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr create mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr diff --git a/compiler/fm/src/file_map.rs b/compiler/fm/src/file_map.rs index 50412d352ec..01ed30f7465 100644 --- a/compiler/fm/src/file_map.rs +++ b/compiler/fm/src/file_map.rs @@ -33,7 +33,8 @@ impl From<&PathBuf> for PathString { #[derive(Debug, Clone)] pub struct FileMap { files: SimpleFiles, - name_to_id: HashMap, + // TODO remove before PR + pub name_to_id: HashMap, } // XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 4aac0fec9c3..695f4b8e1c4 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -297,6 +297,9 @@ impl DefCollector { for collected_import in std::mem::take(&mut def_collector.imports) { match resolve_import(crate_id, &collected_import, &context.def_maps) { Ok(resolved_import) => { + // TODO cleanup + // dbg!("match resolve_import", &resolved_import); + if let Some(error) = resolved_import.error { errors.push(( DefCollectorErrorKind::PathResolutionError(error).into(), @@ -323,6 +326,9 @@ impl DefCollector { } } Err(error) => { + // TODO cleanup + // dbg!("match resolve_import(err)", &error); + let current_def_map = context.def_maps.get(&crate_id).unwrap(); let file_id = current_def_map.file_id(collected_import.module_id); let error = DefCollectorErrorKind::PathResolutionError(error); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index e688f192d3d..7910bd090f5 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -551,6 +551,12 @@ impl<'a> ModCollector<'a> { crate_id: CrateId, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { + // TODO cleanup + if crate_id != CrateId::Stdlib(0) { + dbg!("parse_module_declaration", mod_decl, crate_id); + panic!("hm"); + } + let mut errors: Vec<(CompilationError, FileId)> = vec![]; let child_file_id = match find_module(&context.file_manager, self.file_id, &mod_decl.ident.0.contents) { @@ -681,12 +687,30 @@ fn find_module( anchor: FileId, mod_name: &str, ) -> Result { + // TODO cleanup + let anchor_path = file_manager .path(anchor) .expect("File must exist in file manager in order for us to be resolving its imports.") .with_extension(""); + + // TODO cleanup + if true { + // dbg!("find_module", anchor, mod_name); + dbg!("find_module", mod_name, &anchor, &anchor_path); + dbg!("file_manager", &file_manager.as_file_map().name_to_id.iter().filter(|(x, _)| !x.to_string().contains("std")).collect::>()); + } let anchor_dir = anchor_path.parent().unwrap(); + // TODO cleanup + if true { + dbg!(&anchor_dir); + } + + if mod_name == "bar" { + panic!("ok?") + } + // if `anchor` is a `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, we check siblings of // the anchor at `base/mod_name.nr`. let candidate = if should_check_siblings_for_module(&anchor_path, anchor_dir) { @@ -696,6 +720,16 @@ fn find_module( anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")) }; + dbg!(&candidate); + // // TODO cleanup before PR + // if !candidate.display().to_string().contains("std") { + // panic!("{:?}", candidate); + // } + if file_manager.name_to_id(candidate.clone()).is_none() { + dbg!(&candidate); + panic!("hi!"); + } + file_manager .name_to_id(candidate.clone()) .ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) @@ -704,6 +738,11 @@ fn find_module( /// Returns true if a module's child modules are expected to be in the same directory. /// Returns false if they are expected to be in a subdirectory matching the name of the module. fn should_check_siblings_for_module(module_path: &Path, parent_path: &Path) -> bool { + // // // TODO cleanup + // // dbg!("should_check_siblings_for_module", module_path, parent_path); + // return true; + + if let Some(filename) = module_path.file_stem() { // This check also means a `main.nr` or `lib.nr` file outside of the crate root would // check its same directory for child modules instead of a subdirectory. Should we prohibit diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 343113836ed..9f76f399227 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -88,6 +88,10 @@ pub fn resolve_import( import_directive: &ImportDirective, def_maps: &BTreeMap, ) -> Result { + + // // TODO cleanup + // dbg!("resolve_import", crate_id, import_directive, def_maps.keys().collect::>()); + let allow_contracts = allow_referencing_contracts(def_maps, crate_id, import_directive.module_id); @@ -98,6 +102,9 @@ pub fn resolve_import( mut error, } = resolve_path_to_ns(import_directive, crate_id, crate_id, def_maps, allow_contracts)?; + // TODO cleanup + // dbg!("resolve_import: err", &error); + let name = resolve_path_name(import_directive); let visibility = resolved_namespace @@ -147,6 +154,9 @@ fn resolve_path_to_ns( let import_path = &import_directive.path.segments; let def_map = &def_maps[&crate_id]; + // TODO cleanup + // dbg!("resolve_path_to_ns", &import_path); + match import_directive.path.kind { crate::ast::PathKind::Crate => { // Resolve from the root of the crate @@ -221,6 +231,10 @@ fn resolve_name_in_module( } let first_segment = import_path.first().expect("ice: could not fetch first segment"); + + // TODO: cleanup + // dbg!("resolve_name_in_module (ran)", ¤t_mod, &first_segment); + let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); @@ -228,6 +242,10 @@ fn resolve_name_in_module( let mut warning: Option = None; for (last_segment, current_segment) in import_path.iter().zip(import_path.iter().skip(1)) { + + // TODO cleanup + // dbg!("resolve_name_in_module", &last_segment, ¤t_segment, ¤t_ns); + let (typ, visibility) = match current_ns.types { None => return Err(PathResolutionError::Unresolved(last_segment.clone())), Some((typ, visibility, _)) => (typ, visibility), diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/Nargo.toml b/test_programs/compile_success_empty/mod_nr_entrypoint/Nargo.toml new file mode 100644 index 00000000000..b90b7326186 --- /dev/null +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "mod_nr_entrypoint" +type = "bin" +authors = [""] +compiler_version = ">=0.29.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr new file mode 100644 index 00000000000..0779ce0b7f4 --- /dev/null +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr @@ -0,0 +1,3 @@ +fn in_baz_mod() -> bool { + true +} diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/bar.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/bar.nr new file mode 100644 index 00000000000..f2efe64906d --- /dev/null +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/bar.nr @@ -0,0 +1,3 @@ +pub fn in_bar_mod() -> Field { + 2 +} diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr new file mode 100644 index 00000000000..94b01045cbc --- /dev/null +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr @@ -0,0 +1,3 @@ +pub fn in_foo_mod() -> Field { + 1 +} diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr new file mode 100644 index 00000000000..c71569403a3 --- /dev/null +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr @@ -0,0 +1,8 @@ +use crate::foo::in_foo_mod; +use crate::foo::bar::in_bar_mod; +use crate::baz::in_baz_mod; + +fn main() { + // assert(in_foo_mod() != in_bar_mod()); + assert(in_baz_mod); +} From 0a682aa23d172dfd9d0bdaa5882f84b3c3166799 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 15 May 2024 22:00:39 -0400 Subject: [PATCH 02/41] feat: add skip_prelude hidden cli option, skip inject_prelude when true --- compiler/noirc_driver/src/lib.rs | 9 ++++++++- compiler/noirc_driver/tests/stdlib_warnings.rs | 2 +- .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 10 +++++++--- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 ++ tooling/lsp/src/lib.rs | 2 +- tooling/lsp/src/notifications/mod.rs | 4 ++-- tooling/lsp/src/requests/code_lens_request.rs | 2 +- tooling/lsp/src/requests/goto_declaration.rs | 2 +- tooling/lsp/src/requests/goto_definition.rs | 2 +- tooling/lsp/src/requests/test_run.rs | 2 +- tooling/lsp/src/requests/tests.rs | 2 +- tooling/nargo_cli/src/cli/check_cmd.rs | 4 +++- tooling/nargo_cli/src/cli/export_cmd.rs | 1 + tooling/nargo_cli/src/cli/test_cmd.rs | 2 ++ tooling/nargo_cli/tests/stdlib-tests.rs | 2 +- 15 files changed, 33 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 801c0b685a9..04364452e9e 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -107,6 +107,10 @@ pub struct CompileOptions { /// Enable the experimental elaborator pass #[arg(long, hide = true)] pub use_elaborator: bool, + + /// Skip loading the prelude (stdlib) + #[arg(long, hide = true)] + pub skip_prelude: bool, } fn parse_expression_width(input: &str) -> Result { @@ -250,12 +254,13 @@ pub fn check_crate( deny_warnings: bool, disable_macros: bool, use_elaborator: bool, + skip_prelude: bool, ) -> CompilationResult<()> { let macros: &[&dyn MacroProcessor] = if disable_macros { &[] } else { &[&aztec_macros::AztecMacro as &dyn MacroProcessor] }; let mut errors = vec![]; - let diagnostics = CrateDefMap::collect_defs(crate_id, context, use_elaborator, macros); + let diagnostics = CrateDefMap::collect_defs(crate_id, context, use_elaborator, skip_prelude, macros); errors.extend(diagnostics.into_iter().map(|(error, file_id)| { let diagnostic = CustomDiagnostic::from(&error); diagnostic.in_file(file_id) @@ -293,6 +298,7 @@ pub fn compile_main( options.deny_warnings, options.disable_macros, options.use_elaborator, + options.skip_prelude, )?; let main = context.get_main_function(&crate_id).ok_or_else(|| { @@ -334,6 +340,7 @@ pub fn compile_contract( options.deny_warnings, options.disable_macros, options.use_elaborator, + options.skip_prelude, )?; // TODO: We probably want to error if contracts is empty diff --git a/compiler/noirc_driver/tests/stdlib_warnings.rs b/compiler/noirc_driver/tests/stdlib_warnings.rs index 327c8daad06..f8c30ce5c7a 100644 --- a/compiler/noirc_driver/tests/stdlib_warnings.rs +++ b/compiler/noirc_driver/tests/stdlib_warnings.rs @@ -25,7 +25,7 @@ fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings> let root_crate_id = prepare_crate(&mut context, file_name); let ((), warnings) = - noirc_driver::check_crate(&mut context, root_crate_id, false, false, false)?; + noirc_driver::check_crate(&mut context, root_crate_id, false, false, false, false)?; assert_eq!(warnings, Vec::new(), "stdlib is producing warnings"); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 4aac0fec9c3..0a531a9cc12 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -237,6 +237,7 @@ impl DefCollector { ast: SortedModule, root_file_id: FileId, use_elaborator: bool, + skip_prelude: bool, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; @@ -254,6 +255,7 @@ impl DefCollector { dep.crate_id, context, use_elaborator, + skip_prelude, macro_processors, )); @@ -288,9 +290,11 @@ impl DefCollector { // Add the current crate to the collection of DefMaps context.def_maps.insert(crate_id, def_collector.def_map); - inject_prelude(crate_id, context, crate_root, &mut def_collector.imports); - for submodule in submodules { - inject_prelude(crate_id, context, LocalModuleId(submodule), &mut def_collector.imports); + if !skip_prelude { + inject_prelude(crate_id, context, crate_root, &mut def_collector.imports); + for submodule in submodules { + inject_prelude(crate_id, context, LocalModuleId(submodule), &mut def_collector.imports); + } } // Resolve unresolved imports collected from the crate, one by one. diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 19e06387d43..30117d6ba99 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -74,6 +74,7 @@ impl CrateDefMap { crate_id: CrateId, context: &mut Context, use_elaborator: bool, + skip_prelude: bool, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { // Check if this Crate has already been compiled @@ -123,6 +124,7 @@ impl CrateDefMap { ast, root_file_id, use_elaborator, + skip_prelude, macro_processors, )); diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 05345b96c80..7c194989c22 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -345,7 +345,7 @@ fn prepare_package_from_source_string() { let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver); let (mut context, crate_id) = crate::prepare_source(source.to_string(), &mut state); - let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false, false); + let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); let main_func_id = context.get_main_function(&crate_id); assert!(main_func_id.is_some()); } diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 3856bdc79e9..6f8f60a82b8 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -56,7 +56,7 @@ pub(super) fn on_did_change_text_document( state.input_files.insert(params.text_document.uri.to_string(), text.clone()); let (mut context, crate_id) = prepare_source(text, state); - let _ = check_crate(&mut context, crate_id, false, false, false); + let _ = check_crate(&mut context, crate_id, false, false, false, false); let workspace = match resolve_workspace_for_source_path( params.text_document.uri.to_file_path().unwrap().as_path(), @@ -139,7 +139,7 @@ fn process_noir_document( let (mut context, crate_id) = prepare_package(&workspace_file_manager, &parsed_files, package); - let file_diagnostics = match check_crate(&mut context, crate_id, false, false, false) { + let file_diagnostics = match check_crate(&mut context, crate_id, false, false, false, false) { Ok(((), warnings)) => warnings, Err(errors_and_warnings) => errors_and_warnings, }; diff --git a/tooling/lsp/src/requests/code_lens_request.rs b/tooling/lsp/src/requests/code_lens_request.rs index 744bddedd9d..d5da7cc412f 100644 --- a/tooling/lsp/src/requests/code_lens_request.rs +++ b/tooling/lsp/src/requests/code_lens_request.rs @@ -67,7 +67,7 @@ fn on_code_lens_request_inner( let (mut context, crate_id) = prepare_source(source_string, state); // We ignore the warnings and errors produced by compilation for producing code lenses // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false, false, false); + let _ = check_crate(&mut context, crate_id, false, false, false, false); let collected_lenses = collect_lenses_for_package(&context, crate_id, &workspace, package, None); diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 5cff16b2348..cae19238170 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -46,7 +46,7 @@ fn on_goto_definition_inner( interner = def_interner; } else { // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); interner = &context.def_interner; } diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 32e13ce00f6..7161e2c2e9a 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -54,7 +54,7 @@ fn on_goto_definition_inner( interner = def_interner; } else { // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); interner = &context.def_interner; } diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 83b05ba06a2..1a88b8c7c52 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -60,7 +60,7 @@ fn on_test_run_request_inner( Some(package) => { let (mut context, crate_id) = prepare_package(&workspace_file_manager, &parsed_files, package); - if check_crate(&mut context, crate_id, false, false, false).is_err() { + if check_crate(&mut context, crate_id, false, false, false, false).is_err() { let result = NargoTestRunResult { id: params.id.clone(), result: "error".to_string(), diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index cdf4ad338c4..28e4d8cf738 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -61,7 +61,7 @@ fn on_tests_request_inner( prepare_package(&workspace_file_manager, &parsed_files, package); // We ignore the warnings and errors produced by compilation for producing tests // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false, false, false); + let _ = check_crate(&mut context, crate_id, false, false, false, false); // We don't add test headings for a package if it contains no `#[test]` functions get_package_tests_in_crate(&context, &crate_id, &package.name) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index d5313d96076..9a5b8509eeb 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -88,6 +88,7 @@ fn check_package( compile_options.disable_macros, compile_options.silence_warnings, compile_options.use_elaborator, + compile_options.skip_prelude, )?; if package.is_library() || package.is_contract() { @@ -175,8 +176,9 @@ pub(crate) fn check_crate_and_report_errors( disable_macros: bool, silence_warnings: bool, use_elaborator: bool, + skip_prelude: bool, ) -> Result<(), CompileError> { - let result = check_crate(context, crate_id, deny_warnings, disable_macros, use_elaborator); + let result = check_crate(context, crate_id, deny_warnings, disable_macros, use_elaborator, skip_prelude); report_errors(result, &context.file_manager, deny_warnings, silence_warnings) } diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index 324eed340ad..d2fcbd6f259 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -90,6 +90,7 @@ fn compile_exported_functions( compile_options.disable_macros, compile_options.silence_warnings, compile_options.use_elaborator, + compile_options.skip_prelude, )?; let exported_functions = context.get_all_exported_functions_in_crate(&crate_id); diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 51e21248afd..f25e0a5d8f7 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -176,6 +176,7 @@ fn run_test( compile_options.deny_warnings, compile_options.disable_macros, compile_options.use_elaborator, + compile_options.skip_prelude, ) .expect("Any errors should have occurred when collecting test functions"); @@ -210,6 +211,7 @@ fn get_tests_in_package( compile_options.disable_macros, compile_options.silence_warnings, compile_options.use_elaborator, + compile_options.skip_prelude, )?; Ok(context diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 70a9354f50a..7b05a0212a2 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -29,7 +29,7 @@ fn run_stdlib_tests(use_elaborator: bool) { let (mut context, dummy_crate_id) = prepare_package(&file_manager, &parsed_files, &dummy_package); - let result = check_crate(&mut context, dummy_crate_id, true, false, use_elaborator); + let result = check_crate(&mut context, dummy_crate_id, true, false, use_elaborator, false); report_errors(result, &context.file_manager, true, false) .expect("Error encountered while compiling standard library"); From 0f682a3d499873eaf17895630941bf5e8ab09526 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 15 May 2024 22:05:10 -0400 Subject: [PATCH 03/41] cargo fmt / clippy --- compiler/noirc_driver/src/lib.rs | 3 ++- .../noirc_frontend/src/hir/def_collector/dc_crate.rs | 7 ++++++- tooling/lsp/src/lib.rs | 3 ++- tooling/lsp/src/notifications/mod.rs | 9 +++++---- tooling/nargo_cli/src/cli/check_cmd.rs | 3 ++- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 04364452e9e..6aa24a0e2df 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -260,7 +260,8 @@ pub fn check_crate( if disable_macros { &[] } else { &[&aztec_macros::AztecMacro as &dyn MacroProcessor] }; let mut errors = vec![]; - let diagnostics = CrateDefMap::collect_defs(crate_id, context, use_elaborator, skip_prelude, macros); + let diagnostics = + CrateDefMap::collect_defs(crate_id, context, use_elaborator, skip_prelude, macros); errors.extend(diagnostics.into_iter().map(|(error, file_id)| { let diagnostic = CustomDiagnostic::from(&error); diagnostic.in_file(file_id) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 0a531a9cc12..55c63f187a4 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -293,7 +293,12 @@ impl DefCollector { if !skip_prelude { inject_prelude(crate_id, context, crate_root, &mut def_collector.imports); for submodule in submodules { - inject_prelude(crate_id, context, LocalModuleId(submodule), &mut def_collector.imports); + inject_prelude( + crate_id, + context, + LocalModuleId(submodule), + &mut def_collector.imports, + ); } } diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 7c194989c22..b58f7864533 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -345,7 +345,8 @@ fn prepare_package_from_source_string() { let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver); let (mut context, crate_id) = crate::prepare_source(source.to_string(), &mut state); - let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); + let _check_result = + noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); let main_func_id = context.get_main_function(&crate_id); assert!(main_func_id.is_some()); } diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 6f8f60a82b8..3278e5eaef9 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -139,10 +139,11 @@ fn process_noir_document( let (mut context, crate_id) = prepare_package(&workspace_file_manager, &parsed_files, package); - let file_diagnostics = match check_crate(&mut context, crate_id, false, false, false, false) { - Ok(((), warnings)) => warnings, - Err(errors_and_warnings) => errors_and_warnings, - }; + let file_diagnostics = + match check_crate(&mut context, crate_id, false, false, false, false) { + Ok(((), warnings)) => warnings, + Err(errors_and_warnings) => errors_and_warnings, + }; let package_root_dir: String = package.root_dir.as_os_str().to_string_lossy().into(); diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 9a5b8509eeb..1c3493b4d8a 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -178,7 +178,8 @@ pub(crate) fn check_crate_and_report_errors( use_elaborator: bool, skip_prelude: bool, ) -> Result<(), CompileError> { - let result = check_crate(context, crate_id, deny_warnings, disable_macros, use_elaborator, skip_prelude); + let result = + check_crate(context, crate_id, deny_warnings, disable_macros, use_elaborator, skip_prelude); report_errors(result, &context.file_manager, deny_warnings, silence_warnings) } From a5c4c908ea89a9d3f861df855bbd72168e9968d5 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 15 May 2024 22:59:59 -0400 Subject: [PATCH 04/41] add missing parameter --- compiler/noirc_frontend/src/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index fb80a7d8018..583e0afd22c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -82,6 +82,7 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation program.clone().into_sorted(), root_file_id, false, + false, &[], // No macro processors )); } From ea02811c3c0b494dc42c89d9bf7ea24a0f728bc0 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 16 May 2024 10:41:45 -0400 Subject: [PATCH 05/41] wip debugging --- .../src/hir/def_collector/dc_mod.rs | 4 +++- .../noirc_frontend/src/hir/resolution/import.rs | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 7910bd090f5..2488a01d35a 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -699,6 +699,8 @@ fn find_module( // dbg!("find_module", anchor, mod_name); dbg!("find_module", mod_name, &anchor, &anchor_path); dbg!("file_manager", &file_manager.as_file_map().name_to_id.iter().filter(|(x, _)| !x.to_string().contains("std")).collect::>()); + + dbg!(file_manager.path(anchor)); } let anchor_dir = anchor_path.parent().unwrap(); @@ -707,7 +709,7 @@ fn find_module( dbg!(&anchor_dir); } - if mod_name == "bar" { + if mod_name == "main" || mod_name == "foo" || mod_name == "bar" || anchor_dir.display().to_string().contains("mod_nr_entrypoint") { panic!("ok?") } diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 9f76f399227..e06ca2d6624 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -237,6 +237,12 @@ fn resolve_name_in_module( let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { + dbg!("this?", krate, importing_crate, import_path, starting_mod); + if matches!(krate, CrateId::Root(..)) { + panic!("this.") + } + // panic!("ok?"); + return Err(PathResolutionError::Unresolved(first_segment.clone())); } @@ -247,7 +253,10 @@ fn resolve_name_in_module( // dbg!("resolve_name_in_module", &last_segment, ¤t_segment, ¤t_ns); let (typ, visibility) = match current_ns.types { - None => return Err(PathResolutionError::Unresolved(last_segment.clone())), + None => { + panic!("okkk?"); + return Err(PathResolutionError::Unresolved(last_segment.clone())) + }, Some((typ, visibility, _)) => (typ, visibility), }; @@ -282,6 +291,7 @@ fn resolve_name_in_module( let found_ns = current_mod.find_name(current_segment); if found_ns.is_none() { + panic!("kk"); return Err(PathResolutionError::Unresolved(current_segment.clone())); } @@ -317,7 +327,9 @@ fn resolve_external_dep( let dep_module = current_def_map .extern_prelude .get(&crate_name.0.contents) - .ok_or_else(|| PathResolutionError::Unresolved(crate_name.to_owned()))?; + .ok_or_else(|| { + panic!("external"); PathResolutionError::Unresolved(crate_name.to_owned()) + })?; // Create an import directive for the dependency crate let path_without_crate_name = &path[1..]; // XXX: This will panic if the path is of the form `use dep::std` Ideal algorithm will not distinguish between crate and module From fd76bd838a5d1b237451e52d0fe56e6ff2b47ac1 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 16 May 2024 10:50:51 -0400 Subject: [PATCH 06/41] wip debugging: find_module doesn't appear to get called for import? --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 5 +++-- compiler/noirc_frontend/src/hir/resolution/import.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 2488a01d35a..61dbcec53fa 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -694,15 +694,16 @@ fn find_module( .expect("File must exist in file manager in order for us to be resolving its imports.") .with_extension(""); + let anchor_dir = anchor_path.parent().unwrap(); + // TODO cleanup - if true { + if !(anchor_path.display().to_string().contains("std") || anchor_dir.display().to_string().contains("std")) { // dbg!("find_module", anchor, mod_name); dbg!("find_module", mod_name, &anchor, &anchor_path); dbg!("file_manager", &file_manager.as_file_map().name_to_id.iter().filter(|(x, _)| !x.to_string().contains("std")).collect::>()); dbg!(file_manager.path(anchor)); } - let anchor_dir = anchor_path.parent().unwrap(); // TODO cleanup if true { diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index e06ca2d6624..4dad79b05ba 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -237,8 +237,8 @@ fn resolve_name_in_module( let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { - dbg!("this?", krate, importing_crate, import_path, starting_mod); if matches!(krate, CrateId::Root(..)) { + dbg!("this?", krate, importing_crate, import_path, starting_mod); panic!("this.") } // panic!("ok?"); From e35a630b6f5e9a79cce4c05e0ba6d835cd4aa3c9 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 16 May 2024 11:05:03 -0400 Subject: [PATCH 07/41] debugging (def maps) --- compiler/noirc_frontend/src/hir/resolution/import.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 4dad79b05ba..94669f838a3 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -239,6 +239,7 @@ fn resolve_name_in_module( if current_ns.is_none() { if matches!(krate, CrateId::Root(..)) { dbg!("this?", krate, importing_crate, import_path, starting_mod); + dbg!(def_maps.values().map(|x| x.modules.iter().collect::>()).collect::>()); panic!("this.") } // panic!("ok?"); From f214832dec26c48dcd60fecd9421cc3856b823a4 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 16 May 2024 12:48:25 -0400 Subject: [PATCH 08/41] wip debugging --- .../src/hir/def_collector/dc_mod.rs | 63 ++++++++++++++----- .../src/hir/resolution/import.rs | 10 +-- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 61dbcec53fa..4932e0fd608 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -688,6 +688,7 @@ fn find_module( mod_name: &str, ) -> Result { // TODO cleanup + dbg!("file_manager", &file_manager.as_file_map().name_to_id.iter().filter(|(x, _)| !x.to_string().contains("std")).collect::>()); let anchor_path = file_manager .path(anchor) @@ -716,26 +717,54 @@ fn find_module( // if `anchor` is a `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, we check siblings of // the anchor at `base/mod_name.nr`. - let candidate = if should_check_siblings_for_module(&anchor_path, anchor_dir) { - anchor_dir.join(format!("{mod_name}.{FILE_EXTENSION}")) + + + let mod_nr_candidate = anchor_dir.join(mod_name).join(format!("mod.{FILE_EXTENSION}")); + let parent_candidate = anchor_dir.join(format!("{mod_name}.{FILE_EXTENSION}")); + let child_candidate = anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")); + + let mod_nr_result = file_manager + .name_to_id(mod_nr_candidate.clone()) + .ok_or_else(|| mod_nr_candidate.as_os_str().to_string_lossy().to_string()); + let parent_result = file_manager + .name_to_id(parent_candidate.clone()) + .ok_or_else(|| parent_candidate.as_os_str().to_string_lossy().to_string()); + let child_result = file_manager + .name_to_id(child_candidate.clone()) + .ok_or_else(|| child_candidate.as_os_str().to_string_lossy().to_string()); + + if mod_nr_result.is_ok() && parent_result.is_err() && child_result.is_err() { + mod_nr_result + } else if mod_nr_result.is_err() && parent_result.is_ok() && child_result.is_err() { + parent_result + } else if mod_nr_result.is_err() && parent_result.is_err() && child_result.is_ok() { + child_result } else { - // Otherwise, we check for children of the anchor at `base/anchor/mod_name.nr` - anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")) - }; - - dbg!(&candidate); - // // TODO cleanup before PR - // if !candidate.display().to_string().contains("std") { - // panic!("{:?}", candidate); - // } - if file_manager.name_to_id(candidate.clone()).is_none() { - dbg!(&candidate); - panic!("hi!"); + Err(mod_nr_candidate.as_os_str().to_string_lossy().to_string()) } - file_manager - .name_to_id(candidate.clone()) - .ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) + + // let candidate = if true { + // } else if should_check_siblings_for_module(&anchor_path, anchor_dir) { + // anchor_dir.join(format!("{mod_name}.{FILE_EXTENSION}")) + // } else { + // // Otherwise, we check for children of the anchor at `base/anchor/mod_name.nr` + // anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")) + // }; + // + // // dbg!(&candidate); + // // // // TODO cleanup before PR + // // // if !candidate.display().to_string().contains("std") { + // // // panic!("{:?}", candidate); + // // // } + // // if file_manager.name_to_id(candidate.clone()).is_none() { + // // dbg!(&candidate); + // // panic!("hi!"); + // // } + // + // file_manager + // .name_to_id(candidate.clone()) + // .ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) } /// Returns true if a module's child modules are expected to be in the same directory. diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 94669f838a3..715d1e5d625 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -237,11 +237,11 @@ fn resolve_name_in_module( let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { - if matches!(krate, CrateId::Root(..)) { - dbg!("this?", krate, importing_crate, import_path, starting_mod); - dbg!(def_maps.values().map(|x| x.modules.iter().collect::>()).collect::>()); - panic!("this.") - } + // if matches!(krate, CrateId::Root(..)) { + // dbg!("this?", krate, importing_crate, import_path, starting_mod); + // dbg!(def_maps.values().map(|x| x.modules.iter().collect::>()).collect::>()); + // panic!("this.") + // } // panic!("ok?"); return Err(PathResolutionError::Unresolved(first_segment.clone())); From fa75c8b8234a633ddb338dcb2bcdc5f82e0fc6a5 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 10 Jun 2024 14:33:08 -0400 Subject: [PATCH 09/41] debugging cleanup --- .../src/hir/def_collector/dc_mod.rs | 83 ++----------------- .../src/hir/resolution/import.rs | 18 +--- 2 files changed, 7 insertions(+), 94 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 583e4715dae..fd11f46baac 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, path::Path, vec}; +use std::{collections::HashMap, vec}; use acvm::{AcirField, FieldElement}; use fm::{FileId, FileManager, FILE_EXTENSION}; @@ -571,11 +571,11 @@ impl<'a> ModCollector<'a> { crate_id: CrateId, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { - // TODO cleanup - if crate_id != CrateId::Stdlib(0) { - dbg!("parse_module_declaration", mod_decl, crate_id); - panic!("hm"); - } + // // TODO cleanup + // if crate_id != CrateId::Stdlib(0) { + // dbg!("parse_module_declaration", mod_decl, crate_id); + // panic!("hm"); + // } let mut errors: Vec<(CompilationError, FileId)> = vec![]; let child_file_id = @@ -707,38 +707,14 @@ fn find_module( anchor: FileId, mod_name: &str, ) -> Result { - // TODO cleanup - dbg!("file_manager", &file_manager.as_file_map().name_to_id.iter().filter(|(x, _)| !x.to_string().contains("std")).collect::>()); - let anchor_path = file_manager .path(anchor) .expect("File must exist in file manager in order for us to be resolving its imports.") .with_extension(""); - let anchor_dir = anchor_path.parent().unwrap(); - // TODO cleanup - if !(anchor_path.display().to_string().contains("std") || anchor_dir.display().to_string().contains("std")) { - // dbg!("find_module", anchor, mod_name); - dbg!("find_module", mod_name, &anchor, &anchor_path); - dbg!("file_manager", &file_manager.as_file_map().name_to_id.iter().filter(|(x, _)| !x.to_string().contains("std")).collect::>()); - - dbg!(file_manager.path(anchor)); - } - - // TODO cleanup - if true { - dbg!(&anchor_dir); - } - - if mod_name == "main" || mod_name == "foo" || mod_name == "bar" || anchor_dir.display().to_string().contains("mod_nr_entrypoint") { - panic!("ok?") - } - // if `anchor` is a `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, we check siblings of // the anchor at `base/mod_name.nr`. - - let mod_nr_candidate = anchor_dir.join(mod_name).join(format!("mod.{FILE_EXTENSION}")); let parent_candidate = anchor_dir.join(format!("{mod_name}.{FILE_EXTENSION}")); let child_candidate = anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")); @@ -762,53 +738,6 @@ fn find_module( } else { Err(mod_nr_candidate.as_os_str().to_string_lossy().to_string()) } - - - // let candidate = if true { - // } else if should_check_siblings_for_module(&anchor_path, anchor_dir) { - // anchor_dir.join(format!("{mod_name}.{FILE_EXTENSION}")) - // } else { - // // Otherwise, we check for children of the anchor at `base/anchor/mod_name.nr` - // anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")) - // }; - // - // // dbg!(&candidate); - // // // // TODO cleanup before PR - // // // if !candidate.display().to_string().contains("std") { - // // // panic!("{:?}", candidate); - // // // } - // // if file_manager.name_to_id(candidate.clone()).is_none() { - // // dbg!(&candidate); - // // panic!("hi!"); - // // } - // - // file_manager - // .name_to_id(candidate.clone()) - // .ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) -} - -/// Returns true if a module's child modules are expected to be in the same directory. -/// Returns false if they are expected to be in a subdirectory matching the name of the module. -fn should_check_siblings_for_module(module_path: &Path, parent_path: &Path) -> bool { - // // // TODO cleanup - // // dbg!("should_check_siblings_for_module", module_path, parent_path); - // return true; - - - if let Some(filename) = module_path.file_stem() { - // This check also means a `main.nr` or `lib.nr` file outside of the crate root would - // check its same directory for child modules instead of a subdirectory. Should we prohibit - // `main.nr` and `lib.nr` files outside of the crate root? - filename == "main" - || filename == "lib" - || filename == "mod" - || Some(filename) == parent_path.file_stem() - } else { - // If there's no filename, we arbitrarily return true. - // Alternatively, we could panic, but this is left to a different step where we - // ideally have some source location to issue an error. - true - } } cfg_if::cfg_if! { diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 715d1e5d625..4ca2189aff0 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -232,30 +232,15 @@ fn resolve_name_in_module( let first_segment = import_path.first().expect("ice: could not fetch first segment"); - // TODO: cleanup - // dbg!("resolve_name_in_module (ran)", ¤t_mod, &first_segment); - let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { - // if matches!(krate, CrateId::Root(..)) { - // dbg!("this?", krate, importing_crate, import_path, starting_mod); - // dbg!(def_maps.values().map(|x| x.modules.iter().collect::>()).collect::>()); - // panic!("this.") - // } - // panic!("ok?"); - return Err(PathResolutionError::Unresolved(first_segment.clone())); } let mut warning: Option = None; for (last_segment, current_segment) in import_path.iter().zip(import_path.iter().skip(1)) { - - // TODO cleanup - // dbg!("resolve_name_in_module", &last_segment, ¤t_segment, ¤t_ns); - let (typ, visibility) = match current_ns.types { None => { - panic!("okkk?"); return Err(PathResolutionError::Unresolved(last_segment.clone())) }, Some((typ, visibility, _)) => (typ, visibility), @@ -292,7 +277,6 @@ fn resolve_name_in_module( let found_ns = current_mod.find_name(current_segment); if found_ns.is_none() { - panic!("kk"); return Err(PathResolutionError::Unresolved(current_segment.clone())); } @@ -329,7 +313,7 @@ fn resolve_external_dep( .extern_prelude .get(&crate_name.0.contents) .ok_or_else(|| { - panic!("external"); PathResolutionError::Unresolved(crate_name.to_owned()) + PathResolutionError::Unresolved(crate_name.to_owned()) })?; // Create an import directive for the dependency crate From 3357176cc4220e7fd995f35350fd93632d906587 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jun 2024 16:01:33 +0000 Subject: [PATCH 10/41] chore: fmt --- compiler/noirc_frontend/src/hir/resolution/import.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 4ca2189aff0..28266209b85 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -88,7 +88,6 @@ pub fn resolve_import( import_directive: &ImportDirective, def_maps: &BTreeMap, ) -> Result { - // // TODO cleanup // dbg!("resolve_import", crate_id, import_directive, def_maps.keys().collect::>()); @@ -240,9 +239,7 @@ fn resolve_name_in_module( let mut warning: Option = None; for (last_segment, current_segment) in import_path.iter().zip(import_path.iter().skip(1)) { let (typ, visibility) = match current_ns.types { - None => { - return Err(PathResolutionError::Unresolved(last_segment.clone())) - }, + None => return Err(PathResolutionError::Unresolved(last_segment.clone())), Some((typ, visibility, _)) => (typ, visibility), }; @@ -312,9 +309,7 @@ fn resolve_external_dep( let dep_module = current_def_map .extern_prelude .get(&crate_name.0.contents) - .ok_or_else(|| { - PathResolutionError::Unresolved(crate_name.to_owned()) - })?; + .ok_or_else(|| PathResolutionError::Unresolved(crate_name.to_owned()))?; // Create an import directive for the dependency crate let path_without_crate_name = &path[1..]; // XXX: This will panic if the path is of the form `use dep::std` Ideal algorithm will not distinguish between crate and module From 84073600af2122c76ae0d0e41a58ad9a93b0f899 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jun 2024 16:24:48 +0000 Subject: [PATCH 11/41] chore: fix compilation --- tooling/lsp/src/requests/goto_declaration.rs | 2 +- tooling/lsp/src/requests/goto_definition.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 4ffe6abf88a..97c79cea428 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -45,7 +45,7 @@ fn on_goto_definition_inner( def_interner } else { // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); &context.def_interner }; diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 3a92e28cc11..12bf90377f9 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -53,7 +53,7 @@ fn on_goto_definition_inner( def_interner } else { // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); &context.def_interner }; From c48932f915854da0d9c63ada63595aa7041754f5 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 11 Jun 2024 12:34:29 -0400 Subject: [PATCH 12/41] add missing "mod foo;" declarations: tests passing --- .../compile_success_empty/mod_nr_entrypoint/Prover.toml | 0 .../compile_success_empty/mod_nr_entrypoint/src/baz.nr | 2 +- .../compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr | 2 ++ .../compile_success_empty/mod_nr_entrypoint/src/main.nr | 7 +++++-- 4 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml b/test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr index 0779ce0b7f4..0bcfe8b02ad 100644 --- a/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/baz.nr @@ -1,3 +1,3 @@ -fn in_baz_mod() -> bool { +pub fn in_baz_mod() -> bool { true } diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr index 94b01045cbc..4eac6cb8514 100644 --- a/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/foo/mod.nr @@ -1,3 +1,5 @@ +mod bar; + pub fn in_foo_mod() -> Field { 1 } diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr index c71569403a3..620fd99f6ec 100644 --- a/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr @@ -2,7 +2,10 @@ use crate::foo::in_foo_mod; use crate::foo::bar::in_bar_mod; use crate::baz::in_baz_mod; +mod foo; +mod baz; + fn main() { - // assert(in_foo_mod() != in_bar_mod()); - assert(in_baz_mod); + assert(in_foo_mod() != in_bar_mod()); + assert(in_baz_mod()); } From 3ff48847e4a4fd6eeeafb8df11e718153141de76 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 11 Jun 2024 16:31:49 -0400 Subject: [PATCH 13/41] nargo fmt, add error for overlapping paths, restrict 'main' in path, cleanup --- .../src/hir/def_collector/dc_crate.rs | 6 -- .../src/hir/def_collector/dc_mod.rs | 74 +++++++++++-------- .../src/hir/def_collector/errors.rs | 13 ++++ .../invalid_main_sub_lib/Nargo.toml | 7 ++ .../invalid_main_sub_lib/Prover.toml | 0 .../invalid_main_sub_lib/src/main.nr | 5 ++ .../invalid_main_sub_lib/src/main/lib.nr | 3 + .../invalid_mod_mod_path/Nargo.toml | 7 ++ .../invalid_mod_mod_path/Prover.toml | 2 + .../invalid_mod_mod_path/main/lib.nr | 3 + .../invalid_mod_mod_path/src/main.nr | 4 + .../invalid_mod_mod_path/src/mod.nr | 3 + .../overlapping_dep_and_mod/Nargo.toml | 7 ++ .../overlapping_dep_and_mod/Prover.toml | 0 .../overlapping_dep_and_mod/src/lib.nr | 3 + .../overlapping_dep_and_mod/src/lib/mod.nr | 3 + .../overlapping_dep_and_mod/src/main.nr | 6 ++ .../overlapping_lib_and_mod/Nargo.toml | 7 ++ .../overlapping_lib_and_mod/Prover.toml | 0 .../overlapping_lib_and_mod/src/foo/mod.nr | 3 + .../overlapping_lib_and_mod/src/main.nr | 7 ++ .../overlapping_lib_and_mod/src/main/foo.nr | 4 + .../compile_success_empty/vectors/Prover.toml | 4 +- tooling/nargo_cli/tests/stdlib-tests.rs | 2 +- 24 files changed, 135 insertions(+), 38 deletions(-) create mode 100644 test_programs/compile_failure/invalid_main_sub_lib/Nargo.toml create mode 100644 test_programs/compile_failure/invalid_main_sub_lib/Prover.toml create mode 100644 test_programs/compile_failure/invalid_main_sub_lib/src/main.nr create mode 100644 test_programs/compile_failure/invalid_main_sub_lib/src/main/lib.nr create mode 100644 test_programs/compile_failure/invalid_mod_mod_path/Nargo.toml create mode 100644 test_programs/compile_failure/invalid_mod_mod_path/Prover.toml create mode 100644 test_programs/compile_failure/invalid_mod_mod_path/main/lib.nr create mode 100644 test_programs/compile_failure/invalid_mod_mod_path/src/main.nr create mode 100644 test_programs/compile_failure/invalid_mod_mod_path/src/mod.nr create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/Prover.toml create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr create mode 100644 test_programs/compile_failure/overlapping_lib_and_mod/Nargo.toml create mode 100644 test_programs/compile_failure/overlapping_lib_and_mod/Prover.toml create mode 100644 test_programs/compile_failure/overlapping_lib_and_mod/src/foo/mod.nr create mode 100644 test_programs/compile_failure/overlapping_lib_and_mod/src/main.nr create mode 100644 test_programs/compile_failure/overlapping_lib_and_mod/src/main/foo.nr diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 4cb5b6b43bb..b836262ae82 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -326,9 +326,6 @@ impl DefCollector { for collected_import in std::mem::take(&mut def_collector.imports) { match resolve_import(crate_id, &collected_import, &context.def_maps) { Ok(resolved_import) => { - // TODO cleanup - // dbg!("match resolve_import", &resolved_import); - if let Some(error) = resolved_import.error { errors.push(( DefCollectorErrorKind::PathResolutionError(error).into(), @@ -355,9 +352,6 @@ impl DefCollector { } } Err(error) => { - // TODO cleanup - // dbg!("match resolve_import(err)", &error); - let current_def_map = context.def_maps.get(&crate_id).unwrap(); let file_id = current_def_map.file_id(collected_import.module_id); let error = DefCollectorErrorKind::PathResolutionError(error); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index fd11f46baac..6414a89815b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, vec}; +use std::{collections::HashMap, ffi::OsStr, vec}; use acvm::{AcirField, FieldElement}; use fm::{FileId, FileManager, FILE_EXTENSION}; @@ -571,20 +571,11 @@ impl<'a> ModCollector<'a> { crate_id: CrateId, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { - // // TODO cleanup - // if crate_id != CrateId::Stdlib(0) { - // dbg!("parse_module_declaration", mod_decl, crate_id); - // panic!("hm"); - // } - let mut errors: Vec<(CompilationError, FileId)> = vec![]; let child_file_id = - match find_module(&context.file_manager, self.file_id, &mod_decl.ident.0.contents) { + match find_module(&context.file_manager, self.file_id, mod_decl.ident.clone()) { Ok(child_file_id) => child_file_id, - Err(expected_path) => { - let mod_name = mod_decl.ident.clone(); - let err = - DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path }; + Err(err) => { errors.push((err.into(), self.file_id)); return errors; } @@ -705,8 +696,8 @@ impl<'a> ModCollector<'a> { fn find_module( file_manager: &FileManager, anchor: FileId, - mod_name: &str, -) -> Result { + mod_name: Ident, +) -> Result { let anchor_path = file_manager .path(anchor) .expect("File must exist in file manager in order for us to be resolving its imports.") @@ -715,9 +706,10 @@ fn find_module( // if `anchor` is a `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, we check siblings of // the anchor at `base/mod_name.nr`. - let mod_nr_candidate = anchor_dir.join(mod_name).join(format!("mod.{FILE_EXTENSION}")); - let parent_candidate = anchor_dir.join(format!("{mod_name}.{FILE_EXTENSION}")); - let child_candidate = anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")); + let mod_name_str = mod_name.0.contents.clone(); + let mod_nr_candidate = anchor_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")); + let parent_candidate = anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")); + let child_candidate = anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")); let mod_nr_result = file_manager .name_to_id(mod_nr_candidate.clone()) @@ -725,18 +717,42 @@ fn find_module( let parent_result = file_manager .name_to_id(parent_candidate.clone()) .ok_or_else(|| parent_candidate.as_os_str().to_string_lossy().to_string()); - let child_result = file_manager - .name_to_id(child_candidate.clone()) - .ok_or_else(|| child_candidate.as_os_str().to_string_lossy().to_string()); - - if mod_nr_result.is_ok() && parent_result.is_err() && child_result.is_err() { - mod_nr_result - } else if mod_nr_result.is_err() && parent_result.is_ok() && child_result.is_err() { - parent_result - } else if mod_nr_result.is_err() && parent_result.is_err() && child_result.is_ok() { - child_result - } else { - Err(mod_nr_candidate.as_os_str().to_string_lossy().to_string()) + + let mut path_results = vec![mod_nr_result, parent_result]; + let anchor_path_suffix = anchor_path + .as_path() + .file_name() + .unwrap_or_else(|| OsStr::new("")) + .to_string_lossy() + .to_string(); + if anchor_path_suffix != "main" { + let child_result = file_manager + .name_to_id(child_candidate.clone()) + .ok_or_else(|| child_candidate.as_os_str().to_string_lossy().to_string()); + path_results.push(child_result); + } + + let found_paths: Vec<_> = path_results.into_iter().flat_map(|result| result.ok()).collect(); + match found_paths.len() { + 0 => { + let expected_path = parent_candidate.as_os_str().to_string_lossy().to_string(); + Err(DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path }) + } + 1 => Ok(found_paths[0]), + _ => { + let overlapping_paths: Vec<_> = found_paths + .into_iter() + .map(|found_path| { + file_manager + .path(found_path) + .expect("all modules to be in the file manager") + .as_os_str() + .to_string_lossy() + .to_string() + }) + .collect(); + Err(DefCollectorErrorKind::OverlappingModuleDecls { mod_name, overlapping_paths }) + } } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index edeb463e10d..5eedfaa9b85 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -28,6 +28,8 @@ pub enum DefCollectorErrorKind { Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident }, #[error("unresolved import")] UnresolvedModuleDecl { mod_name: Ident, expected_path: String }, + #[error("overlapping imports")] + OverlappingModuleDecls { mod_name: Ident, overlapping_paths: Vec }, #[error("path resolution error")] PathResolutionError(PathResolutionError), #[error("Non-struct type used in impl")] @@ -129,6 +131,17 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { span, ) } + DefCollectorErrorKind::OverlappingModuleDecls { mod_name, overlapping_paths } => { + let span = mod_name.0.span(); + let mod_name = &mod_name.0.contents; + let overlapping_paths: String = overlapping_paths.iter().map(|overlapping_path| overlapping_path.to_owned() + "\n").collect(); + + Diagnostic::simple_error( + format!("Overlapping modules `{mod_name}` at paths:\n`{overlapping_paths}`"), + String::new(), + span, + ) + } DefCollectorErrorKind::PathResolutionError(error) => error.into(), DefCollectorErrorKind::NonStructTypeInImpl { span } => Diagnostic::simple_error( "Non-struct type used in impl".into(), diff --git a/test_programs/compile_failure/invalid_main_sub_lib/Nargo.toml b/test_programs/compile_failure/invalid_main_sub_lib/Nargo.toml new file mode 100644 index 00000000000..12776f47712 --- /dev/null +++ b/test_programs/compile_failure/invalid_main_sub_lib/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "invalid_main_sub_lib" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/invalid_main_sub_lib/Prover.toml b/test_programs/compile_failure/invalid_main_sub_lib/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_failure/invalid_main_sub_lib/src/main.nr b/test_programs/compile_failure/invalid_main_sub_lib/src/main.nr new file mode 100644 index 00000000000..4658900a47a --- /dev/null +++ b/test_programs/compile_failure/invalid_main_sub_lib/src/main.nr @@ -0,0 +1,5 @@ +mod lib; + +use crate::lib::foo; + +fn main() {} diff --git a/test_programs/compile_failure/invalid_main_sub_lib/src/main/lib.nr b/test_programs/compile_failure/invalid_main_sub_lib/src/main/lib.nr new file mode 100644 index 00000000000..0f89316b5bd --- /dev/null +++ b/test_programs/compile_failure/invalid_main_sub_lib/src/main/lib.nr @@ -0,0 +1,3 @@ +pub fn foo() -> bool { + true +} diff --git a/test_programs/compile_failure/invalid_mod_mod_path/Nargo.toml b/test_programs/compile_failure/invalid_mod_mod_path/Nargo.toml new file mode 100644 index 00000000000..7a93d385c1c --- /dev/null +++ b/test_programs/compile_failure/invalid_mod_mod_path/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "invalid_mod_mod_path" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/invalid_mod_mod_path/Prover.toml b/test_programs/compile_failure/invalid_mod_mod_path/Prover.toml new file mode 100644 index 00000000000..e0a68175d07 --- /dev/null +++ b/test_programs/compile_failure/invalid_mod_mod_path/Prover.toml @@ -0,0 +1,2 @@ +x = "" +y = "" diff --git a/test_programs/compile_failure/invalid_mod_mod_path/main/lib.nr b/test_programs/compile_failure/invalid_mod_mod_path/main/lib.nr new file mode 100644 index 00000000000..0f89316b5bd --- /dev/null +++ b/test_programs/compile_failure/invalid_mod_mod_path/main/lib.nr @@ -0,0 +1,3 @@ +pub fn foo() -> bool { + true +} diff --git a/test_programs/compile_failure/invalid_mod_mod_path/src/main.nr b/test_programs/compile_failure/invalid_mod_mod_path/src/main.nr new file mode 100644 index 00000000000..86fa197360f --- /dev/null +++ b/test_programs/compile_failure/invalid_mod_mod_path/src/main.nr @@ -0,0 +1,4 @@ +mod crate::mod; + +fn main() { +} diff --git a/test_programs/compile_failure/invalid_mod_mod_path/src/mod.nr b/test_programs/compile_failure/invalid_mod_mod_path/src/mod.nr new file mode 100644 index 00000000000..e2f24cb9226 --- /dev/null +++ b/test_programs/compile_failure/invalid_mod_mod_path/src/mod.nr @@ -0,0 +1,3 @@ +pub foo() -> bool { + true +} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml new file mode 100644 index 00000000000..18f046fceea --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "overlapping_dep_and_mod" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/Prover.toml b/test_programs/compile_failure/overlapping_dep_and_mod/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr b/test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr new file mode 100644 index 00000000000..0f89316b5bd --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr @@ -0,0 +1,3 @@ +pub fn foo() -> bool { + true +} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr b/test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr new file mode 100644 index 00000000000..0f89316b5bd --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr @@ -0,0 +1,3 @@ +pub fn foo() -> bool { + true +} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr b/test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr new file mode 100644 index 00000000000..2f5622981fb --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr @@ -0,0 +1,6 @@ +mod lib; + +use crate::lib::foo; + + +fn main() { } diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/Nargo.toml b/test_programs/compile_failure/overlapping_lib_and_mod/Nargo.toml new file mode 100644 index 00000000000..c9d59b22a39 --- /dev/null +++ b/test_programs/compile_failure/overlapping_lib_and_mod/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "overlapping_lib_and_mod" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/Prover.toml b/test_programs/compile_failure/overlapping_lib_and_mod/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/foo/mod.nr b/test_programs/compile_failure/overlapping_lib_and_mod/src/foo/mod.nr new file mode 100644 index 00000000000..261729b812b --- /dev/null +++ b/test_programs/compile_failure/overlapping_lib_and_mod/src/foo/mod.nr @@ -0,0 +1,3 @@ +pub fn bar() -> bool { + true +} diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/main.nr b/test_programs/compile_failure/overlapping_lib_and_mod/src/main.nr new file mode 100644 index 00000000000..12a3d91f941 --- /dev/null +++ b/test_programs/compile_failure/overlapping_lib_and_mod/src/main.nr @@ -0,0 +1,7 @@ +mod foo; + +use foo::bar; + +fn main() { + assert(bar()); +} diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/main/foo.nr b/test_programs/compile_failure/overlapping_lib_and_mod/src/main/foo.nr new file mode 100644 index 00000000000..f17d7d7cec5 --- /dev/null +++ b/test_programs/compile_failure/overlapping_lib_and_mod/src/main/foo.nr @@ -0,0 +1,4 @@ +pub fn bar() -> bool { + true +} + diff --git a/test_programs/compile_success_empty/vectors/Prover.toml b/test_programs/compile_success_empty/vectors/Prover.toml index f28f2f8cc48..e0a68175d07 100644 --- a/test_programs/compile_success_empty/vectors/Prover.toml +++ b/test_programs/compile_success_empty/vectors/Prover.toml @@ -1,2 +1,2 @@ -x = "5" -y = "10" +x = "" +y = "" diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 0bb967e7502..bddc2e94354 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -30,7 +30,7 @@ fn run_stdlib_tests() { let (mut context, dummy_crate_id) = prepare_package(&file_manager, &parsed_files, &dummy_package); - let result = check_crate(&mut context, dummy_crate_id, true, false, false); + let result = check_crate(&mut context, dummy_crate_id, true, false, false, false); report_errors(result, &context.file_manager, true, false) .expect("Error encountered while compiling standard library"); From 239ecc1c127199439f6c57f2986dfe7fc561604c Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Thu, 27 Jun 2024 10:55:10 -0400 Subject: [PATCH 14/41] Update compiler/fm/src/file_map.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- compiler/fm/src/file_map.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/fm/src/file_map.rs b/compiler/fm/src/file_map.rs index 01ed30f7465..e9acea8473b 100644 --- a/compiler/fm/src/file_map.rs +++ b/compiler/fm/src/file_map.rs @@ -33,8 +33,6 @@ impl From<&PathBuf> for PathString { #[derive(Debug, Clone)] pub struct FileMap { files: SimpleFiles, - // TODO remove before PR - pub name_to_id: HashMap, } // XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId From 71ed978cb3d26ed29e1ce29655e9beef299f4627 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 07:14:06 -0300 Subject: [PATCH 15/41] Make things compile again --- compiler/fm/src/file_map.rs | 1 + compiler/noirc_driver/tests/stdlib_warnings.rs | 2 +- .../src/hir/def_collector/dc_crate.rs | 13 +++---------- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 12 ++++++++---- .../noirc_frontend/src/hir/resolution/import.rs | 6 ------ tooling/lsp/src/lib.rs | 3 +-- tooling/lsp/src/notifications/mod.rs | 11 +++++------ tooling/lsp/src/requests/code_lens_request.rs | 2 +- tooling/lsp/src/requests/goto_declaration.rs | 2 +- tooling/lsp/src/requests/goto_definition.rs | 2 +- tooling/lsp/src/requests/test_run.rs | 2 +- tooling/lsp/src/requests/tests.rs | 2 +- 12 files changed, 24 insertions(+), 34 deletions(-) diff --git a/compiler/fm/src/file_map.rs b/compiler/fm/src/file_map.rs index e9acea8473b..fc6820fb000 100644 --- a/compiler/fm/src/file_map.rs +++ b/compiler/fm/src/file_map.rs @@ -33,6 +33,7 @@ impl From<&PathBuf> for PathString { #[derive(Debug, Clone)] pub struct FileMap { files: SimpleFiles, + pub name_to_id: HashMap, } // XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId diff --git a/compiler/noirc_driver/tests/stdlib_warnings.rs b/compiler/noirc_driver/tests/stdlib_warnings.rs index 68a5e141e9c..9b2aeaecd94 100644 --- a/compiler/noirc_driver/tests/stdlib_warnings.rs +++ b/compiler/noirc_driver/tests/stdlib_warnings.rs @@ -26,7 +26,7 @@ fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings> let root_crate_id = prepare_crate(&mut context, file_name); let ((), warnings) = - noirc_driver::check_crate(&mut context, root_crate_id, false, false, false, false)?; + noirc_driver::check_crate(&mut context, root_crate_id, false, false, false)?; assert_eq!(warnings, Vec::new(), "stdlib is producing {} warnings", warnings.len()); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index da3cc026ec6..37ece01c805 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -320,16 +320,9 @@ impl DefCollector { // Add the current crate to the collection of DefMaps context.def_maps.insert(crate_id, def_collector.def_map); - if !skip_prelude { - inject_prelude(crate_id, context, crate_root, &mut def_collector.imports); - for submodule in submodules { - inject_prelude( - crate_id, - context, - LocalModuleId(submodule), - &mut def_collector.imports, - ); - } + inject_prelude(crate_id, context, crate_root, &mut def_collector.imports); + for submodule in submodules { + inject_prelude(crate_id, context, LocalModuleId(submodule), &mut def_collector.imports); } // Resolve unresolved imports collected from the crate, one by one. diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 2eee411c102..4b564c6aad0 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -820,7 +820,8 @@ pub(crate) fn collect_global( mod tests { use super::*; - use std::path::PathBuf; + use noirc_errors::Spanned; + use std::path::{Path, PathBuf}; use tempfile::{tempdir, TempDir}; // Returns the absolute path to the file @@ -842,8 +843,9 @@ mod tests { let file_id = fm.add_file_with_source(entry_file_name, "fn foo() {}".to_string()).unwrap(); let dep_file_name = Path::new("foo.nr"); + let mod_name = Ident(Spanned::from_position(0, 1, "foo".to_string())); create_dummy_file(&dir, dep_file_name); - find_module(&fm, file_id, "foo").unwrap_err(); + find_module(&fm, file_id, mod_name).unwrap_err(); } #[test] @@ -881,9 +883,11 @@ mod tests { fm.add_file_with_source(sub_dir_nr_path.as_path(), "fn foo() {}".to_string()); // First check for the sub_dir.nr file and add it to the FileManager - let sub_dir_file_id = find_module(&fm, file_id, sub_dir_name).unwrap(); + let sub_dir_mod_name = Ident(Spanned::from_position(0, 1, sub_dir_name.to_string())); + let sub_dir_file_id = find_module(&fm, file_id, sub_dir_mod_name).unwrap(); // Now check for files in it's subdirectory - find_module(&fm, sub_dir_file_id, "foo").unwrap(); + let mod_name = Ident(Spanned::from_position(0, 1, "foo".to_string())); + find_module(&fm, sub_dir_file_id, mod_name).unwrap(); } } diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 352c120a290..acc85dc44ed 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -95,9 +95,6 @@ pub fn resolve_import( mut error, } = resolve_path_to_ns(import_directive, crate_id, crate_id, def_maps)?; - // TODO cleanup - // dbg!("resolve_import: err", &error); - let name = resolve_path_name(import_directive); let visibility = resolved_namespace @@ -138,9 +135,6 @@ fn resolve_path_to_ns( let import_path = &import_directive.path.segments; let def_map = &def_maps[&crate_id]; - // TODO cleanup - // dbg!("resolve_path_to_ns", &import_path); - match import_directive.path.kind { crate::ast::PathKind::Crate => { // Resolve from the root of the crate diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 233c2712f1d..304a2d34e47 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -351,8 +351,7 @@ fn prepare_package_from_source_string() { let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver); let (mut context, crate_id) = crate::prepare_source(source.to_string(), &mut state); - let _check_result = - noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); + let _check_result = noirc_driver::check_crate(&mut context, crate_id, false, false, false); let main_func_id = context.get_main_function(&crate_id); assert!(main_func_id.is_some()); } diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 3278e5eaef9..3856bdc79e9 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -56,7 +56,7 @@ pub(super) fn on_did_change_text_document( state.input_files.insert(params.text_document.uri.to_string(), text.clone()); let (mut context, crate_id) = prepare_source(text, state); - let _ = check_crate(&mut context, crate_id, false, false, false, false); + let _ = check_crate(&mut context, crate_id, false, false, false); let workspace = match resolve_workspace_for_source_path( params.text_document.uri.to_file_path().unwrap().as_path(), @@ -139,11 +139,10 @@ fn process_noir_document( let (mut context, crate_id) = prepare_package(&workspace_file_manager, &parsed_files, package); - let file_diagnostics = - match check_crate(&mut context, crate_id, false, false, false, false) { - Ok(((), warnings)) => warnings, - Err(errors_and_warnings) => errors_and_warnings, - }; + let file_diagnostics = match check_crate(&mut context, crate_id, false, false, false) { + Ok(((), warnings)) => warnings, + Err(errors_and_warnings) => errors_and_warnings, + }; let package_root_dir: String = package.root_dir.as_os_str().to_string_lossy().into(); diff --git a/tooling/lsp/src/requests/code_lens_request.rs b/tooling/lsp/src/requests/code_lens_request.rs index d5da7cc412f..744bddedd9d 100644 --- a/tooling/lsp/src/requests/code_lens_request.rs +++ b/tooling/lsp/src/requests/code_lens_request.rs @@ -67,7 +67,7 @@ fn on_code_lens_request_inner( let (mut context, crate_id) = prepare_source(source_string, state); // We ignore the warnings and errors produced by compilation for producing code lenses // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false, false, false, false); + let _ = check_crate(&mut context, crate_id, false, false, false); let collected_lenses = collect_lenses_for_package(&context, crate_id, &workspace, package, None); diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 97c79cea428..4ffe6abf88a 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -45,7 +45,7 @@ fn on_goto_definition_inner( def_interner } else { // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); &context.def_interner }; diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 12bf90377f9..3a92e28cc11 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -53,7 +53,7 @@ fn on_goto_definition_inner( def_interner } else { // We ignore the warnings and errors produced by compilation while resolving the definition - let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false, false); + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); &context.def_interner }; diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 1a88b8c7c52..83b05ba06a2 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -60,7 +60,7 @@ fn on_test_run_request_inner( Some(package) => { let (mut context, crate_id) = prepare_package(&workspace_file_manager, &parsed_files, package); - if check_crate(&mut context, crate_id, false, false, false, false).is_err() { + if check_crate(&mut context, crate_id, false, false, false).is_err() { let result = NargoTestRunResult { id: params.id.clone(), result: "error".to_string(), diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index 28e4d8cf738..cdf4ad338c4 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -61,7 +61,7 @@ fn on_tests_request_inner( prepare_package(&workspace_file_manager, &parsed_files, package); // We ignore the warnings and errors produced by compilation for producing tests // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false, false, false, false); + let _ = check_crate(&mut context, crate_id, false, false, false); // We don't add test headings for a package if it contains no `#[test]` functions get_package_tests_in_crate(&context, &crate_id, &package.name) From 0f565f1f27a3e6aff74f1930973375099ef680cc Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 07:17:52 -0300 Subject: [PATCH 16/41] Remove the `skip_prelude` option --- compiler/noirc_driver/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index e42e5a5eb2c..4ba9b85f967 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -103,10 +103,6 @@ pub struct CompileOptions { #[arg(long, hide = true)] pub use_legacy: bool, - /// Skip loading the prelude (stdlib) - #[arg(long, hide = true)] - pub skip_prelude: bool, - /// Outputs the paths to any modified artifacts #[arg(long, hide = true)] pub show_artifact_paths: bool, From d99d7f150880f06aec19406ba618c9f8eef5db23 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 07:18:45 -0300 Subject: [PATCH 17/41] No need for name_to_id to be pub --- compiler/fm/src/file_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/fm/src/file_map.rs b/compiler/fm/src/file_map.rs index fc6820fb000..50412d352ec 100644 --- a/compiler/fm/src/file_map.rs +++ b/compiler/fm/src/file_map.rs @@ -33,7 +33,7 @@ impl From<&PathBuf> for PathString { #[derive(Debug, Clone)] pub struct FileMap { files: SimpleFiles, - pub name_to_id: HashMap, + name_to_id: HashMap, } // XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId From ff6f7e0fd0074840c29e9954b81fd0a2542b2f52 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 07:20:15 -0300 Subject: [PATCH 18/41] Pass reference to Ident, only clone when needed --- .../src/hir/def_collector/dc_mod.rs | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 4b564c6aad0..162f4f03a37 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -557,14 +557,14 @@ impl<'a> ModCollector<'a> { macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; - let child_file_id = - match find_module(&context.file_manager, self.file_id, mod_decl.ident.clone()) { - Ok(child_file_id) => child_file_id, - Err(err) => { - errors.push((err.into(), self.file_id)); - return errors; - } - }; + let child_file_id = match find_module(&context.file_manager, self.file_id, &mod_decl.ident) + { + Ok(child_file_id) => child_file_id, + Err(err) => { + errors.push((err.into(), self.file_id)); + return errors; + } + }; let location = Location { file: self.file_id, span: mod_decl.ident.span() }; @@ -681,7 +681,7 @@ impl<'a> ModCollector<'a> { fn find_module( file_manager: &FileManager, anchor: FileId, - mod_name: Ident, + mod_name: &Ident, ) -> Result { let anchor_path = file_manager .path(anchor) @@ -721,7 +721,10 @@ fn find_module( match found_paths.len() { 0 => { let expected_path = parent_candidate.as_os_str().to_string_lossy().to_string(); - Err(DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path }) + Err(DefCollectorErrorKind::UnresolvedModuleDecl { + mod_name: mod_name.clone(), + expected_path, + }) } 1 => Ok(found_paths[0]), _ => { @@ -736,7 +739,10 @@ fn find_module( .to_string() }) .collect(); - Err(DefCollectorErrorKind::OverlappingModuleDecls { mod_name, overlapping_paths }) + Err(DefCollectorErrorKind::OverlappingModuleDecls { + mod_name: mod_name.clone(), + overlapping_paths, + }) } } } @@ -845,7 +851,7 @@ mod tests { let dep_file_name = Path::new("foo.nr"); let mod_name = Ident(Spanned::from_position(0, 1, "foo".to_string())); create_dummy_file(&dir, dep_file_name); - find_module(&fm, file_id, mod_name).unwrap_err(); + find_module(&fm, file_id, &mod_name).unwrap_err(); } #[test] @@ -884,10 +890,10 @@ mod tests { // First check for the sub_dir.nr file and add it to the FileManager let sub_dir_mod_name = Ident(Spanned::from_position(0, 1, sub_dir_name.to_string())); - let sub_dir_file_id = find_module(&fm, file_id, sub_dir_mod_name).unwrap(); + let sub_dir_file_id = find_module(&fm, file_id, &sub_dir_mod_name).unwrap(); // Now check for files in it's subdirectory let mod_name = Ident(Spanned::from_position(0, 1, "foo".to_string())); - find_module(&fm, sub_dir_file_id, mod_name).unwrap(); + find_module(&fm, sub_dir_file_id, &mod_name).unwrap(); } } From 5559bc9d050b2043536b5c7b136e4f4cc0dfb6f5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 10:23:07 -0300 Subject: [PATCH 19/41] No need to create files when dealing with FileManager --- Cargo.lock | 2 - compiler/fm/Cargo.toml | 1 - compiler/fm/src/lib.rs | 42 ++++------ compiler/noirc_frontend/Cargo.toml | 1 - .../src/hir/def_collector/dc_mod.rs | 84 +++++++++---------- 5 files changed, 56 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d2afe025c69..37376ad7c80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1542,7 +1542,6 @@ dependencies = [ "codespan-reporting", "iter-extended", "serde", - "tempfile", ] [[package]] @@ -2928,7 +2927,6 @@ dependencies = [ "smol_str", "strum", "strum_macros", - "tempfile", "thiserror", "tracing", ] diff --git a/compiler/fm/Cargo.toml b/compiler/fm/Cargo.toml index f556f305c78..1a356d93d89 100644 --- a/compiler/fm/Cargo.toml +++ b/compiler/fm/Cargo.toml @@ -13,5 +13,4 @@ codespan-reporting.workspace = true serde.workspace = true [dev-dependencies] -tempfile.workspace = true iter-extended.workspace = true diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index bb080c62c0e..c4566f4ade7 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -185,24 +185,17 @@ mod path_normalization { #[cfg(test)] mod tests { use super::*; - use tempfile::{tempdir, TempDir}; - // Returns the absolute path to the file - fn create_dummy_file(dir: &TempDir, file_name: &Path) -> PathBuf { - let file_path = dir.path().join(file_name); - let _file = std::fs::File::create(&file_path).unwrap(); - file_path + fn add_file(fm: &mut FileManager, file_name: &Path) -> FileId { + fm.add_file_with_source(file_name, "fn foo() {}".to_string()).unwrap() } #[test] fn path_resolve_file_module_other_ext() { - let dir = tempdir().unwrap(); - let file_name = Path::new("foo.nr"); - create_dummy_file(&dir, file_name); + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); - let mut fm = FileManager::new(dir.path()); - - let file_id = fm.add_file_with_source(file_name, "fn foo() {}".to_string()).unwrap(); + let file_id = add_file(&mut fm, Path::new("foo.nr")); assert!(fm.path(file_id).unwrap().ends_with("foo.nr")); } @@ -213,23 +206,20 @@ mod tests { /// they should both resolve to ../foo.nr #[test] fn path_resolve_modules_with_different_paths_as_same_file() { - let dir = tempdir().unwrap(); - let sub_dir = TempDir::new_in(&dir).unwrap(); - let sub_sub_dir = TempDir::new_in(&sub_dir).unwrap(); - - let mut fm = FileManager::new(dir.path()); + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); - // Create a lib.nr file at the root. - let file_name = Path::new("lib.nr"); - create_dummy_file(&dir, file_name); + let sub_dir = dir.join("sub_dir"); + let sub_sub_dir = sub_dir.join("sub_sub_dir"); - // Create another path with `./` and `../` inside it - let second_file_name = PathBuf::from(sub_sub_dir.path()).join("./../../lib.nr"); + // Create a lib.nr file at the root and add it to the file manager. + let file_id = add_file(&mut fm, Path::new("lib.nr")); - // Add both files to the file manager - let file_id = fm.add_file_with_source(file_name, "fn foo() {}".to_string()).unwrap(); - let second_file_id = - fm.add_file_with_source(&second_file_name, "fn foo() {}".to_string()).unwrap(); + // Create another path with `./` and `../` inside it, and add it to the file manager + let second_file_id = add_file( + &mut fm, + PathBuf::from(sub_sub_dir.as_path()).join("./../../lib.nr").as_path(), + ); assert_eq!(file_id, second_file_id); } diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 381eb4a9bb9..cc5a9b1e652 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -36,7 +36,6 @@ lalrpop-util = { version = "0.20.2", features = ["lexer"] } base64.workspace = true strum = "0.24" strum_macros = "0.24" -tempfile.workspace = true [build-dependencies] lalrpop = "0.20.2" diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 162f4f03a37..89e46845539 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -828,72 +828,68 @@ mod tests { use noirc_errors::Spanned; use std::path::{Path, PathBuf}; - use tempfile::{tempdir, TempDir}; - // Returns the absolute path to the file - fn create_dummy_file(dir: &TempDir, file_name: &Path) -> PathBuf { - let file_path = dir.path().join(file_name); - let _file = std::fs::File::create(&file_path).unwrap(); - file_path + fn add_file(file_manager: &mut FileManager, dir: &Path, file_name: &str) -> FileId { + file_manager + .add_file_with_source(&dir.join(file_name), "fn foo() {}".to_string()) + .expect("could not add file to file manager and obtain a FileId") } - #[test] - fn path_resolve_file_module() { - let dir = tempdir().unwrap(); - - let entry_file_name = Path::new("my_dummy_file.nr"); - create_dummy_file(&dir, entry_file_name); + fn find_module( + file_manager: &FileManager, + anchor: FileId, + mod_name: &str, + ) -> Result { + let mod_name = Ident(Spanned::from_position(0, 1, mod_name.to_string())); + super::find_module(file_manager, anchor, &mod_name) + } - let mut fm = FileManager::new(dir.path()); + #[test] + fn find_module_cannot_find_non_existing_file() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); - let file_id = fm.add_file_with_source(entry_file_name, "fn foo() {}".to_string()).unwrap(); + // Create a my_dummy_file.nr + // Now we have temp_dir/my_dummy_file.nr + let file_id = add_file(&mut fm, &dir, "my_dummy_file.nr"); - let dep_file_name = Path::new("foo.nr"); - let mod_name = Ident(Spanned::from_position(0, 1, "foo".to_string())); - create_dummy_file(&dir, dep_file_name); - find_module(&fm, file_id, &mod_name).unwrap_err(); + find_module(&fm, file_id, "foo").unwrap_err(); } #[test] - fn path_resolve_sub_module() { - let dir = tempdir().unwrap(); - let mut fm = FileManager::new(dir.path()); + fn find_module_sub_module() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); // Create a lib.nr file at the root. - // we now have dir/lib.nr - let lib_nr_path = create_dummy_file(&dir, Path::new("lib.nr")); - let file_id = fm - .add_file_with_source(lib_nr_path.as_path(), "fn foo() {}".to_string()) - .expect("could not add file to file manager and obtain a FileId"); + // we now have temp_dir/lib.nr + let file_id = add_file(&mut fm, &dir, "lib.nr"); // Create a sub directory // we now have: - // - dir/lib.nr - // - dir/sub_dir - let sub_dir = TempDir::new_in(&dir).unwrap(); - let sub_dir_name = sub_dir.path().file_name().unwrap().to_str().unwrap(); + // - temp_dir/lib.nr + // - temp_dir/sub_dir + let sub_dir_name = "sub_dir"; + let sub_dir_path = dir.join(sub_dir_name); + std::fs::create_dir(&sub_dir_path).unwrap(); // Add foo.nr to the subdirectory // we no have: - // - dir/lib.nr - // - dir/sub_dir/foo.nr - let foo_nr_path = create_dummy_file(&sub_dir, Path::new("foo.nr")); - fm.add_file_with_source(foo_nr_path.as_path(), "fn foo() {}".to_string()); + // - temp_dir/lib.nr + // - temp_dir/sub_dir/foo.nr + add_file(&mut fm, &sub_dir_path, "foo.nr"); // Add a parent module for the sub_dir // we no have: - // - dir/lib.nr - // - dir/sub_dir.nr - // - dir/sub_dir/foo.nr - let sub_dir_nr_path = create_dummy_file(&dir, Path::new(&format!("{sub_dir_name}.nr"))); - fm.add_file_with_source(sub_dir_nr_path.as_path(), "fn foo() {}".to_string()); + // - temp_dir/lib.nr + // - temp_dir/sub_dir.nr + // - temp_dir/sub_dir/foo.nr + add_file(&mut fm, &dir, &format!("{sub_dir_name}.nr")); - // First check for the sub_dir.nr file and add it to the FileManager - let sub_dir_mod_name = Ident(Spanned::from_position(0, 1, sub_dir_name.to_string())); - let sub_dir_file_id = find_module(&fm, file_id, &sub_dir_mod_name).unwrap(); + // First check for the sub_dir.nr file + let sub_dir_file_id = find_module(&fm, file_id, sub_dir_name).unwrap(); // Now check for files in it's subdirectory - let mod_name = Ident(Spanned::from_position(0, 1, "foo".to_string())); - find_module(&fm, sub_dir_file_id, &mod_name).unwrap(); + find_module(&fm, sub_dir_file_id, "foo").unwrap(); } } From e13f443c18b6a8c3d0dbccd6b8b9234c528148c6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 10:35:56 -0300 Subject: [PATCH 20/41] Simplify tests a bit --- compiler/fm/src/lib.rs | 9 +++--- .../src/hir/def_collector/dc_mod.rs | 28 +++++++------------ 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index c4566f4ade7..2e52d802479 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -195,7 +195,7 @@ mod tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let file_id = add_file(&mut fm, Path::new("foo.nr")); + let file_id = add_file(&mut fm, &dir.join("foo.nr")); assert!(fm.path(file_id).unwrap().ends_with("foo.nr")); } @@ -209,13 +209,12 @@ mod tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let sub_dir = dir.join("sub_dir"); - let sub_sub_dir = sub_dir.join("sub_sub_dir"); - // Create a lib.nr file at the root and add it to the file manager. - let file_id = add_file(&mut fm, Path::new("lib.nr")); + let file_id = add_file(&mut fm, &dir.join("lib.nr")); // Create another path with `./` and `../` inside it, and add it to the file manager + let sub_dir = dir.join("sub_dir"); + let sub_sub_dir = sub_dir.join("sub_sub_dir"); let second_file_id = add_file( &mut fm, PathBuf::from(sub_sub_dir.as_path()).join("./../../lib.nr").as_path(), diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 89e46845539..b3e9b4ca29d 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -829,9 +829,9 @@ mod tests { use noirc_errors::Spanned; use std::path::{Path, PathBuf}; - fn add_file(file_manager: &mut FileManager, dir: &Path, file_name: &str) -> FileId { + fn add_file(file_manager: &mut FileManager, file_name: &Path) -> FileId { file_manager - .add_file_with_source(&dir.join(file_name), "fn foo() {}".to_string()) + .add_file_with_source(&file_name, "fn foo() {}".to_string()) .expect("could not add file to file manager and obtain a FileId") } @@ -851,7 +851,7 @@ mod tests { // Create a my_dummy_file.nr // Now we have temp_dir/my_dummy_file.nr - let file_id = add_file(&mut fm, &dir, "my_dummy_file.nr"); + let file_id = add_file(&mut fm, &dir.join("my_dummy_file.nr")); find_module(&fm, file_id, "foo").unwrap_err(); } @@ -863,31 +863,23 @@ mod tests { // Create a lib.nr file at the root. // we now have temp_dir/lib.nr - let file_id = add_file(&mut fm, &dir, "lib.nr"); + let file_id = add_file(&mut fm, &dir.join("lib.nr")); - // Create a sub directory - // we now have: - // - temp_dir/lib.nr - // - temp_dir/sub_dir - let sub_dir_name = "sub_dir"; - let sub_dir_path = dir.join(sub_dir_name); - std::fs::create_dir(&sub_dir_path).unwrap(); - - // Add foo.nr to the subdirectory + // Add a parent module for the sub_dir // we no have: // - temp_dir/lib.nr - // - temp_dir/sub_dir/foo.nr - add_file(&mut fm, &sub_dir_path, "foo.nr"); + // - temp_dir/sub_dir.nr + add_file(&mut fm, &dir.join(&format!("sub_dir.nr"))); - // Add a parent module for the sub_dir + // Add foo.nr to the subdirectory // we no have: // - temp_dir/lib.nr // - temp_dir/sub_dir.nr // - temp_dir/sub_dir/foo.nr - add_file(&mut fm, &dir, &format!("{sub_dir_name}.nr")); + add_file(&mut fm, &dir.join("sub_dir").join("foo.nr")); // First check for the sub_dir.nr file - let sub_dir_file_id = find_module(&fm, file_id, sub_dir_name).unwrap(); + let sub_dir_file_id = find_module(&fm, file_id, "sub_dir").unwrap(); // Now check for files in it's subdirectory find_module(&fm, sub_dir_file_id, "foo").unwrap(); From 70e880fea4817056a3b199d3b84f6ca93e781c5b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 11:03:00 -0300 Subject: [PATCH 21/41] Add a few more tests, and fix the overlapping_lib_and_mod program that fails to compile --- .../src/hir/def_collector/dc_mod.rs | 63 +++++++++++++------ .../src/{main => }/foo.nr | 0 2 files changed, 44 insertions(+), 19 deletions(-) rename test_programs/compile_failure/overlapping_lib_and_mod/src/{main => }/foo.nr (100%) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index b3e9b4ca29d..f4497061c35 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -845,7 +845,7 @@ mod tests { } #[test] - fn find_module_cannot_find_non_existing_file() { + fn find_module_errors_if_cannot_find_file() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -853,35 +853,60 @@ mod tests { // Now we have temp_dir/my_dummy_file.nr let file_id = add_file(&mut fm, &dir.join("my_dummy_file.nr")); - find_module(&fm, file_id, "foo").unwrap_err(); + let result = find_module(&fm, file_id, "foo"); + assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); } #[test] - fn find_module_sub_module() { + fn find_module_can_find_nested_modules() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create a lib.nr file at the root. - // we now have temp_dir/lib.nr - let file_id = add_file(&mut fm, &dir.join("lib.nr")); - - // Add a parent module for the sub_dir - // we no have: - // - temp_dir/lib.nr - // - temp_dir/sub_dir.nr - add_file(&mut fm, &dir.join(&format!("sub_dir.nr"))); - - // Add foo.nr to the subdirectory - // we no have: - // - temp_dir/lib.nr - // - temp_dir/sub_dir.nr - // - temp_dir/sub_dir/foo.nr + // Create this tree structure: + // - lib.nr + // - sub_dir.nr + // - sub_dir/foo.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("sub_dir.nr")); add_file(&mut fm, &dir.join("sub_dir").join("foo.nr")); // First check for the sub_dir.nr file - let sub_dir_file_id = find_module(&fm, file_id, "sub_dir").unwrap(); + let sub_dir_file_id = find_module(&fm, lib_file_id, "sub_dir").unwrap(); // Now check for files in it's subdirectory find_module(&fm, sub_dir_file_id, "foo").unwrap(); } + + #[test] + fn find_module_can_find_as_mod_dot_nr() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - lib.nr + // - foo/mod.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("foo").join("mod.nr")); + + // Check that searching "foo" finds the mod.nr file + find_module(&fm, lib_file_id, "foo").unwrap(); + } + + #[test] + fn find_module_errors_if_there_are_overlapping_module_declarations() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - lib.nr + // - foo.nr + // - foo/mod.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("foo.nr")); + add_file(&mut fm, &dir.join("foo").join("mod.nr")); + + // Check that searching "foo" gives an error + let result = find_module(&fm, lib_file_id, "foo"); + assert!(matches!(result, Err(DefCollectorErrorKind::OverlappingModuleDecls { .. }))); + } } diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/main/foo.nr b/test_programs/compile_failure/overlapping_lib_and_mod/src/foo.nr similarity index 100% rename from test_programs/compile_failure/overlapping_lib_and_mod/src/main/foo.nr rename to test_programs/compile_failure/overlapping_lib_and_mod/src/foo.nr From 80291e2876fb21f303e95e4288098bf601638288 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 11:17:45 -0300 Subject: [PATCH 22/41] Remove maybe wrong compile failure case --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 6 +++--- .../overlapping_dep_and_mod/Nargo.toml | 6 ------ .../overlapping_dep_and_mod/bin/Nargo.toml | 8 -------- .../overlapping_dep_and_mod/bin/Prover.toml | 0 .../overlapping_dep_and_mod/bin/src/main.nr | 12 ------------ .../overlapping_dep_and_mod/foo/Nargo.toml | 7 ------- .../overlapping_dep_and_mod/foo/src/lib.nr | 3 --- .../overlapping_dep_and_mod/src/lib.nr | 3 --- .../overlapping_dep_and_mod/src/lib/mod.nr | 3 --- .../overlapping_dep_and_mod/src/main.nr | 6 ------ .../overlapping_lib_and_mod/Prover.toml | 0 .../Nargo.toml | 0 .../Prover.toml | 0 .../src/foo.nr | 0 .../src/foo/mod.nr | 0 .../src/main.nr | 0 16 files changed, 3 insertions(+), 51 deletions(-) delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/bin/Prover.toml delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr delete mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr delete mode 100644 test_programs/compile_failure/overlapping_lib_and_mod/Prover.toml rename test_programs/compile_failure/{overlapping_lib_and_mod => overlapping_mod}/Nargo.toml (100%) rename test_programs/compile_failure/{overlapping_dep_and_mod => overlapping_mod}/Prover.toml (100%) rename test_programs/compile_failure/{overlapping_lib_and_mod => overlapping_mod}/src/foo.nr (100%) rename test_programs/compile_failure/{overlapping_lib_and_mod => overlapping_mod}/src/foo/mod.nr (100%) rename test_programs/compile_failure/{overlapping_lib_and_mod => overlapping_mod}/src/main.nr (100%) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index f4497061c35..0f46916fd2f 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -691,7 +691,7 @@ fn find_module( // if `anchor` is a `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, we check siblings of // the anchor at `base/mod_name.nr`. - let mod_name_str = mod_name.0.contents.clone(); + let mod_name_str = &mod_name.0.contents; let mod_nr_candidate = anchor_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")); let parent_candidate = anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")); let child_candidate = anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")); @@ -870,10 +870,10 @@ mod tests { add_file(&mut fm, &dir.join("sub_dir.nr")); add_file(&mut fm, &dir.join("sub_dir").join("foo.nr")); - // First check for the sub_dir.nr file + // `mod sub_dir` from `lib.nr` should find `sub_dir.nr` let sub_dir_file_id = find_module(&fm, lib_file_id, "sub_dir").unwrap(); - // Now check for files in it's subdirectory + // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` find_module(&fm, sub_dir_file_id, "foo").unwrap(); } diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml deleted file mode 100644 index 658ec7a2055..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml +++ /dev/null @@ -1,6 +0,0 @@ -[workspace] -members = [ - "bin", - "foo", -] -default-member = "bin" \ No newline at end of file diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml deleted file mode 100644 index 57e704462db..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "overlapping_dep_and_mod" -type = "bin" -authors = [""] -compiler_version = ">=0.29.0" - -[dependencies] -foo = { path = "../foo" } diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/bin/Prover.toml b/test_programs/compile_failure/overlapping_dep_and_mod/bin/Prover.toml deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr b/test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr deleted file mode 100644 index 675e889b7e5..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr +++ /dev/null @@ -1,12 +0,0 @@ -fn main() -> pub Field { - assert(foo::bar() + foo::baz() == 3); - assert(foo::bar() == 1); - assert(foo::baz() == 2); - foo::bar() + foo::baz() -} - -mod foo { - pub(crate) fn bar() -> Field { - 1 - } -} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml deleted file mode 100644 index 857d4e722a8..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "foo" -type = "lib" -authors = [""] -compiler_version = ">=0.29.0" - -[dependencies] diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr b/test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr deleted file mode 100644 index 7834e2c9276..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr +++ /dev/null @@ -1,3 +0,0 @@ -pub fn baz() -> Field { - 2 -} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr b/test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr deleted file mode 100644 index 0f89316b5bd..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/src/lib.nr +++ /dev/null @@ -1,3 +0,0 @@ -pub fn foo() -> bool { - true -} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr b/test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr deleted file mode 100644 index 0f89316b5bd..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/src/lib/mod.nr +++ /dev/null @@ -1,3 +0,0 @@ -pub fn foo() -> bool { - true -} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr b/test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr deleted file mode 100644 index 2f5622981fb..00000000000 --- a/test_programs/compile_failure/overlapping_dep_and_mod/src/main.nr +++ /dev/null @@ -1,6 +0,0 @@ -mod lib; - -use crate::lib::foo; - - -fn main() { } diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/Prover.toml b/test_programs/compile_failure/overlapping_lib_and_mod/Prover.toml deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/Nargo.toml b/test_programs/compile_failure/overlapping_mod/Nargo.toml similarity index 100% rename from test_programs/compile_failure/overlapping_lib_and_mod/Nargo.toml rename to test_programs/compile_failure/overlapping_mod/Nargo.toml diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/Prover.toml b/test_programs/compile_failure/overlapping_mod/Prover.toml similarity index 100% rename from test_programs/compile_failure/overlapping_dep_and_mod/Prover.toml rename to test_programs/compile_failure/overlapping_mod/Prover.toml diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/foo.nr b/test_programs/compile_failure/overlapping_mod/src/foo.nr similarity index 100% rename from test_programs/compile_failure/overlapping_lib_and_mod/src/foo.nr rename to test_programs/compile_failure/overlapping_mod/src/foo.nr diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/foo/mod.nr b/test_programs/compile_failure/overlapping_mod/src/foo/mod.nr similarity index 100% rename from test_programs/compile_failure/overlapping_lib_and_mod/src/foo/mod.nr rename to test_programs/compile_failure/overlapping_mod/src/foo/mod.nr diff --git a/test_programs/compile_failure/overlapping_lib_and_mod/src/main.nr b/test_programs/compile_failure/overlapping_mod/src/main.nr similarity index 100% rename from test_programs/compile_failure/overlapping_lib_and_mod/src/main.nr rename to test_programs/compile_failure/overlapping_mod/src/main.nr From e1601bb68169f0a5b6ca445aa8c5d3a94fe38f54 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 14:17:49 -0300 Subject: [PATCH 23/41] More tests and some code cleanup --- .../src/hir/def_collector/dc_mod.rs | 146 ++++++++++++++---- 1 file changed, 119 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 0f46916fd2f..ce83277b77b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,4 +1,5 @@ -use std::{collections::HashMap, ffi::OsStr, vec}; +use std::path::{Path, PathBuf}; +use std::{collections::HashMap, vec}; use acvm::{AcirField, FieldElement}; use fm::{FileId, FileManager, FILE_EXTENSION}; @@ -689,38 +690,32 @@ fn find_module( .with_extension(""); let anchor_dir = anchor_path.parent().unwrap(); - // if `anchor` is a `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, we check siblings of - // the anchor at `base/mod_name.nr`. + // Assuming anchor is called "anchor.nr" and we are looking up a module named "mod_name"... + // This is "mod_name" let mod_name_str = &mod_name.0.contents; + + // Check "mod_name/mod.nr" let mod_nr_candidate = anchor_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")); - let parent_candidate = anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")); - let child_candidate = anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")); - - let mod_nr_result = file_manager - .name_to_id(mod_nr_candidate.clone()) - .ok_or_else(|| mod_nr_candidate.as_os_str().to_string_lossy().to_string()); - let parent_result = file_manager - .name_to_id(parent_candidate.clone()) - .ok_or_else(|| parent_candidate.as_os_str().to_string_lossy().to_string()); - - let mut path_results = vec![mod_nr_result, parent_result]; - let anchor_path_suffix = anchor_path - .as_path() - .file_name() - .unwrap_or_else(|| OsStr::new("")) - .to_string_lossy() - .to_string(); - if anchor_path_suffix != "main" { - let child_result = file_manager - .name_to_id(child_candidate.clone()) - .ok_or_else(|| child_candidate.as_os_str().to_string_lossy().to_string()); + let mod_nr_result = find_in_name_to_id(&file_manager, &mod_nr_candidate); + + // Check "mod_name.nr" + let sibling_candidate = anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")); + let sibling_result = find_in_name_to_id(&file_manager, &sibling_candidate); + + let mut path_results = vec![mod_nr_result, sibling_result]; + + // We also check "anchor/mod_name.nr", but only if anchor isn't one of the special names: + // `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, + if !should_check_siblings_for_module(&anchor_path, anchor_dir) { + let child_candidate = anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")); + let child_result = find_in_name_to_id(&file_manager, &child_candidate); path_results.push(child_result); } let found_paths: Vec<_> = path_results.into_iter().flat_map(|result| result.ok()).collect(); match found_paths.len() { 0 => { - let expected_path = parent_candidate.as_os_str().to_string_lossy().to_string(); + let expected_path = sibling_candidate.as_os_str().to_string_lossy().to_string(); Err(DefCollectorErrorKind::UnresolvedModuleDecl { mod_name: mod_name.clone(), expected_path, @@ -747,6 +742,30 @@ fn find_module( } } +fn find_in_name_to_id(file_manager: &FileManager, path: &PathBuf) -> Result { + file_manager + .name_to_id(path.clone()) + .ok_or_else(|| path.as_os_str().to_string_lossy().to_string()) +} +/// Returns true if a module's child modules are expected to be in the same directory. +/// Returns false if they are expected to be in a subdirectory matching the name of the module. +fn should_check_siblings_for_module(module_path: &Path, parent_path: &Path) -> bool { + if let Some(filename) = module_path.file_stem() { + // This check also means a `main.nr` or `lib.nr` file outside of the crate root would + // check its same directory for child modules instead of a subdirectory. Should we prohibit + // `main.nr` and `lib.nr` files outside of the crate root? + filename == "main" + || filename == "lib" + || filename == "mod" + || Some(filename) == parent_path.file_stem() + } else { + // If there's no filename, we arbitrarily return true. + // Alternatively, we could panic, but this is left to a different step where we + // ideally have some source location to issue an error. + true + } +} + cfg_if::cfg_if! { if #[cfg(feature = "bls12_381")] { pub const CHOSEN_FIELD: &str = "bls12_381"; @@ -858,7 +877,57 @@ mod tests { } #[test] - fn find_module_can_find_nested_modules() { + fn find_module_errors_because_cannot_find_mod_relative_to_main() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - main.nr + // - main/foo.nr + let main_file_id = add_file(&mut fm, &dir.join("main.nr")); + add_file(&mut fm, &dir.join("main").join("foo.nr")); + + let result = find_module(&fm, main_file_id, "foo"); + assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); + } + + #[test] + fn find_module_errors_because_cannot_find_mod_relative_to_lib() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - lib.nr + // - lib/foo.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("lib").join("foo.nr")); + + let result = find_module(&fm, lib_file_id, "foo"); + assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); + } + + #[test] + fn find_module_can_find_module_in_the_same_directory() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - lib.nr + // - bar.nr + // - foo.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("bar.nr")); + add_file(&mut fm, &dir.join("foo.nr")); + + // `mod bar` from `lib.nr` should find `bar.nr` + let bar_file_id = find_module(&fm, lib_file_id, "bar").unwrap(); + + // `mod foo` from `bar.nr` should find `foo.nr` + find_module(&fm, bar_file_id, "foo").unwrap(); + } + + #[test] + fn find_module_can_find_module_nested_in_directory() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -893,7 +962,7 @@ mod tests { } #[test] - fn find_module_errors_if_there_are_overlapping_module_declarations() { + fn find_module_errors_if_module_is_found_in_name_dot_nr_and_name_slash_mod_dot_nr() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -909,4 +978,27 @@ mod tests { let result = find_module(&fm, lib_file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::OverlappingModuleDecls { .. }))); } + + #[test] + fn find_module_errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - lib.nr + // - foo.nr + // - bar.nr + // - foo/bar.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("foo.nr")); + add_file(&mut fm, &dir.join("bar.nr")); + add_file(&mut fm, &dir.join("foo").join("bar.nr")); + + // Find `mod foo` from `lib` + let foo_file_id = find_module(&fm, lib_file_id, "foo").unwrap(); + + // Check that `mod bar` from `foo` gives an error + let result = find_module(&fm, foo_file_id, "bar"); + assert!(matches!(result, Err(DefCollectorErrorKind::OverlappingModuleDecls { .. }))); + } } From 032e9732f516f617df3e43b3dbb83d281ec1ceda Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 14:24:39 -0300 Subject: [PATCH 24/41] Undo unrelated changes --- compiler/noirc_frontend/src/hir/resolution/import.rs | 1 - test_programs/compile_success_empty/vectors/Prover.toml | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index acc85dc44ed..282ee8a23c2 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -214,7 +214,6 @@ fn resolve_name_in_module( } let first_segment = import_path.first().expect("ice: could not fetch first segment"); - let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); diff --git a/test_programs/compile_success_empty/vectors/Prover.toml b/test_programs/compile_success_empty/vectors/Prover.toml index e0a68175d07..f28f2f8cc48 100644 --- a/test_programs/compile_success_empty/vectors/Prover.toml +++ b/test_programs/compile_success_empty/vectors/Prover.toml @@ -1,2 +1,2 @@ -x = "" -y = "" +x = "5" +y = "10" From 8e86139d79f5b3bef4f43157d7d6a84ae4e672a8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 15:18:41 -0300 Subject: [PATCH 25/41] Get the logic right --- .../src/hir/def_collector/dc_mod.rs | 176 +++++++++++------- .../src/hir/def_collector/errors.rs | 13 +- 2 files changed, 110 insertions(+), 79 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index ce83277b77b..dc2ef1b54f6 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::path::Path; use std::{collections::HashMap, vec}; use acvm::{AcirField, FieldElement}; @@ -694,59 +694,48 @@ fn find_module( // This is "mod_name" let mod_name_str = &mod_name.0.contents; - // Check "mod_name/mod.nr" - let mod_nr_candidate = anchor_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")); - let mod_nr_result = find_in_name_to_id(&file_manager, &mod_nr_candidate); - - // Check "mod_name.nr" - let sibling_candidate = anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")); - let sibling_result = find_in_name_to_id(&file_manager, &sibling_candidate); - - let mut path_results = vec![mod_nr_result, sibling_result]; + // The logic of finding a module depends on whether the module we are in (anchor) is + // one of the special "main.nr", "lib.nr", "mod.nr" or "{mod_name}.nr" names. + let (mod_name_candidate, mod_nr_candidate) = + if should_check_siblings_for_module(&anchor_path, anchor_dir) { + // We are in a "main.nr" (or similar) file. + // A "mod mod_name;" statement should look up in "mod_name.nr" or "mod_name/mod.nr". + ( + // Check "mod_name.nr" + anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")), + // Check "mod_name/mod.nr" + anchor_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")), + ) + } else { + // We are not in a "main.nr" (or similar file), so we are in, say, "foo.nr". + // A "mod mod_name;" statement should look up in "foo/mod_name.nr" or "foo/mod_name/mod.nr"; + ( + // Check "foo/mod_name.nr" + anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")), + // Check "foo/mod_name/mod.nr" + anchor_path.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")), + ) + }; - // We also check "anchor/mod_name.nr", but only if anchor isn't one of the special names: - // `main.nr`, `lib.nr`, `mod.nr` or `{mod_name}.nr`, - if !should_check_siblings_for_module(&anchor_path, anchor_dir) { - let child_candidate = anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")); - let child_result = find_in_name_to_id(&file_manager, &child_candidate); - path_results.push(child_result); - } - - let found_paths: Vec<_> = path_results.into_iter().flat_map(|result| result.ok()).collect(); - match found_paths.len() { - 0 => { - let expected_path = sibling_candidate.as_os_str().to_string_lossy().to_string(); - Err(DefCollectorErrorKind::UnresolvedModuleDecl { - mod_name: mod_name.clone(), - expected_path, - }) - } - 1 => Ok(found_paths[0]), - _ => { - let overlapping_paths: Vec<_> = found_paths - .into_iter() - .map(|found_path| { - file_manager - .path(found_path) - .expect("all modules to be in the file manager") - .as_os_str() - .to_string_lossy() - .to_string() - }) - .collect(); - Err(DefCollectorErrorKind::OverlappingModuleDecls { - mod_name: mod_name.clone(), - overlapping_paths, - }) - } + let mod_name_result = file_manager.name_to_id(mod_name_candidate.clone()); + let mod_nr_result = file_manager.name_to_id(mod_nr_candidate.clone()); + + match (mod_nr_result, mod_name_result) { + (Some(_), Some(_)) => Err(DefCollectorErrorKind::OverlappingModuleDecls { + mod_name: mod_name.clone(), + expected_path: mod_name_candidate.as_os_str().to_string_lossy().to_string(), + alternative_path: mod_nr_candidate.as_os_str().to_string_lossy().to_string(), + }), + (Some(mod_nr_id), None) => Ok(mod_nr_id), + (None, Some(name_id)) => Ok(name_id), + (None, None) => Err(DefCollectorErrorKind::UnresolvedModuleDecl { + mod_name: mod_name.clone(), + expected_path: mod_name_candidate.as_os_str().to_string_lossy().to_string(), + alternative_path: mod_nr_candidate.as_os_str().to_string_lossy().to_string(), + }), } } -fn find_in_name_to_id(file_manager: &FileManager, path: &PathBuf) -> Result { - file_manager - .name_to_id(path.clone()) - .ok_or_else(|| path.as_os_str().to_string_lossy().to_string()) -} /// Returns true if a module's child modules are expected to be in the same directory. /// Returns false if they are expected to be in a subdirectory matching the name of the module. fn should_check_siblings_for_module(module_path: &Path, parent_path: &Path) -> bool { @@ -907,7 +896,22 @@ mod tests { } #[test] - fn find_module_can_find_module_in_the_same_directory() { + fn find_module_errors_because_cannot_find_sibling_mod_for_regular_name() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - foo.nr + // - bar.nr + let foo_file_id = add_file(&mut fm, &dir.join("foo.nr")); + add_file(&mut fm, &dir.join("bar.nr")); + + let result = find_module(&fm, foo_file_id, "bar"); + assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); + } + + #[test] + fn find_module_cannot_find_module_in_the_same_directory_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -923,11 +927,42 @@ mod tests { let bar_file_id = find_module(&fm, lib_file_id, "bar").unwrap(); // `mod foo` from `bar.nr` should find `foo.nr` - find_module(&fm, bar_file_id, "foo").unwrap(); + let result = find_module(&fm, bar_file_id, "foo"); + assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); + } + + #[test] + fn find_module_can_find_module_in_sibling_dir_for_regular_name() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - sub_dir.nr + // - sub_dir/foo.nr + let sub_dir_file_id = add_file(&mut fm, &dir.join("sub_dir.nr")); + add_file(&mut fm, &dir.join("sub_dir").join("foo.nr")); + + // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` + find_module(&fm, sub_dir_file_id, "foo").unwrap(); } #[test] - fn find_module_can_find_module_nested_in_directory() { + fn find_module_can_find_module_in_sibling_dir_mod_nr_for_regular_name() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - sub_dir.nr + // - sub_dir/foo/mod.nr + let sub_dir_file_id = add_file(&mut fm, &dir.join("sub_dir.nr")); + add_file(&mut fm, &dir.join("sub_dir").join("foo").join("mod.nr")); + + // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` + find_module(&fm, sub_dir_file_id, "foo").unwrap(); + } + + #[test] + fn find_module_can_find_module_in_sibling_dir_for_special_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -947,7 +982,7 @@ mod tests { } #[test] - fn find_module_can_find_as_mod_dot_nr() { + fn find_module_can_find_as_mod_dot_nr_for_special_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -962,43 +997,40 @@ mod tests { } #[test] - fn find_module_errors_if_module_is_found_in_name_dot_nr_and_name_slash_mod_dot_nr() { + fn find_module_errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr_for_regular_name( + ) { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); // Create this tree structure: - // - lib.nr // - foo.nr - // - foo/mod.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("foo.nr")); - add_file(&mut fm, &dir.join("foo").join("mod.nr")); + // - foo/bar.nr + // - foo/bar/mod.nr + let foo_file_id = add_file(&mut fm, &dir.join("foo.nr")); + add_file(&mut fm, &dir.join("foo").join("bar.nr")); + add_file(&mut fm, &dir.join("foo").join("bar").join("mod.nr")); - // Check that searching "foo" gives an error - let result = find_module(&fm, lib_file_id, "foo"); + // Check that `mod bar` from `foo` gives an error + let result = find_module(&fm, foo_file_id, "bar"); assert!(matches!(result, Err(DefCollectorErrorKind::OverlappingModuleDecls { .. }))); } #[test] - fn find_module_errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr() { + fn find_module_errors_if_module_is_found_in_name_dot_nr_and_name_slash_mod_dot_nr_for_special_name( + ) { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); // Create this tree structure: // - lib.nr // - foo.nr - // - bar.nr - // - foo/bar.nr + // - foo/mod.nr let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); add_file(&mut fm, &dir.join("foo.nr")); - add_file(&mut fm, &dir.join("bar.nr")); - add_file(&mut fm, &dir.join("foo").join("bar.nr")); - - // Find `mod foo` from `lib` - let foo_file_id = find_module(&fm, lib_file_id, "foo").unwrap(); + add_file(&mut fm, &dir.join("foo").join("mod.nr")); - // Check that `mod bar` from `foo` gives an error - let result = find_module(&fm, foo_file_id, "bar"); + // Check that searching "foo" gives an error + let result = find_module(&fm, lib_file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::OverlappingModuleDecls { .. }))); } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 84ec4704a7e..37c5a460667 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -27,9 +27,9 @@ pub enum DefCollectorErrorKind { #[error("duplicate {typ} found in namespace")] Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident }, #[error("unresolved import")] - UnresolvedModuleDecl { mod_name: Ident, expected_path: String }, + UnresolvedModuleDecl { mod_name: Ident, expected_path: String, alternative_path: String }, #[error("overlapping imports")] - OverlappingModuleDecls { mod_name: Ident, overlapping_paths: Vec }, + OverlappingModuleDecls { mod_name: Ident, expected_path: String, alternative_path: String }, #[error("path resolution error")] PathResolutionError(PathResolutionError), #[error("Non-struct type used in impl")] @@ -123,23 +123,22 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { diag } } - DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path } => { + DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path, alternative_path } => { let span = mod_name.0.span(); let mod_name = &mod_name.0.contents; Diagnostic::simple_error( - format!("No module `{mod_name}` at path `{expected_path}`"), + format!("No module `{mod_name}` at path `{expected_path}` or `{alternative_path}`"), String::new(), span, ) } - DefCollectorErrorKind::OverlappingModuleDecls { mod_name, overlapping_paths } => { + DefCollectorErrorKind::OverlappingModuleDecls { mod_name, expected_path, alternative_path } => { let span = mod_name.0.span(); let mod_name = &mod_name.0.contents; - let overlapping_paths: String = overlapping_paths.iter().map(|overlapping_path| overlapping_path.to_owned() + "\n").collect(); Diagnostic::simple_error( - format!("Overlapping modules `{mod_name}` at paths:\n`{overlapping_paths}`"), + format!("Overlapping modules `{mod_name}` at path `{expected_path}` and `{alternative_path}`"), String::new(), span, ) From 7dfb94dab01a20926bb4bbe79aeb3b9c2c583504 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 15:37:24 -0300 Subject: [PATCH 26/41] Simplify a bit more --- .../src/hir/def_collector/dc_mod.rs | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index dc2ef1b54f6..a8633abdeda 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -694,30 +694,20 @@ fn find_module( // This is "mod_name" let mod_name_str = &mod_name.0.contents; - // The logic of finding a module depends on whether the module we are in (anchor) is - // one of the special "main.nr", "lib.nr", "mod.nr" or "{mod_name}.nr" names. - let (mod_name_candidate, mod_nr_candidate) = - if should_check_siblings_for_module(&anchor_path, anchor_dir) { - // We are in a "main.nr" (or similar) file. - // A "mod mod_name;" statement should look up in "mod_name.nr" or "mod_name/mod.nr". - ( - // Check "mod_name.nr" - anchor_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")), - // Check "mod_name/mod.nr" - anchor_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")), - ) - } else { - // We are not in a "main.nr" (or similar file), so we are in, say, "foo.nr". - // A "mod mod_name;" statement should look up in "foo/mod_name.nr" or "foo/mod_name/mod.nr"; - ( - // Check "foo/mod_name.nr" - anchor_path.join(format!("{mod_name_str}.{FILE_EXTENSION}")), - // Check "foo/mod_name/mod.nr" - anchor_path.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")), - ) - }; + // If we are in a special name like "main.nr", "lib.nr", "mod.nr" or "{mod_name}.nr", + // the search starts at the same directory, otherwise it starts in a nested directory. + let start_dir = if should_check_siblings_for_module(&anchor_path, anchor_dir) { + anchor_dir + } else { + anchor_path.as_path() + }; + // Check "mod_name.nr" + let mod_name_candidate = start_dir.join(format!("{mod_name_str}.{FILE_EXTENSION}")); let mod_name_result = file_manager.name_to_id(mod_name_candidate.clone()); + + // Check "mod_name/mod.nr" + let mod_nr_candidate = start_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")); let mod_nr_result = file_manager.name_to_id(mod_nr_candidate.clone()); match (mod_nr_result, mod_name_result) { From 404658668c126131c09326df75d1bcf5b124309a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 28 Jun 2024 17:43:11 -0300 Subject: [PATCH 27/41] clippy --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index a8633abdeda..b0ff823db38 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -707,7 +707,7 @@ fn find_module( let mod_name_result = file_manager.name_to_id(mod_name_candidate.clone()); // Check "mod_name/mod.nr" - let mod_nr_candidate = start_dir.join(&mod_name_str).join(format!("mod.{FILE_EXTENSION}")); + let mod_nr_candidate = start_dir.join(mod_name_str).join(format!("mod.{FILE_EXTENSION}")); let mod_nr_result = file_manager.name_to_id(mod_nr_candidate.clone()); match (mod_nr_result, mod_name_result) { @@ -829,7 +829,7 @@ mod tests { fn add_file(file_manager: &mut FileManager, file_name: &Path) -> FileId { file_manager - .add_file_with_source(&file_name, "fn foo() {}".to_string()) + .add_file_with_source(file_name, "fn foo() {}".to_string()) .expect("could not add file to file manager and obtain a FileId") } From bb5e8909198f81a016da4855e74d14857bc1dbeb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 1 Jul 2024 07:45:41 -0300 Subject: [PATCH 28/41] Document `mod.nr` --- .../noir/modules_packages_crates/modules.md | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/docs/docs/noir/modules_packages_crates/modules.md b/docs/docs/noir/modules_packages_crates/modules.md index ae822a1cff4..9fffd925b7b 100644 --- a/docs/docs/noir/modules_packages_crates/modules.md +++ b/docs/docs/noir/modules_packages_crates/modules.md @@ -49,6 +49,27 @@ crate ``` +The module filename may also be the name of the module as a directory with the contents in a +file named `mod.nr` within that directory. The above example can alternatively be expressed like this: + +Filename : `src/main.nr` + +```rust +mod foo; + +fn main() { + foo::hello_world(); +} +``` + +Filename : `src/foo/mod.nr` + +```rust +fn from_foo() {} +``` + +Note that it's an error to have both files `src/foo.nr` and `src/foo/mod.nr` in the filesystem. + ### Importing a module throughout the tree All modules are accessible from the `crate::` namespace. @@ -103,3 +124,28 @@ crate └── bar └── from_bar ``` + +Similar to importing a module in the crate root, modules can be placed in a `mod.nr` file, like this: + +Filename : `src/main.nr` + +```rust +mod foo; + +fn main() { + foo::from_foo(); +} +``` + +Filename : `src/foo/mod.nr` + +```rust +mod bar; +fn from_foo() {} +``` + +Filename : `src/foo/bar/mod.nr` + +```rust +fn from_bar() {} +``` \ No newline at end of file From b60b49c6510c03c86b710d8c9d001cbd697971f4 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 1 Jul 2024 12:35:36 +0100 Subject: [PATCH 29/41] chore: shorten test names --- .../src/hir/def_collector/dc_mod.rs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index b0ff823db38..5dacb153aaa 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -821,7 +821,7 @@ pub(crate) fn collect_global( } #[cfg(test)] -mod tests { +mod find_module_tests { use super::*; use noirc_errors::Spanned; @@ -843,7 +843,7 @@ mod tests { } #[test] - fn find_module_errors_if_cannot_find_file() { + fn errors_if_cannot_find_file() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -856,7 +856,7 @@ mod tests { } #[test] - fn find_module_errors_because_cannot_find_mod_relative_to_main() { + fn errors_because_cannot_find_mod_relative_to_main() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -871,7 +871,7 @@ mod tests { } #[test] - fn find_module_errors_because_cannot_find_mod_relative_to_lib() { + fn errors_because_cannot_find_mod_relative_to_lib() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -886,7 +886,7 @@ mod tests { } #[test] - fn find_module_errors_because_cannot_find_sibling_mod_for_regular_name() { + fn errors_because_cannot_find_sibling_mod_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -901,7 +901,7 @@ mod tests { } #[test] - fn find_module_cannot_find_module_in_the_same_directory_for_regular_name() { + fn cannot_find_module_in_the_same_directory_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -922,7 +922,7 @@ mod tests { } #[test] - fn find_module_can_find_module_in_sibling_dir_for_regular_name() { + fn finds_module_in_sibling_dir_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -937,7 +937,7 @@ mod tests { } #[test] - fn find_module_can_find_module_in_sibling_dir_mod_nr_for_regular_name() { + fn finds_module_in_sibling_dir_mod_nr_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -952,7 +952,7 @@ mod tests { } #[test] - fn find_module_can_find_module_in_sibling_dir_for_special_name() { + fn finds_module_in_sibling_dir_for_special_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -972,7 +972,7 @@ mod tests { } #[test] - fn find_module_can_find_as_mod_dot_nr_for_special_name() { + fn finds_mod_dot_nr_for_special_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -987,8 +987,7 @@ mod tests { } #[test] - fn find_module_errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr_for_regular_name( - ) { + fn errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); From a71f6e5700914758fd952cd27e8b342c8956a240 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:45:18 +0100 Subject: [PATCH 30/41] Delete test_programs/compile_failure/invalid_mod_mod_path/Prover.toml --- test_programs/compile_failure/invalid_mod_mod_path/Prover.toml | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 test_programs/compile_failure/invalid_mod_mod_path/Prover.toml diff --git a/test_programs/compile_failure/invalid_mod_mod_path/Prover.toml b/test_programs/compile_failure/invalid_mod_mod_path/Prover.toml deleted file mode 100644 index e0a68175d07..00000000000 --- a/test_programs/compile_failure/invalid_mod_mod_path/Prover.toml +++ /dev/null @@ -1,2 +0,0 @@ -x = "" -y = "" From 9bf5a0edad09f6d3dd01a01dd740343efae625fc Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:45:42 +0100 Subject: [PATCH 31/41] Delete test_programs/compile_failure/overlapping_mod/Prover.toml --- test_programs/compile_failure/overlapping_mod/Prover.toml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test_programs/compile_failure/overlapping_mod/Prover.toml diff --git a/test_programs/compile_failure/overlapping_mod/Prover.toml b/test_programs/compile_failure/overlapping_mod/Prover.toml deleted file mode 100644 index e69de29bb2d..00000000000 From 461a724b9b3514202864852a9791885536ebbbf7 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:46:53 +0100 Subject: [PATCH 32/41] Delete test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml --- test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml b/test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml deleted file mode 100644 index e69de29bb2d..00000000000 From 9a5c713c9bd5005c49df128f6bbcb09bca6f5eb6 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:49:54 +0100 Subject: [PATCH 33/41] Delete test_programs/compile_failure/invalid_main_sub_lib/Prover.toml --- test_programs/compile_failure/invalid_main_sub_lib/Prover.toml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test_programs/compile_failure/invalid_main_sub_lib/Prover.toml diff --git a/test_programs/compile_failure/invalid_main_sub_lib/Prover.toml b/test_programs/compile_failure/invalid_main_sub_lib/Prover.toml deleted file mode 100644 index e69de29bb2d..00000000000 From dff7ade3e4a436c9c4bc27d8850b97fd1f9b5fb9 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 1 Jul 2024 13:00:38 +0100 Subject: [PATCH 34/41] chore: add failing test case --- .../src/hir/def_collector/dc_mod.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 5dacb153aaa..5bc459979ab 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -986,6 +986,22 @@ mod find_module_tests { find_module(&fm, lib_file_id, "foo").unwrap(); } + #[test] + fn errors_mod_dot_nr_in_same_directory() { + let dir = PathBuf::new(); + let mut fm = FileManager::new(&dir); + + // Create this tree structure: + // - lib.nr + // - mod.nr + let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); + add_file(&mut fm, &dir.join("foo").join("mod.nr")); + + // Check that searching "foo" does not pick up the mod.nr file + let result = find_module(&fm, lib_file_id, "foo"); + assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); + } + #[test] fn errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr_for_regular_name() { let dir = PathBuf::new(); From 454daa926b8c6f15b618c1ac01703c022558e59f Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 1 Jul 2024 13:18:49 +0100 Subject: [PATCH 35/41] Update compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 5bc459979ab..a9bc65b95ba 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -916,7 +916,7 @@ mod find_module_tests { // `mod bar` from `lib.nr` should find `bar.nr` let bar_file_id = find_module(&fm, lib_file_id, "bar").unwrap(); - // `mod foo` from `bar.nr` should find `foo.nr` + // `mod foo` from `bar.nr` should fail to find `foo.nr` let result = find_module(&fm, bar_file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); } From 46712a1791d8a246b215655d3afd61960726f256 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 1 Jul 2024 13:22:35 +0100 Subject: [PATCH 36/41] chore: rename tests --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index a9bc65b95ba..054e2ddd40c 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1003,7 +1003,7 @@ mod find_module_tests { } #[test] - fn errors_if_module_is_found_in_name_dot_nr_and_anchor_slash_name_dot_nr_for_regular_name() { + fn errors_if_file_exists_at_both_potential_module_locations_for_regular_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); @@ -1021,8 +1021,7 @@ mod find_module_tests { } #[test] - fn find_module_errors_if_module_is_found_in_name_dot_nr_and_name_slash_mod_dot_nr_for_special_name( - ) { + fn errors_if_file_exists_at_both_potential_module_locations_for_special_name() { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); From 621bdf5fb3133f8cb3ad401b31f8d39d6f3dc892 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 1 Jul 2024 10:37:38 -0300 Subject: [PATCH 37/41] Restore program that fails to compile --- .../overlapping_dep_and_mod/Nargo.toml | 6 ++++++ .../overlapping_dep_and_mod/bin/Nargo.toml | 8 ++++++++ .../overlapping_dep_and_mod/bin/Prover.toml | 0 .../overlapping_dep_and_mod/bin/src/main.nr | 12 ++++++++++++ .../overlapping_dep_and_mod/foo/Nargo.toml | 7 +++++++ .../overlapping_dep_and_mod/foo/src/lib.nr | 3 +++ 6 files changed, 36 insertions(+) create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/bin/Prover.toml create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml create mode 100644 test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml new file mode 100644 index 00000000000..b2c3e5f94be --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/Nargo.toml @@ -0,0 +1,6 @@ +[workspace] +members = [ + "bin", + "foo", +] +default-member = "bin" diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml new file mode 100644 index 00000000000..57e704462db --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/bin/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "overlapping_dep_and_mod" +type = "bin" +authors = [""] +compiler_version = ">=0.29.0" + +[dependencies] +foo = { path = "../foo" } diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/bin/Prover.toml b/test_programs/compile_failure/overlapping_dep_and_mod/bin/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr b/test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr new file mode 100644 index 00000000000..675e889b7e5 --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/bin/src/main.nr @@ -0,0 +1,12 @@ +fn main() -> pub Field { + assert(foo::bar() + foo::baz() == 3); + assert(foo::bar() == 1); + assert(foo::baz() == 2); + foo::bar() + foo::baz() +} + +mod foo { + pub(crate) fn bar() -> Field { + 1 + } +} diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml b/test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml new file mode 100644 index 00000000000..857d4e722a8 --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/foo/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "foo" +type = "lib" +authors = [""] +compiler_version = ">=0.29.0" + +[dependencies] diff --git a/test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr b/test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr new file mode 100644 index 00000000000..7834e2c9276 --- /dev/null +++ b/test_programs/compile_failure/overlapping_dep_and_mod/foo/src/lib.nr @@ -0,0 +1,3 @@ +pub fn baz() -> Field { + 2 +} From 9656af742c3376329a64fc8a6d6da4ca9a899111 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 1 Jul 2024 11:16:50 -0300 Subject: [PATCH 38/41] Tests that are easier to write and read --- .../src/hir/def_collector/dc_mod.rs | 106 ++++++------------ 1 file changed, 36 insertions(+), 70 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 054e2ddd40c..19aad9b3e9e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -825,11 +825,16 @@ mod find_module_tests { use super::*; use noirc_errors::Spanned; - use std::path::{Path, PathBuf}; + use std::path::PathBuf; + + fn add_file(file_manager: &mut FileManager, dir: &PathBuf, file_name: &str) -> FileId { + let mut target_filename = dir.clone(); + for path in file_name.split("/") { + target_filename = target_filename.join(path); + } - fn add_file(file_manager: &mut FileManager, file_name: &Path) -> FileId { file_manager - .add_file_with_source(file_name, "fn foo() {}".to_string()) + .add_file_with_source(&target_filename, "fn foo() {}".to_string()) .expect("could not add file to file manager and obtain a FileId") } @@ -845,11 +850,9 @@ mod find_module_tests { #[test] fn errors_if_cannot_find_file() { let dir = PathBuf::new(); - let mut fm = FileManager::new(&dir); + let mut fm = FileManager::new(&PathBuf::new()); - // Create a my_dummy_file.nr - // Now we have temp_dir/my_dummy_file.nr - let file_id = add_file(&mut fm, &dir.join("my_dummy_file.nr")); + let file_id = add_file(&mut fm, &dir, &"my_dummy_file.nr"); let result = find_module(&fm, file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -860,11 +863,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - main.nr - // - main/foo.nr - let main_file_id = add_file(&mut fm, &dir.join("main.nr")); - add_file(&mut fm, &dir.join("main").join("foo.nr")); + let main_file_id = add_file(&mut fm, &dir, &"main.nr"); + add_file(&mut fm, &dir, &"main/foo.nr"); let result = find_module(&fm, main_file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -875,11 +875,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - lib.nr - // - lib/foo.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("lib").join("foo.nr")); + let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); + add_file(&mut fm, &dir, "lib/foo.nr"); let result = find_module(&fm, lib_file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -890,11 +887,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - foo.nr - // - bar.nr - let foo_file_id = add_file(&mut fm, &dir.join("foo.nr")); - add_file(&mut fm, &dir.join("bar.nr")); + let foo_file_id = add_file(&mut fm, &dir, &"foo.nr"); + add_file(&mut fm, &dir, &"bar.nr"); let result = find_module(&fm, foo_file_id, "bar"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -905,13 +899,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - lib.nr - // - bar.nr - // - foo.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("bar.nr")); - add_file(&mut fm, &dir.join("foo.nr")); + let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); + add_file(&mut fm, &dir, &"bar.nr"); + add_file(&mut fm, &dir, &"foo.nr"); // `mod bar` from `lib.nr` should find `bar.nr` let bar_file_id = find_module(&fm, lib_file_id, "bar").unwrap(); @@ -926,11 +916,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - sub_dir.nr - // - sub_dir/foo.nr - let sub_dir_file_id = add_file(&mut fm, &dir.join("sub_dir.nr")); - add_file(&mut fm, &dir.join("sub_dir").join("foo.nr")); + let sub_dir_file_id = add_file(&mut fm, &dir, &"sub_dir.nr"); + add_file(&mut fm, &dir, &"sub_dir/foo.nr"); // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` find_module(&fm, sub_dir_file_id, "foo").unwrap(); @@ -941,11 +928,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - sub_dir.nr - // - sub_dir/foo/mod.nr - let sub_dir_file_id = add_file(&mut fm, &dir.join("sub_dir.nr")); - add_file(&mut fm, &dir.join("sub_dir").join("foo").join("mod.nr")); + let sub_dir_file_id = add_file(&mut fm, &dir, &"sub_dir.nr"); + add_file(&mut fm, &dir, &"sub_dir/foo/mod.nr"); // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` find_module(&fm, sub_dir_file_id, "foo").unwrap(); @@ -956,13 +940,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - lib.nr - // - sub_dir.nr - // - sub_dir/foo.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("sub_dir.nr")); - add_file(&mut fm, &dir.join("sub_dir").join("foo.nr")); + let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); + add_file(&mut fm, &dir, &"sub_dir.nr"); + add_file(&mut fm, &dir, &"sub_dir/foo.nr"); // `mod sub_dir` from `lib.nr` should find `sub_dir.nr` let sub_dir_file_id = find_module(&fm, lib_file_id, "sub_dir").unwrap(); @@ -976,11 +956,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - lib.nr - // - foo/mod.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("foo").join("mod.nr")); + let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); + add_file(&mut fm, &dir, &"foo/mod.nr"); // Check that searching "foo" finds the mod.nr file find_module(&fm, lib_file_id, "foo").unwrap(); @@ -991,11 +968,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - lib.nr - // - mod.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("foo").join("mod.nr")); + let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); + add_file(&mut fm, &dir, &"mod.nr"); // Check that searching "foo" does not pick up the mod.nr file let result = find_module(&fm, lib_file_id, "foo"); @@ -1007,13 +981,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - foo.nr - // - foo/bar.nr - // - foo/bar/mod.nr - let foo_file_id = add_file(&mut fm, &dir.join("foo.nr")); - add_file(&mut fm, &dir.join("foo").join("bar.nr")); - add_file(&mut fm, &dir.join("foo").join("bar").join("mod.nr")); + let foo_file_id = add_file(&mut fm, &dir, &"foo.nr"); + add_file(&mut fm, &dir, &"foo/bar.nr"); + add_file(&mut fm, &dir, &"foo/bar/mod.nr"); // Check that `mod bar` from `foo` gives an error let result = find_module(&fm, foo_file_id, "bar"); @@ -1025,13 +995,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - // Create this tree structure: - // - lib.nr - // - foo.nr - // - foo/mod.nr - let lib_file_id = add_file(&mut fm, &dir.join("lib.nr")); - add_file(&mut fm, &dir.join("foo.nr")); - add_file(&mut fm, &dir.join("foo").join("mod.nr")); + let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); + add_file(&mut fm, &dir, &"foo.nr"); + add_file(&mut fm, &dir, &"foo/mod.nr"); // Check that searching "foo" gives an error let result = find_module(&fm, lib_file_id, "foo"); From 7665a5b0345045fc91174844f94940508336cf14 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 1 Jul 2024 12:07:16 -0300 Subject: [PATCH 39/41] Merge match branches --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 19aad9b3e9e..b0f50fafa24 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -716,8 +716,7 @@ fn find_module( expected_path: mod_name_candidate.as_os_str().to_string_lossy().to_string(), alternative_path: mod_nr_candidate.as_os_str().to_string_lossy().to_string(), }), - (Some(mod_nr_id), None) => Ok(mod_nr_id), - (None, Some(name_id)) => Ok(name_id), + (Some(id), None) | (None, Some(id)) => Ok(id), (None, None) => Err(DefCollectorErrorKind::UnresolvedModuleDecl { mod_name: mod_name.clone(), expected_path: mod_name_candidate.as_os_str().to_string_lossy().to_string(), From 254a967fdcea9fd0dc6280c9fe47692703b8d323 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 1 Jul 2024 12:07:48 -0300 Subject: [PATCH 40/41] clippy --- .../src/hir/def_collector/dc_mod.rs | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index b0f50fafa24..e51f0498a35 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -851,7 +851,7 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&PathBuf::new()); - let file_id = add_file(&mut fm, &dir, &"my_dummy_file.nr"); + let file_id = add_file(&mut fm, &dir, "my_dummy_file.nr"); let result = find_module(&fm, file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -862,8 +862,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let main_file_id = add_file(&mut fm, &dir, &"main.nr"); - add_file(&mut fm, &dir, &"main/foo.nr"); + let main_file_id = add_file(&mut fm, &dir, "main.nr"); + add_file(&mut fm, &dir, "main/foo.nr"); let result = find_module(&fm, main_file_id, "foo"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -886,8 +886,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let foo_file_id = add_file(&mut fm, &dir, &"foo.nr"); - add_file(&mut fm, &dir, &"bar.nr"); + let foo_file_id = add_file(&mut fm, &dir, "foo.nr"); + add_file(&mut fm, &dir, "bar.nr"); let result = find_module(&fm, foo_file_id, "bar"); assert!(matches!(result, Err(DefCollectorErrorKind::UnresolvedModuleDecl { .. }))); @@ -898,9 +898,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); - add_file(&mut fm, &dir, &"bar.nr"); - add_file(&mut fm, &dir, &"foo.nr"); + let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); + add_file(&mut fm, &dir, "bar.nr"); + add_file(&mut fm, &dir, "foo.nr"); // `mod bar` from `lib.nr` should find `bar.nr` let bar_file_id = find_module(&fm, lib_file_id, "bar").unwrap(); @@ -915,8 +915,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let sub_dir_file_id = add_file(&mut fm, &dir, &"sub_dir.nr"); - add_file(&mut fm, &dir, &"sub_dir/foo.nr"); + let sub_dir_file_id = add_file(&mut fm, &dir, "sub_dir.nr"); + add_file(&mut fm, &dir, "sub_dir/foo.nr"); // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` find_module(&fm, sub_dir_file_id, "foo").unwrap(); @@ -927,8 +927,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let sub_dir_file_id = add_file(&mut fm, &dir, &"sub_dir.nr"); - add_file(&mut fm, &dir, &"sub_dir/foo/mod.nr"); + let sub_dir_file_id = add_file(&mut fm, &dir, "sub_dir.nr"); + add_file(&mut fm, &dir, "sub_dir/foo/mod.nr"); // `mod foo` from `sub_dir.nr` should find `sub_dir/foo.nr` find_module(&fm, sub_dir_file_id, "foo").unwrap(); @@ -939,9 +939,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); - add_file(&mut fm, &dir, &"sub_dir.nr"); - add_file(&mut fm, &dir, &"sub_dir/foo.nr"); + let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); + add_file(&mut fm, &dir, "sub_dir.nr"); + add_file(&mut fm, &dir, "sub_dir/foo.nr"); // `mod sub_dir` from `lib.nr` should find `sub_dir.nr` let sub_dir_file_id = find_module(&fm, lib_file_id, "sub_dir").unwrap(); @@ -956,7 +956,7 @@ mod find_module_tests { let mut fm = FileManager::new(&dir); let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); - add_file(&mut fm, &dir, &"foo/mod.nr"); + add_file(&mut fm, &dir, "foo/mod.nr"); // Check that searching "foo" finds the mod.nr file find_module(&fm, lib_file_id, "foo").unwrap(); @@ -967,8 +967,8 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); - add_file(&mut fm, &dir, &"mod.nr"); + let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); + add_file(&mut fm, &dir, "mod.nr"); // Check that searching "foo" does not pick up the mod.nr file let result = find_module(&fm, lib_file_id, "foo"); @@ -980,9 +980,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let foo_file_id = add_file(&mut fm, &dir, &"foo.nr"); - add_file(&mut fm, &dir, &"foo/bar.nr"); - add_file(&mut fm, &dir, &"foo/bar/mod.nr"); + let foo_file_id = add_file(&mut fm, &dir, "foo.nr"); + add_file(&mut fm, &dir, "foo/bar.nr"); + add_file(&mut fm, &dir, "foo/bar/mod.nr"); // Check that `mod bar` from `foo` gives an error let result = find_module(&fm, foo_file_id, "bar"); @@ -994,9 +994,9 @@ mod find_module_tests { let dir = PathBuf::new(); let mut fm = FileManager::new(&dir); - let lib_file_id = add_file(&mut fm, &dir, &"lib.nr"); - add_file(&mut fm, &dir, &"foo.nr"); - add_file(&mut fm, &dir, &"foo/mod.nr"); + let lib_file_id = add_file(&mut fm, &dir, "lib.nr"); + add_file(&mut fm, &dir, "foo.nr"); + add_file(&mut fm, &dir, "foo/mod.nr"); // Check that searching "foo" gives an error let result = find_module(&fm, lib_file_id, "foo"); From d98751e7de8bd5bdf2dac6af4a83944c350c83e8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 1 Jul 2024 14:46:31 -0300 Subject: [PATCH 41/41] clippy --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index e51f0498a35..c9198a1d04c 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -824,11 +824,11 @@ mod find_module_tests { use super::*; use noirc_errors::Spanned; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; - fn add_file(file_manager: &mut FileManager, dir: &PathBuf, file_name: &str) -> FileId { - let mut target_filename = dir.clone(); - for path in file_name.split("/") { + fn add_file(file_manager: &mut FileManager, dir: &Path, file_name: &str) -> FileId { + let mut target_filename = PathBuf::from(&dir); + for path in file_name.split('/') { target_filename = target_filename.join(path); }