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 a458814ce9e..aa2bedb37b7 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -1173,6 +1173,11 @@ fn resolve_function_set( resolver.set_self_type(self_type.clone()); resolver.set_trait_id(unresolved_functions.trait_id); + // Without this, impl methods can accidentally be placed in contracts. See #3254 + if self_type.is_some() { + resolver.set_in_contract(false); + } + let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id); interner.push_fn_meta(func_meta, func_id); interner.update_fn(func_id, hir_func); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index a4a85d85929..d527433630f 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -87,6 +87,14 @@ pub struct Resolver<'a> { /// Set to the current type if we're resolving an impl self_type: Option, + /// True if the current module is a contract. + /// This is usually determined by self.path_resolver.module_id(), but it can + /// be overriden 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 @@ -119,6 +127,9 @@ impl<'a> Resolver<'a> { def_maps: &'a BTreeMap, file: FileId, ) -> Resolver<'a> { + let module_id = path_resolver.module_id(); + let in_contract = module_id.module(def_maps).is_contract; + Self { path_resolver, def_maps, @@ -131,6 +142,7 @@ impl<'a> Resolver<'a> { errors: Vec::new(), lambda_stack: Vec::new(), file, + in_contract, } } @@ -805,9 +817,16 @@ impl<'a> Resolver<'a> { } } + /// Override whether this name resolver is within a contract or not. + /// This will affect which types are allowed as parameters to methods as well + /// as which modifiers are allowed on a function. + pub(crate) fn set_in_contract(&mut self, in_contract: bool) { + self.in_contract = in_contract; + } + /// True if the 'pub' keyword is allowed on parameters in this function fn pub_allowed(&self, func: &NoirFunction) -> bool { - if self.in_contract() { + if self.in_contract { !func.def.is_unconstrained } else { func.name() == MAIN_FUNCTION @@ -815,7 +834,7 @@ impl<'a> Resolver<'a> { } fn is_entry_point_function(&self, func: &NoirFunction) -> bool { - if self.in_contract() { + if self.in_contract { func.attributes().is_contract_entry_point() } else { func.name() == MAIN_FUNCTION @@ -824,7 +843,7 @@ impl<'a> Resolver<'a> { /// True if the `distinct` keyword is allowed on a function's return type fn distinct_allowed(&self, func: &NoirFunction) -> bool { - if self.in_contract() { + if self.in_contract { // "open" and "unconstrained" functions are compiled to brillig and thus duplication of // witness indices in their abis is not a concern. !func.def.is_unconstrained && !func.def.is_open @@ -836,7 +855,7 @@ impl<'a> Resolver<'a> { fn handle_function_type(&mut self, function: &FuncId) { let function_type = self.interner.function_modifiers(function).contract_function_type; - if !self.in_contract() && function_type == Some(ContractFunctionType::Open) { + if !self.in_contract && function_type == Some(ContractFunctionType::Open) { let span = self.interner.function_ident(function).span(); self.errors.push(ResolverError::ContractFunctionTypeInNormalFunction { span }); self.interner.function_modifiers_mut(function).contract_function_type = None; @@ -844,7 +863,7 @@ impl<'a> Resolver<'a> { } fn handle_is_function_internal(&mut self, function: &FuncId) { - if !self.in_contract() { + if !self.in_contract { if self.interner.function_modifiers(function).is_internal == Some(true) { let span = self.interner.function_ident(function).span(); self.push_err(ResolverError::ContractFunctionInternalInNormalFunction { span }); @@ -1605,11 +1624,6 @@ impl<'a> Resolver<'a> { } } - fn in_contract(&self) -> bool { - let module_id = self.path_resolver.module_id(); - module_id.module(self.def_maps).is_contract - } - fn resolve_fmt_str_literal(&mut self, str: String, call_expr_span: Span) -> HirLiteral { let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}") .expect("ICE: an invalid regex pattern was used for checking format strings"); diff --git a/tooling/nargo_cli/tests/compile_success_contract/contract_with_impl/Nargo.toml b/tooling/nargo_cli/tests/compile_success_contract/contract_with_impl/Nargo.toml new file mode 100644 index 00000000000..99340cf80b5 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_success_contract/contract_with_impl/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "simple_contract" +type = "contract" +authors = [""] +compiler_version = "0.1" + +[dependencies] + diff --git a/tooling/nargo_cli/tests/compile_success_contract/contract_with_impl/src/main.nr b/tooling/nargo_cli/tests/compile_success_contract/contract_with_impl/src/main.nr new file mode 100644 index 00000000000..fa04e0d3e7b --- /dev/null +++ b/tooling/nargo_cli/tests/compile_success_contract/contract_with_impl/src/main.nr @@ -0,0 +1,8 @@ + +contract Foo { + struct T { x: [Field] } + + impl T { + fn t(self){} + } +}