From 2c22fe555dc41fffc623026b4b8c57d44b869cd2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 4 Sep 2024 17:39:58 -0300 Subject: [PATCH] fix: collect functions generated by attributes (#5930) # Description ## Problem Resolves #5901 ## Summary ## Additional Context ## 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. --------- Co-authored-by: jfecher --- .../noirc_frontend/src/elaborator/comptime.rs | 29 +++---- .../src/hir/def_collector/dc_mod.rs | 75 +++++++++++-------- .../unquote_function/Nargo.toml | 7 ++ .../unquote_function/src/main.nr | 12 +++ .../unquote_struct/src/main.nr | 6 +- 5 files changed, 82 insertions(+), 47 deletions(-) create mode 100644 test_programs/compile_success_empty/unquote_function/Nargo.toml create mode 100644 test_programs/compile_success_empty/unquote_function/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index cf6679af8e9..7da5efd0b5a 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -326,21 +326,24 @@ impl<'context> Elaborator<'context> { ) { match item { TopLevelStatement::Function(function) => { - let id = self.interner.push_empty_fn(); - let module = self.module_id(); - self.interner.push_function(id, &function.def, module, location); + let module_id = self.module_id(); - if self.interner.is_in_lsp_mode() && !function.def.is_test() { - self.interner.register_function(id, &function.def); + if let Some(id) = dc_mod::collect_function( + self.interner, + self.def_maps.get_mut(&self.crate_id).unwrap(), + &function, + module_id, + self.file, + &mut self.errors, + ) { + let functions = vec![(self.local_module, id, function)]; + generated_items.functions.push(UnresolvedFunctions { + file_id: self.file, + functions, + trait_id: None, + self_type: None, + }); } - - let functions = vec![(self.local_module, id, function)]; - generated_items.functions.push(UnresolvedFunctions { - file_id: self.file, - functions, - trait_id: None, - self_type: None, - }); } TopLevelStatement::TraitImpl(mut trait_impl) => { let (methods, associated_types, associated_constants) = 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 d6432b0ca56..79b55f60b76 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -248,25 +248,16 @@ impl<'a> ModCollector<'a> { let module = ModuleId { krate, local_id: self.module_id }; for function in functions { - // check if optional field attribute is compatible with native field - if let Some(field) = function.attributes().get_field_attribute() { - if !is_native_field(&field) { - continue; - } - } - - let name = function.name_ident().clone(); - let func_id = context.def_interner.push_empty_fn(); - let visibility = function.def.visibility; - - // First create dummy function in the DefInterner - // So that we can get a FuncId - let location = Location::new(function.span(), self.file_id); - context.def_interner.push_function(func_id, &function.def, module, location); - - if context.def_interner.is_in_lsp_mode() && !function.def.is_test() { - context.def_interner.register_function(func_id, &function.def); - } + let Some(func_id) = collect_function( + &mut context.def_interner, + &mut self.def_collector.def_map, + &function, + module, + self.file_id, + &mut errors, + ) else { + continue; + }; // Now link this func_id to a crate level map with the noir function and the module id // Encountering a NoirFunction, we retrieve it's module_data to get the namespace @@ -275,19 +266,6 @@ impl<'a> ModCollector<'a> { // With this method we iterate each function in the Crate and not each module // This may not be great because we have to pull the module_data for each function unresolved_functions.push_fn(self.module_id, func_id, function); - - // Add function to scope/ns of the module - let result = self.def_collector.def_map.modules[self.module_id.0] - .declare_function(name, visibility, func_id); - - if let Err((first_def, second_def)) = result { - let error = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::Function, - first_def, - second_def, - }; - errors.push((error.into(), self.file_id)); - } } self.def_collector.items.functions.push(unresolved_functions); @@ -842,6 +820,39 @@ fn push_child_module( Ok(mod_id) } +pub fn collect_function( + interner: &mut NodeInterner, + def_map: &mut CrateDefMap, + function: &NoirFunction, + module: ModuleId, + file: FileId, + errors: &mut Vec<(CompilationError, FileId)>, +) -> Option { + if let Some(field) = function.attributes().get_field_attribute() { + if !is_native_field(&field) { + return None; + } + } + let name = function.name_ident().clone(); + let func_id = interner.push_empty_fn(); + let visibility = function.def.visibility; + let location = Location::new(function.span(), file); + interner.push_function(func_id, &function.def, module, location); + if interner.is_in_lsp_mode() && !function.def.is_test() { + interner.register_function(func_id, &function.def); + } + let result = def_map.modules[module.local_id.0].declare_function(name, visibility, func_id); + if let Err((first_def, second_def)) = result { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::Function, + first_def, + second_def, + }; + errors.push((error.into(), file)); + } + Some(func_id) +} + pub fn collect_struct( interner: &mut NodeInterner, def_map: &mut CrateDefMap, diff --git a/test_programs/compile_success_empty/unquote_function/Nargo.toml b/test_programs/compile_success_empty/unquote_function/Nargo.toml new file mode 100644 index 00000000000..aa56a5798df --- /dev/null +++ b/test_programs/compile_success_empty/unquote_function/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "unquote_function" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/unquote_function/src/main.nr b/test_programs/compile_success_empty/unquote_function/src/main.nr new file mode 100644 index 00000000000..273a091b26d --- /dev/null +++ b/test_programs/compile_success_empty/unquote_function/src/main.nr @@ -0,0 +1,12 @@ +fn main() { + bar(); +} + +#[output_function] +fn foo() {} + +comptime fn output_function(_f: FunctionDefinition) -> Quoted { + quote { + fn bar() {} + } +} diff --git a/test_programs/compile_success_empty/unquote_struct/src/main.nr b/test_programs/compile_success_empty/unquote_struct/src/main.nr index e90711dd710..603440b5c76 100644 --- a/test_programs/compile_success_empty/unquote_struct/src/main.nr +++ b/test_programs/compile_success_empty/unquote_struct/src/main.nr @@ -10,11 +10,13 @@ fn foo(x: Field, y: u32) -> u32 { // Given a function, wrap its parameters in a struct definition comptime fn output_struct(f: FunctionDefinition) -> Quoted { - let fields = f.parameters().map(|param: (Quoted, Type)| { + let fields = f.parameters().map( + |param: (Quoted, Type)| { let name = param.0; let typ = param.1; quote { $name: $typ, } - }).join(quote {}); + } + ).join(quote {}); quote { struct Foo { $fields }