From 7632b890d8a6ab4b356539dbd94e662775cf9416 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Sep 2023 19:56:47 +0000 Subject: [PATCH 1/6] add contract library method attribute --- compiler/noirc_frontend/src/lexer/lexer.rs | 13 +++++++++++++ compiler/noirc_frontend/src/lexer/token.rs | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index f3fe0b6aef..5b6e010cc7 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -536,6 +536,19 @@ mod tests { &Token::Attribute(Attribute::Primary(PrimaryAttribute::Test(TestScope::None))) ); } + + #[test] + fn contract_library_method_attribute() { + let input = r#"#[contract_library_method]"#; + let mut lexer = Lexer::new(input); + + let token = lexer.next().unwrap().unwrap(); + assert_eq!( + token.token(), + &Token::Attribute(Attribute::Secondary(SecondaryAttribute::ContractLibraryMethod)) + ); + } + #[test] fn test_attribute_with_valid_scope() { let input = r#"#[test(should_fail)]"#; diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index adbf8f6575..e99dcb72fb 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -374,6 +374,16 @@ impl Attributes { Self { primary: None, secondary: Vec::new() } } + /// Returns true if one of the secondary attributes is `contract_library_method` + /// + /// This is useful for finding out if we should compile a contract method + /// as an entry point or not. + pub fn has_contract_library_method(&self) -> bool { + self.secondary + .iter() + .any(|attribute| attribute == &SecondaryAttribute::ContractLibraryMethod) + } + /// Returns note if a deprecated secondary attribute is found pub fn get_deprecated_note(&self) -> Option> { self.secondary.iter().find_map(|attr| match attr { @@ -454,6 +464,9 @@ impl Attribute { } // Secondary attributes ["deprecated"] => Attribute::Secondary(SecondaryAttribute::Deprecated(None)), + ["contract_library_method"] => { + Attribute::Secondary(SecondaryAttribute::ContractLibraryMethod) + } ["deprecated", name] => { if !name.starts_with('"') && !name.ends_with('"') { return Err(LexerErrorKind::MalformedFuncAttribute { @@ -527,6 +540,10 @@ impl fmt::Display for PrimaryAttribute { #[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] pub enum SecondaryAttribute { Deprecated(Option), + // This is an attribute to specify that a function + // is a helper method for a contract and should not be seen as + // the entry point. + ContractLibraryMethod, Custom(String), } @@ -538,6 +555,7 @@ impl fmt::Display for SecondaryAttribute { write!(f, r#"#[deprecated("{note}")]"#) } SecondaryAttribute::Custom(ref k) => write!(f, "#[{k}]"), + SecondaryAttribute::ContractLibraryMethod => write!(f, "#[contract_library_method]"), } } } @@ -559,6 +577,7 @@ impl AsRef for SecondaryAttribute { SecondaryAttribute::Deprecated(Some(string)) => string, SecondaryAttribute::Deprecated(None) => "", SecondaryAttribute::Custom(string) => string, + SecondaryAttribute::ContractLibraryMethod => "", } } } From 4652c921d26bd7b9528731ec1f2a7c7eb9b1cef8 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Sep 2023 19:57:12 +0000 Subject: [PATCH 2/6] add failing test --- .../non_entry_point_method/Nargo.toml | 7 +++++++ .../non_entry_point_method/src/main.nr | 6 ++++++ 2 files changed, 13 insertions(+) create mode 100644 tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/src/main.nr diff --git a/tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/Nargo.toml b/tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/Nargo.toml new file mode 100644 index 0000000000..c17e430c70 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "no_entry_points" +type = "contract" +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/src/main.nr b/tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/src/main.nr new file mode 100644 index 0000000000..5c7152029c --- /dev/null +++ b/tooling/nargo_cli/tests/compile_success_contract/non_entry_point_method/src/main.nr @@ -0,0 +1,6 @@ +contract Foo { + struct PlaceholderStruct{x : u32 } + + #[contract_library_method] + fn has_mut(_context : &mut PlaceholderStruct) {} +} \ No newline at end of file From 1d2239d19ccd55a8b65e3e994ace3db469f613ce Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Sep 2023 19:57:53 +0000 Subject: [PATCH 3/6] add method to return a functions attributes --- compiler/noirc_frontend/src/node_interner.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 5e4604d7bd..cffa5e92e3 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -19,6 +19,7 @@ use crate::hir_def::{ function::{FuncMeta, HirFunction}, stmt::HirStatement, }; +use crate::token::Attributes; use crate::{ Generics, Shared, TypeAliasType, TypeBinding, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind, @@ -547,6 +548,10 @@ impl NodeInterner { self.definition_name(name_id) } + pub fn function_attributes(&self, func_id: &FuncId) -> Attributes { + self.function_meta(func_id).attributes.clone() + } + /// Returns the interned statement corresponding to `stmt_id` pub fn statement(&self, stmt_id: &StmtId) -> HirStatement { let def = From a4b0e33bb8f1d55cd1e721281ef55d0106cd3d09 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Sep 2023 19:58:55 +0000 Subject: [PATCH 4/6] modify get_all_contracts to now return whether a function is a library method --- .../noirc_frontend/src/hir/def_map/mod.rs | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 2d5f7f3819..1a7c6674e0 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -155,13 +155,23 @@ impl CrateDefMap { /// Go through all modules in this crate, find all `contract ... { ... }` declarations, /// and collect them all into a Vec. - pub fn get_all_contracts(&self) -> Vec { + pub fn get_all_contracts(&self, interner: &NodeInterner) -> Vec { self.modules .iter() .filter_map(|(id, module)| { if module.is_contract { - let functions = + let function_ids: Vec = module.value_definitions().filter_map(|id| id.as_function()).collect(); + + let functions = function_ids + .into_iter() + .map(|id| { + let is_entry_point = + !interner.function_attributes(&id).has_contract_library_method(); + ContractFunctionMeta { function_id: id, is_entry_point } + }) + .collect(); + let name = self.get_module_path(id, module.parent); Some(Contract { name, location: module.location, functions }) } else { @@ -204,13 +214,25 @@ impl CrateDefMap { } } +/// Specifies a contract function and extra metadata that +/// one can use when processing a contract function. +/// +/// One of these is whether the contract function is an entry point. +/// The caller should only type-check these functions and not attempt +/// to create a circuit for them. +pub struct ContractFunctionMeta { + pub function_id: FuncId, + /// Indicates whether the function is an entry point + pub is_entry_point: bool, +} + /// A 'contract' in Noir source code with the given name and functions. /// This is not an AST node, it is just a convenient form to return for CrateDefMap::get_all_contracts. pub struct Contract { /// To keep `name` semi-unique, it is prefixed with the names of parent modules via CrateDefMap::get_module_path pub name: String, pub location: Location, - pub functions: Vec, + pub functions: Vec, } /// Given a FileId, fetch the File, from the FileManager and parse it's content From b228a4c9ec188fd84eabdaa0fac4c9b9e98b3991 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Sep 2023 19:59:17 +0000 Subject: [PATCH 5/6] refactor call to get_all_contracts --- compiler/noirc_frontend/src/hir/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index 1bdd3a62b7..eb67fbaa8a 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -168,7 +168,7 @@ impl Context { pub fn get_all_contracts(&self, crate_id: &CrateId) -> Vec { self.def_map(crate_id) .expect("The local crate should be analyzed already") - .get_all_contracts() + .get_all_contracts(&self.def_interner) } fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData { From 3c673f2c3b5bac7ffd679b0760a5fee787c7b8ba Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Sep 2023 20:00:12 +0000 Subject: [PATCH 6/6] modify driver to skip contract functions which are not entry points --- compiler/noirc_driver/src/lib.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 1b627adb3e..45b005f755 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -232,16 +232,28 @@ fn compile_contract_inner( ) -> Result { let mut functions = Vec::new(); let mut errors = Vec::new(); - for function_id in &contract.functions { - let name = context.function_name(function_id).to_owned(); - let function = match compile_no_check(context, options, *function_id) { + for contract_function in &contract.functions { + let function_id = contract_function.function_id; + let is_entry_point = contract_function.is_entry_point; + + let name = context.function_name(&function_id).to_owned(); + + // We assume that functions have already been type checked. + // This is the exact same assumption that compile_no_check makes. + // If it is not an entry-point point, we can then just skip the + // compilation step. It will also not be added to the ABI. + if !is_entry_point { + continue; + } + + let function = match compile_no_check(context, options, function_id) { Ok(function) => function, Err(new_error) => { errors.push(new_error); continue; } }; - let func_meta = context.def_interner.function_meta(function_id); + let func_meta = context.def_interner.function_meta(&function_id); let func_type = func_meta .contract_function_type .expect("Expected contract function to have a contract visibility");