diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 46406083a09..e3cc4840453 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -93,14 +93,6 @@ pub struct Elaborator<'context> { in_unconstrained_fn: bool, nested_loops: usize, - /// True if the current module is a contract. - /// This is usually determined by self.path_resolver.module_id(), but it can - /// be overridden for impls. Impls are an odd case since the methods within resolve - /// as if they're in the parent module, but should be placed in a child module. - /// Since they should be within a child module, in_contract is manually set to false - /// for these so we can still resolve them in the parent module without them being in a contract. - in_contract: bool, - /// Contains a mapping of the current struct or functions's generics to /// unique type variables if we're resolving a struct. Empty otherwise. /// This is a Vec rather than a map to preserve the order a functions generics @@ -183,7 +175,6 @@ impl<'context> Elaborator<'context> { file: FileId::dummy(), in_unconstrained_fn: false, nested_loops: 0, - in_contract: false, generics: Vec::new(), lambda_stack: Vec::new(), self_type: None, @@ -319,12 +310,6 @@ impl<'context> Elaborator<'context> { let old_function = std::mem::replace(&mut self.current_function, Some(id)); - // Without this, impl methods can accidentally be placed in contracts. See #3254 - let was_in_contract = self.in_contract; - if self.self_type.is_some() { - self.in_contract = false; - } - self.scopes.start_function(); let old_item = std::mem::replace(&mut self.current_item, Some(DependencyId::Function(id))); @@ -429,7 +414,6 @@ impl<'context> Elaborator<'context> { self.type_variables.clear(); self.in_unconstrained_fn = false; self.interner.update_fn(id, hir_func); - self.in_contract = was_in_contract; self.current_function = old_function; self.current_item = old_item; } @@ -610,11 +594,13 @@ impl<'context> Elaborator<'context> { ) { self.current_function = Some(func_id); - // Without this, impl methods can accidentally be placed in contracts. See #3254 - let was_in_contract = self.in_contract; - if self.self_type.is_some() { - self.in_contract = false; - } + let in_contract = if self.self_type.is_some() { + // Without this, impl methods can accidentally be placed in contracts. + // See: https://github.com/noir-lang/noir/issues/3254 + false + } else { + self.in_contract() + }; self.scopes.start_function(); self.current_item = Some(DependencyId::Function(func_id)); @@ -623,12 +609,13 @@ impl<'context> Elaborator<'context> { let id = self.interner.function_definition_id(func_id); let name_ident = HirIdent::non_trait_method(id, location); - let is_entry_point = self.is_entry_point_function(func); + let is_entry_point = self.is_entry_point_function(func, in_contract); self.run_lint(|_| lints::inlining_attributes(func).map(Into::into)); self.run_lint(|_| lints::missing_pub(func, is_entry_point).map(Into::into)); self.run_lint(|elaborator| { - lints::unnecessary_pub_return(func, elaborator.pub_allowed(func)).map(Into::into) + lints::unnecessary_pub_return(func, elaborator.pub_allowed(func, in_contract)) + .map(Into::into) }); self.run_lint(|_| lints::oracle_not_marked_unconstrained(func).map(Into::into)); self.run_lint(|elaborator| { @@ -644,7 +631,7 @@ impl<'context> Elaborator<'context> { let has_no_predicates_attribute = func.attributes().is_no_predicates(); let should_fold = func.attributes().is_foldable(); let has_inline_attribute = has_no_predicates_attribute || should_fold; - let is_pub_allowed = self.pub_allowed(func); + let is_pub_allowed = self.pub_allowed(func, in_contract); self.add_generics(&func.def.generics); let mut trait_constraints = self.resolve_trait_constraints(&func.def.where_clause); @@ -727,7 +714,6 @@ impl<'context> Elaborator<'context> { }; self.interner.push_fn_meta(meta, func_id); - self.in_contract = was_in_contract; self.current_function = None; self.scopes.end_function(); self.current_item = None; @@ -752,12 +738,23 @@ impl<'context> Elaborator<'context> { /// True if the `pub` keyword is allowed on parameters in this function /// `pub` on function parameters is only allowed for entry point functions - fn pub_allowed(&self, func: &NoirFunction) -> bool { - self.is_entry_point_function(func) || func.attributes().is_foldable() + fn pub_allowed(&self, func: &NoirFunction, in_contract: bool) -> bool { + self.is_entry_point_function(func, in_contract) || func.attributes().is_foldable() + } + + /// Returns `true` if the current module is a contract. + /// + /// This is usually determined by `self.module_id()`, but it can + /// be overridden for impls. Impls are an odd case since the methods within resolve + /// as if they're in the parent module, but should be placed in a child module. + /// Since they should be within a child module, they should be elaborated as if + /// `in_contract` is `false` so we can still resolve them in the parent module without them being in a contract. + fn in_contract(&self) -> bool { + self.module_id().module(self.def_maps).is_contract } - fn is_entry_point_function(&self, func: &NoirFunction) -> bool { - if self.in_contract { + fn is_entry_point_function(&self, func: &NoirFunction, in_contract: bool) -> bool { + if in_contract { func.attributes().is_contract_entry_point() } else { func.name() == MAIN_FUNCTION @@ -1289,7 +1286,7 @@ impl<'context> Elaborator<'context> { self.current_item = Some(DependencyId::Global(global_id)); let let_stmt = global.stmt_def; - if !self.module_id().module(self.def_maps).is_contract + if !self.in_contract() && let_stmt.attributes.iter().any(|attr| matches!(attr, SecondaryAttribute::Abi(_))) { let span = let_stmt.pattern.span(); @@ -1418,12 +1415,9 @@ impl<'context> Elaborator<'context> { for (local_module, id, func) in &mut function_set.functions { self.local_module = *local_module; - let was_in_contract = self.in_contract; - self.in_contract = self.module_id().module(self.def_maps).is_contract; self.recover_generics(|this| { this.define_function_meta(func, *id, false); }); - self.in_contract = was_in_contract; } } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 06f7b0f9b6c..66b04fe5792 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -254,8 +254,7 @@ impl<'context> Elaborator<'context> { } let expected_generic_count = struct_type.borrow().generics.len(); - - if !self.in_contract + if !self.in_contract() && self .interner .struct_attributes(&struct_type.borrow().id)