Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: mod.nr entrypoint #5039

Merged
merged 51 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
0af1f6f
wip
michaeljklein May 16, 2024
0a682aa
feat: add skip_prelude hidden cli option, skip inject_prelude when true
michaeljklein May 16, 2024
0f682a3
cargo fmt / clippy
michaeljklein May 16, 2024
a5c4c90
add missing parameter
michaeljklein May 16, 2024
ea02811
wip debugging
michaeljklein May 16, 2024
8ef80ae
Merge branch 'michaeljklein/no-stdlib-cli' into michaeljklein/mod-nr-…
michaeljklein May 16, 2024
fd76bd8
wip debugging: find_module doesn't appear to get called for import?
michaeljklein May 16, 2024
e35a630
debugging (def maps)
michaeljklein May 16, 2024
f214832
wip debugging
michaeljklein May 16, 2024
1acbca1
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
michaeljklein Jun 10, 2024
fa75c8b
debugging cleanup
michaeljklein Jun 10, 2024
414e765
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
michaeljklein Jun 10, 2024
ef4b60c
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
TomAFrench Jun 11, 2024
3357176
chore: fmt
TomAFrench Jun 11, 2024
8407360
chore: fix compilation
TomAFrench Jun 11, 2024
c48932f
add missing "mod foo;" declarations: tests passing
michaeljklein Jun 11, 2024
3ff4884
nargo fmt, add error for overlapping paths, restrict 'main' in path, …
michaeljklein Jun 11, 2024
e6dbe62
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
michaeljklein Jun 20, 2024
98cc4dd
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
TomAFrench Jun 25, 2024
239ecc1
Update compiler/fm/src/file_map.rs
michaeljklein Jun 27, 2024
ccd8576
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
michaeljklein Jun 27, 2024
5acff7f
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
asterite Jun 28, 2024
71ed978
Make things compile again
asterite Jun 28, 2024
0f565f1
Remove the `skip_prelude` option
asterite Jun 28, 2024
d99d7f1
No need for name_to_id to be pub
asterite Jun 28, 2024
ff6f7e0
Pass reference to Ident, only clone when needed
asterite Jun 28, 2024
5559bc9
No need to create files when dealing with FileManager
asterite Jun 28, 2024
e13f443
Simplify tests a bit
asterite Jun 28, 2024
70e880f
Add a few more tests, and fix the overlapping_lib_and_mod program tha…
asterite Jun 28, 2024
80291e2
Remove maybe wrong compile failure case
asterite Jun 28, 2024
e1601bb
More tests and some code cleanup
asterite Jun 28, 2024
032e973
Undo unrelated changes
asterite Jun 28, 2024
7232716
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
asterite Jun 28, 2024
8e86139
Get the logic right
asterite Jun 28, 2024
7dfb94d
Simplify a bit more
asterite Jun 28, 2024
4046586
clippy
asterite Jun 28, 2024
bb5e890
Document `mod.nr`
asterite Jul 1, 2024
b60b49c
chore: shorten test names
TomAFrench Jul 1, 2024
a71f6e5
Delete test_programs/compile_failure/invalid_mod_mod_path/Prover.toml
TomAFrench Jul 1, 2024
9bf5a0e
Delete test_programs/compile_failure/overlapping_mod/Prover.toml
TomAFrench Jul 1, 2024
461a724
Delete test_programs/compile_success_empty/mod_nr_entrypoint/Prover.toml
TomAFrench Jul 1, 2024
9a5c713
Delete test_programs/compile_failure/invalid_main_sub_lib/Prover.toml
TomAFrench Jul 1, 2024
dff7ade
chore: add failing test case
TomAFrench Jul 1, 2024
454daa9
Update compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
TomAFrench Jul 1, 2024
46712a1
chore: rename tests
TomAFrench Jul 1, 2024
d94ace3
Merge branch 'master' into michaeljklein/mod-nr-entrypoint
asterite Jul 1, 2024
621bdf5
Restore program that fails to compile
asterite Jul 1, 2024
9656af7
Tests that are easier to write and read
asterite Jul 1, 2024
7665a5b
Merge match branches
asterite Jul 1, 2024
254a967
clippy
asterite Jul 1, 2024
d98751e
clippy
asterite Jul 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ impl From<&PathBuf> for PathString {
#[derive(Debug, Clone)]
pub struct FileMap {
files: SimpleFiles<PathString, String>,
name_to_id: HashMap<PathString, FileId>,
// TODO remove before PR
asterite marked this conversation as resolved.
Show resolved Hide resolved
pub name_to_id: HashMap<PathString, FileId>,
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
Expand Down
10 changes: 9 additions & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ pub struct CompileOptions {
#[arg(long, hide = true)]
pub use_elaborator: 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,
Expand Down Expand Up @@ -258,12 +262,14 @@ 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)
Expand Down Expand Up @@ -301,6 +307,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(|| {
Expand Down Expand Up @@ -342,6 +349,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
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/tests/stdlib_warnings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
21 changes: 18 additions & 3 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,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![];
Expand All @@ -274,6 +275,7 @@ impl DefCollector {
dep.crate_id,
context,
use_elaborator,
skip_prelude,
macro_processors,
));

Expand Down Expand Up @@ -308,15 +310,25 @@ 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.
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(),
Expand All @@ -343,6 +355,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);
Expand Down
56 changes: 28 additions & 28 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -571,6 +571,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) {
Expand Down Expand Up @@ -709,34 +715,28 @@ 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}"))
} else {
// Otherwise, we check for children of the anchor at `base/anchor/mod_name.nr`
anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}"))
};

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 {
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()
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 {
// If there's no filename, we arbitrarily return true.
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
// Alternatively, we could panic, but this is left to a different step where we
// ideally have some source location to issue an error.
true
Err(mod_nr_candidate.as_os_str().to_string_lossy().to_string())
}
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -123,6 +124,7 @@ impl CrateDefMap {
ast,
root_file_id,
use_elaborator,
skip_prelude,
macro_processors,
));

Expand Down
10 changes: 10 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ pub fn resolve_import(
import_directive: &ImportDirective,
def_maps: &BTreeMap<CrateId, CrateDefMap>,
) -> Result<ResolvedImport, PathResolutionError> {
// // TODO cleanup
asterite marked this conversation as resolved.
Show resolved Hide resolved
// dbg!("resolve_import", crate_id, import_directive, def_maps.keys().collect::<Vec<_>>());

let allow_contracts =
allow_referencing_contracts(def_maps, crate_id, import_directive.module_id);

Expand All @@ -98,6 +101,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
Expand Down Expand Up @@ -147,6 +153,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
Expand Down Expand Up @@ -221,6 +230,7 @@ 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()));
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "mod_nr_entrypoint"
type = "bin"
authors = [""]
compiler_version = ">=0.29.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn in_baz_mod() -> bool {
true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn in_bar_mod() -> Field {
2
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn in_foo_mod() -> Field {
1
}
Original file line number Diff line number Diff line change
@@ -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);
}
3 changes: 2 additions & 1 deletion tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,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);
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());
}
11 changes: 6 additions & 5 deletions tooling/lsp/src/notifications/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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) {
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();

Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/code_lens_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/goto_declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/goto_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -161,8 +162,10 @@ 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)
}

Expand Down
1 change: 1 addition & 0 deletions tooling/nargo_cli/src/cli/export_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading
Loading