From 7a854937ca5a300ae05f335612d2ff72ce88b4b1 Mon Sep 17 00:00:00 2001 From: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Date: Sat, 8 Jul 2023 11:55:03 +0200 Subject: [PATCH] feat: Adding internal keyword (#1873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add internal keyword * chore: remove 👺 * fix: comment cleanup * feat: add `is_internal` to abi * Update crates/noirc_frontend/src/hir/resolution/resolver.rs Co-authored-by: jfecher * fix: cleanup * fix: is_internal name * Update crates/noirc_driver/src/lib.rs Co-authored-by: jfecher --------- Co-authored-by: jfecher Co-authored-by: kevaundray --- crates/nargo/src/artifacts/contract.rs | 2 ++ crates/nargo/src/ops/preprocess.rs | 1 + .../contracts/src/main.nr | 2 ++ crates/noirc_driver/src/contract.rs | 2 ++ crates/noirc_driver/src/lib.rs | 1 + crates/noirc_frontend/src/ast/expression.rs | 2 ++ .../src/hir/resolution/errors.rs | 7 +++++++ .../src/hir/resolution/resolver.rs | 14 ++++++++++++++ .../noirc_frontend/src/hir/type_check/mod.rs | 1 + crates/noirc_frontend/src/hir_def/function.rs | 7 ++++++- crates/noirc_frontend/src/lexer/token.rs | 3 +++ crates/noirc_frontend/src/parser/parser.rs | 18 ++++++++++++------ 12 files changed, 53 insertions(+), 7 deletions(-) diff --git a/crates/nargo/src/artifacts/contract.rs b/crates/nargo/src/artifacts/contract.rs index 4174207692..a718eac979 100644 --- a/crates/nargo/src/artifacts/contract.rs +++ b/crates/nargo/src/artifacts/contract.rs @@ -28,6 +28,8 @@ pub struct PreprocessedContractFunction { pub function_type: ContractFunctionType, + pub is_internal: bool, + pub abi: Abi, #[serde( diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs index 90364bec60..b1613ea319 100644 --- a/crates/nargo/src/ops/preprocess.rs +++ b/crates/nargo/src/ops/preprocess.rs @@ -53,6 +53,7 @@ pub fn preprocess_contract_function( Ok(PreprocessedContractFunction { name: func.name, function_type: func.function_type, + is_internal: func.is_internal, abi: func.abi, bytecode: optimized_bytecode, diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/contracts/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/contracts/src/main.nr index 53e094eb4c..c8d70cc2d0 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/contracts/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/contracts/src/main.nr @@ -5,4 +5,6 @@ fn main(x : Field, y : pub Field) { contract Foo { fn double(x: Field) -> pub Field { x * 2 } fn triple(x: Field) -> pub Field { x * 3 } + internal fn quadruple(x: Field) -> pub Field { x * 4 } + open internal fn skibbidy(x: Field) -> pub Field { x * 5 } } diff --git a/crates/noirc_driver/src/contract.rs b/crates/noirc_driver/src/contract.rs index c0a5453494..a25d258c9b 100644 --- a/crates/noirc_driver/src/contract.rs +++ b/crates/noirc_driver/src/contract.rs @@ -41,6 +41,8 @@ pub struct ContractFunction { pub function_type: ContractFunctionType, + pub is_internal: bool, + pub abi: Abi, #[serde(serialize_with = "serialize_circuit", deserialize_with = "deserialize_circuit")] diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 9912c7f68e..7f092cdf70 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -305,6 +305,7 @@ fn compile_contract( functions.push(ContractFunction { name, function_type, + is_internal: func_meta.is_internal.unwrap_or(false), abi: function.abi, bytecode: function.circuit, }); diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index f3788919d8..88fff61728 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -328,6 +328,8 @@ pub struct FunctionDefinition { /// True if this function was defined with the 'open' keyword pub is_open: bool, + pub is_internal: bool, + /// True if this function was defined with the 'unconstrained' keyword pub is_unconstrained: bool, diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 80638897a5..296be28b5a 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -64,6 +64,8 @@ pub enum ResolverError { MutableReferenceToImmutableVariable { variable: String, span: Span }, #[error("Mutable references to array indices are unsupported")] MutableReferenceToArrayElement { span: Span }, + #[error("Function is not defined in a contract yet sets is_internal")] + ContractFunctionInternalInNormalFunction { span: Span }, } impl ResolverError { @@ -268,6 +270,11 @@ impl From for Diagnostic { ResolverError::MutableReferenceToArrayElement { span } => { Diagnostic::simple_error("Mutable references to array elements are currently unsupported".into(), "Try storing the element in a fresh variable first".into(), span) }, + ResolverError::ContractFunctionInternalInNormalFunction { span } => Diagnostic::simple_error( + "Only functions defined within contracts can set their functions to be internal".into(), + "Non-contract functions cannot be 'internal'".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 47a40d8c14..ea0f341e98 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -652,6 +652,7 @@ impl<'a> Resolver<'a> { kind: func.kind, attributes, contract_function_type: self.handle_function_type(func), + is_internal: self.handle_is_function_internal(func), is_unconstrained: func.def.is_unconstrained, location, typ, @@ -697,6 +698,19 @@ impl<'a> Resolver<'a> { } } + fn handle_is_function_internal(&mut self, func: &NoirFunction) -> Option { + if self.in_contract() { + Some(func.def.is_internal) + } else { + if func.def.is_internal { + self.push_err(ResolverError::ContractFunctionInternalInNormalFunction { + span: func.name_ident().span(), + }); + } + None + } + } + fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) { if self.generics.is_empty() { return; diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index fa47477a94..b8bb6c788e 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -232,6 +232,7 @@ mod test { attributes: None, location, contract_function_type: None, + is_internal: None, is_unconstrained: false, typ: Type::Function(vec![Type::field(None), Type::field(None)], Box::new(Type::Unit)), parameters: vec![ diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index 60cb8ea4f8..304772f1d0 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -121,10 +121,15 @@ pub struct FuncMeta { /// Attribute per function. pub attributes: Option, - /// This function's visibility in its contract. + /// This function's type in its contract. /// If this function is not in a contract, this is always 'Secret'. pub contract_function_type: Option, + /// This function's visibility. + /// If this function is internal can only be called by itself. + /// Will be None if not in contract. + pub is_internal: Option, + pub is_unconstrained: bool, pub parameters: Parameters, diff --git a/crates/noirc_frontend/src/lexer/token.rs b/crates/noirc_frontend/src/lexer/token.rs index 3eb5b49914..9dbf97f2d8 100644 --- a/crates/noirc_frontend/src/lexer/token.rs +++ b/crates/noirc_frontend/src/lexer/token.rs @@ -431,6 +431,7 @@ pub enum Keyword { Impl, If, In, + Internal, Let, Mod, Mut, @@ -466,6 +467,7 @@ impl fmt::Display for Keyword { Keyword::Impl => write!(f, "impl"), Keyword::If => write!(f, "if"), Keyword::In => write!(f, "in"), + Keyword::Internal => write!(f, "internal"), Keyword::Let => write!(f, "let"), Keyword::Mod => write!(f, "mod"), Keyword::Mut => write!(f, "mut"), @@ -504,6 +506,7 @@ impl Keyword { "impl" => Keyword::Impl, "if" => Keyword::If, "in" => Keyword::In, + "internal" => Keyword::Internal, "let" => Keyword::Let, "mod" => Keyword::Mod, "mut" => Keyword::Mut, diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 2303825dd0..8bc232cae3 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -162,7 +162,10 @@ fn function_definition(allow_self: bool) -> impl NoirParser { .map( |( ( - ((((attribute, (is_unconstrained, is_open)), name), generics), parameters), + ( + (((attribute, (is_unconstrained, is_open, is_internal)), name), generics), + parameters, + ), ((return_distinctness, return_visibility), return_type), ), body, @@ -172,6 +175,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser { name, attribute, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive is_open, + is_internal, is_unconstrained, generics, parameters, @@ -185,14 +189,16 @@ fn function_definition(allow_self: bool) -> impl NoirParser { ) } -/// function_modifiers: 'unconstrained' 'open' | 'unconstrained' | 'open' | %empty -/// -/// returns (is_unconstrained, is_open) for whether each keyword was present -fn function_modifiers() -> impl NoirParser<(bool, bool)> { +/// function_modifiers: 'open internal' | 'internal' | 'unconstrained' | 'open' | %empty +/// returns (is_unconstrained, is_open, is_internal) for whether each keyword was present +fn function_modifiers() -> impl NoirParser<(bool, bool, bool)> { keyword(Keyword::Unconstrained) .or_not() .then(keyword(Keyword::Open).or_not()) - .map(|(unconstrained, open)| (unconstrained.is_some(), open.is_some())) + .then(keyword(Keyword::Internal).or_not()) + .map(|((unconstrained, open), internal)| { + (unconstrained.is_some(), open.is_some(), internal.is_some()) + }) } /// non_empty_ident_list: ident ',' non_empty_ident_list