From 55d8e05b84ae23a1a218e7027542af175979885c Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 17 Jun 2024 15:08:25 -0500 Subject: [PATCH] chore: Use the elaborator by default (#5246) # Description ## Problem\* ## Summary\* Removes the `--use-elaborator` flag, replacing it with `--use-legacy` to enable the old name resolver & type checker passes which are now off by default in favor of the elaborator. ## Additional Context No user visible changes are expected from this PR. ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_driver/src/lib.rs | 12 ++-- compiler/noirc_frontend/src/elaborator/mod.rs | 2 + .../src/hir/def_collector/dc_crate.rs | 6 +- .../noirc_frontend/src/hir/def_map/mod.rs | 4 +- tooling/nargo_cli/build.rs | 55 +++++++++---------- tooling/nargo_cli/src/cli/check_cmd.rs | 6 +- tooling/nargo_cli/src/cli/export_cmd.rs | 2 +- tooling/nargo_cli/src/cli/test_cmd.rs | 4 +- 8 files changed, 46 insertions(+), 45 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 2c8679eaa95..dde2bd08d74 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -99,9 +99,9 @@ pub struct CompileOptions { #[arg(long, hide = true)] pub force_brillig: bool, - /// Enable the experimental elaborator pass + /// Use the deprecated name resolution & type checking passes instead of the elaborator #[arg(long, hide = true)] - pub use_elaborator: bool, + pub use_legacy: bool, /// Outputs the paths to any modified artifacts #[arg(long, hide = true)] @@ -257,13 +257,13 @@ pub fn check_crate( crate_id: CrateId, deny_warnings: bool, disable_macros: bool, - use_elaborator: bool, + use_legacy: 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_legacy, macros); errors.extend(diagnostics.into_iter().map(|(error, file_id)| { let diagnostic = CustomDiagnostic::from(&error); diagnostic.in_file(file_id) @@ -300,7 +300,7 @@ pub fn compile_main( crate_id, options.deny_warnings, options.disable_macros, - options.use_elaborator, + options.use_legacy, )?; let main = context.get_main_function(&crate_id).ok_or_else(|| { @@ -341,7 +341,7 @@ pub fn compile_contract( crate_id, options.deny_warnings, options.disable_macros, - options.use_elaborator, + options.use_legacy, )?; // TODO: We probably want to error if contracts is empty diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 93e8b36f4e0..0b7e5a58849 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1138,6 +1138,7 @@ impl<'context> Elaborator<'context> { fn elaborate_global(&mut self, global: UnresolvedGlobal) { let old_module = std::mem::replace(&mut self.local_module, global.module_id); let old_file = std::mem::replace(&mut self.file, global.file_id); + let old_item = self.current_item.take(); let global_id = global.global_id; self.current_item = Some(DependencyId::Global(global_id)); @@ -1171,6 +1172,7 @@ impl<'context> Elaborator<'context> { self.type_variables.clear(); self.local_module = old_module; self.file = old_file; + self.current_item = old_item; } fn elaborate_comptime_global(&mut self, global_id: GlobalId) { 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 4d64481bc9f..216ed5fc545 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -256,7 +256,7 @@ impl DefCollector { context: &mut Context, ast: SortedModule, root_file_id: FileId, - use_elaborator: bool, + use_legacy: bool, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; @@ -273,7 +273,7 @@ impl DefCollector { errors.extend(CrateDefMap::collect_defs( dep.crate_id, context, - use_elaborator, + use_legacy, macro_processors, )); @@ -351,7 +351,7 @@ impl DefCollector { } } - if use_elaborator { + if !use_legacy { let mut more_errors = Elaborator::elaborate(context, crate_id, def_collector.items); errors.append(&mut more_errors); return errors; diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 19e06387d43..59205f74d89 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -73,7 +73,7 @@ impl CrateDefMap { pub fn collect_defs( crate_id: CrateId, context: &mut Context, - use_elaborator: bool, + use_legacy: bool, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { // Check if this Crate has already been compiled @@ -122,7 +122,7 @@ impl CrateDefMap { context, ast, root_file_id, - use_elaborator, + use_legacy, macro_processors, )); diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 846160320cf..d2822ba11ab 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -87,23 +87,23 @@ fn generate_execution_success_tests(test_file: &mut File, test_data_dir: &Path) test_file, r#" #[test] -fn execution_success_{test_name}() {{ +fn execution_success_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("execute").arg("--force"); + cmd.arg("execute").arg("--force").arg("--use-legacy"); cmd.assert().success(); }} #[test] -fn execution_success_elaborator_{test_name}() {{ +fn execution_success_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("execute").arg("--force").arg("--use-elaborator"); + cmd.arg("execute").arg("--force"); cmd.assert().success(); }} @@ -146,23 +146,23 @@ fn generate_execution_failure_tests(test_file: &mut File, test_data_dir: &Path) test_file, r#" #[test] -fn execution_failure_{test_name}() {{ +fn execution_failure_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("execute").arg("--force"); + cmd.arg("execute").arg("--force").arg("--use-legacy"); cmd.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not()); }} #[test] -fn execution_failure_elaborator_{test_name}() {{ +fn execution_failure_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("execute").arg("--force").arg("--use-elaborator"); + cmd.arg("execute").arg("--force"); cmd.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not()); }} @@ -194,23 +194,23 @@ fn generate_noir_test_success_tests(test_file: &mut File, test_data_dir: &Path) test_file, r#" #[test] -fn noir_test_success_{test_name}() {{ +fn noir_test_success_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("test"); + cmd.arg("test").arg("--use-legacy"); cmd.assert().success(); }} #[test] -fn noir_test_success_elaborator_{test_name}() {{ +fn noir_test_success_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("test").arg("--use-elaborator"); + cmd.arg("test"); cmd.assert().success(); }} @@ -242,23 +242,23 @@ fn generate_noir_test_failure_tests(test_file: &mut File, test_data_dir: &Path) test_file, r#" #[test] -fn noir_test_failure_{test_name}() {{ +fn noir_test_failure_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("test"); + cmd.arg("test").arg("--use-legacy"); cmd.assert().failure(); }} #[test] -fn noir_test_failure_elaborator_{test_name}() {{ +fn noir_test_failure_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("test").arg("--use-elaborator"); + cmd.arg("test"); cmd.assert().failure(); }} @@ -293,14 +293,14 @@ fn generate_compile_success_empty_tests(test_file: &mut File, test_data_dir: &Pa test_file, r#" #[test]{comptime_ignored} -fn compile_success_empty_{test_name}() {{ - +fn compile_success_empty_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); cmd.arg("info"); cmd.arg("--json"); cmd.arg("--force"); + cmd.arg("--use-legacy"); let output = cmd.output().expect("Failed to execute command"); @@ -317,14 +317,13 @@ fn compile_success_empty_{test_name}() {{ }} #[test] -fn compile_success_empty_elaborator_{test_name}() {{ +fn compile_success_empty_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); cmd.arg("info"); cmd.arg("--json"); cmd.arg("--force"); - cmd.arg("--use-elaborator"); let output = cmd.output().expect("Failed to execute command"); @@ -367,22 +366,22 @@ fn generate_compile_success_contract_tests(test_file: &mut File, test_data_dir: test_file, r#" #[test] -fn compile_success_contract_{test_name}() {{ +fn compile_success_contract_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("compile").arg("--force"); + cmd.arg("compile").arg("--force").arg("--use-legacy"); cmd.assert().success(); }} #[test] -fn compile_success_contract_elaborator_{test_name}() {{ +fn compile_success_contract_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("compile").arg("--force").arg("--use-elaborator"); + cmd.arg("compile").arg("--force"); cmd.assert().success(); }} @@ -414,22 +413,22 @@ fn generate_compile_failure_tests(test_file: &mut File, test_data_dir: &Path) { test_file, r#" #[test] -fn compile_failure_{test_name}() {{ +fn compile_failure_legacy_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("compile").arg("--force"); + cmd.arg("compile").arg("--force").arg("--use-legacy"); cmd.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not()); }} #[test] -fn compile_failure_elaborator_{test_name}() {{ +fn compile_failure_{test_name}() {{ let test_program_dir = PathBuf::from("{test_dir}"); let mut cmd = Command::cargo_bin("nargo").unwrap(); cmd.arg("--program-dir").arg(test_program_dir); - cmd.arg("compile").arg("--force").arg("--use-elaborator"); + cmd.arg("compile").arg("--force"); cmd.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not()); }} diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index e2e1f147b90..2db3b10429a 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -87,7 +87,7 @@ fn check_package( compile_options.deny_warnings, compile_options.disable_macros, compile_options.silence_warnings, - compile_options.use_elaborator, + compile_options.use_legacy, )?; if package.is_library() || package.is_contract() { @@ -160,9 +160,9 @@ pub(crate) fn check_crate_and_report_errors( deny_warnings: bool, disable_macros: bool, silence_warnings: bool, - use_elaborator: bool, + use_legacy: 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_legacy); 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..ee30b29b0f0 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -89,7 +89,7 @@ fn compile_exported_functions( compile_options.deny_warnings, compile_options.disable_macros, compile_options.silence_warnings, - compile_options.use_elaborator, + compile_options.use_legacy, )?; 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 99c284e5019..1cda8d8382e 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -175,7 +175,7 @@ fn run_test + Default>( crate_id, compile_options.deny_warnings, compile_options.disable_macros, - compile_options.use_elaborator, + compile_options.use_legacy, ) .expect("Any errors should have occurred when collecting test functions"); @@ -209,7 +209,7 @@ fn get_tests_in_package( compile_options.deny_warnings, compile_options.disable_macros, compile_options.silence_warnings, - compile_options.use_elaborator, + compile_options.use_legacy, )?; Ok(context