From a58f6c6a2df093a0d3c812feed38774082fc2c3b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 28 Mar 2023 14:33:56 +0100 Subject: [PATCH 1/3] Allow secret functions to use public parameters --- .../noirc_frontend/src/hir/resolution/resolver.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index b33230a6f55..9fb5cd06459 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -596,7 +596,7 @@ impl<'a> Resolver<'a> { let mut parameter_types = vec![]; for (pattern, typ, visibility) in func.parameters().iter().cloned() { - if func.name() != "main" && visibility == noirc_abi::AbiVisibility::Public { + if visibility == noirc_abi::AbiVisibility::Public && !self.pub_allowed(func) { self.push_err(ResolverError::UnnecessaryPub { ident: func.name_ident().clone() }) } @@ -610,9 +610,7 @@ impl<'a> Resolver<'a> { self.declare_numeric_generics(¶meter_types, &return_type); - if func.name() == "main" - && *return_type != Type::Unit - && func.def.return_visibility != noirc_abi::AbiVisibility::Public + if func.def.return_visibility != noirc_abi::AbiVisibility::Public && self.pub_allowed(func) { self.push_err(ResolverError::NecessaryPub { ident: func.name_ident().clone() }) } @@ -645,6 +643,15 @@ impl<'a> Resolver<'a> { } } + /// True if the 'pub' keyword is allowed on parameters in this function + fn pub_allowed(&self, func: &NoirFunction) -> bool { + if self.in_contract() { + !func.def.is_unconstrained && !func.def.is_open + } else { + func.name() == "main" + } + } + fn handle_function_type(&mut self, func: &NoirFunction) -> Option { if func.def.is_open { if self.in_contract() { From 6ebd7fb5a2b7c9749e18791e38da14da6140c1cc Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 28 Mar 2023 15:53:56 +0100 Subject: [PATCH 2/3] Add carve-out for the Unit type for pub return values --- crates/noirc_frontend/src/hir/resolution/resolver.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 9fb5cd06459..a6d2b783298 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -610,7 +610,10 @@ impl<'a> Resolver<'a> { self.declare_numeric_generics(¶meter_types, &return_type); - if func.def.return_visibility != noirc_abi::AbiVisibility::Public && self.pub_allowed(func) + // 'pub_allowed' also implies 'pub' is required on return types + if self.pub_allowed(func) + && return_type.as_ref() != &Type::Unit + && func.def.return_visibility != noirc_abi::AbiVisibility::Public { self.push_err(ResolverError::NecessaryPub { ident: func.name_ident().clone() }) } From ea94548217069863574ab026525aba7985b34e1b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 28 Mar 2023 16:37:03 +0100 Subject: [PATCH 3/3] Fix contract test --- crates/nargo/tests/test_data/contracts/src/main.nr | 4 ++-- crates/noirc_frontend/src/hir/resolution/errors.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/nargo/tests/test_data/contracts/src/main.nr b/crates/nargo/tests/test_data/contracts/src/main.nr index 98233a34922..f236186d426 100644 --- a/crates/nargo/tests/test_data/contracts/src/main.nr +++ b/crates/nargo/tests/test_data/contracts/src/main.nr @@ -3,6 +3,6 @@ fn main(x : Field, y : pub Field) { } contract Foo { - fn double(x: Field) -> Field { x * 2 } - fn triple(x: Field) -> Field { x * 3 } + fn double(x: Field) -> pub Field { x * 2 } + fn triple(x: Field) -> pub Field { x * 3 } } diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 706e2604f3d..3ce7d9d0249 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -166,7 +166,7 @@ impl From for Diagnostic { ident.0.span(), ); - diag.add_note("The `pub` keyword only has effects on arguments to the main function of a program. Thus, adding it to other function parameters can be deceiving and should be removed".to_owned()); + diag.add_note("The `pub` keyword only has effects on arguments to the entry-point function of a program. Thus, adding it to other function parameters can be deceiving and should be removed".to_owned()); diag } ResolverError::NecessaryPub { ident } => { @@ -178,7 +178,7 @@ impl From for Diagnostic { ident.0.span(), ); - diag.add_note("The `pub` keyword is mandatory for the main function return type because the verifier cannot retrieve private witness and thus the function will not be able to return a 'priv' value".to_owned()); + diag.add_note("The `pub` keyword is mandatory for the entry-point function return type because the verifier cannot retrieve private witness and thus the function will not be able to return a 'priv' value".to_owned()); diag } ResolverError::ExpectedComptimeVariable { name, span } => Diagnostic::simple_error(