diff --git a/compiler/noirc_frontend/src/elaborator/lints.rs b/compiler/noirc_frontend/src/elaborator/lints.rs index 78df10fa94c..8253921d305 100644 --- a/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/compiler/noirc_frontend/src/elaborator/lints.rs @@ -1,20 +1,19 @@ use crate::{ - ast::FunctionKind, + ast::{FunctionKind, Ident}, graph::CrateId, hir::{ resolution::errors::{PubPosition, ResolverError}, type_check::TypeCheckError, }, - hir_def::expr::HirIdent, + hir_def::{expr::HirIdent, function::FuncMeta}, macros_api::{ - HirExpression, HirLiteral, NodeInterner, NoirFunction, Signedness, UnaryOp, - UnresolvedTypeData, Visibility, + HirExpression, HirLiteral, NodeInterner, NoirFunction, Signedness, UnaryOp, Visibility, }, - node_interner::{DefinitionKind, ExprId, FuncId}, + node_interner::{DefinitionKind, ExprId, FuncId, FunctionModifiers}, Type, }; -use noirc_errors::Span; +use noirc_errors::{Span, Spanned}; pub(super) fn deprecated_function(interner: &NodeInterner, expr: ExprId) -> Option { let HirExpression::Ident(HirIdent { location, id, impl_kind: _ }, _) = @@ -39,16 +38,17 @@ pub(super) fn deprecated_function(interner: &NodeInterner, expr: ExprId) -> Opti /// Inline attributes are only relevant for constrained functions /// as all unconstrained functions are not inlined and so /// associated attributes are disallowed. -pub(super) fn inlining_attributes(func: &NoirFunction) -> Option { - if func.def.is_unconstrained { - let attributes = func.attributes().clone(); - - if attributes.is_no_predicates() { - Some(ResolverError::NoPredicatesAttributeOnUnconstrained { - ident: func.name_ident().clone(), - }) - } else if attributes.is_foldable() { - Some(ResolverError::FoldAttributeOnUnconstrained { ident: func.name_ident().clone() }) +pub(super) fn inlining_attributes( + func: &FuncMeta, + modifiers: &FunctionModifiers, +) -> Option { + if modifiers.is_unconstrained { + if modifiers.attributes.is_no_predicates() { + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::NoPredicatesAttributeOnUnconstrained { ident }) + } else if modifiers.attributes.is_foldable() { + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::FoldAttributeOnUnconstrained { ident }) } else { None } @@ -59,24 +59,30 @@ pub(super) fn inlining_attributes(func: &NoirFunction) -> Option /// Attempting to define new low level (`#[builtin]` or `#[foreign]`) functions outside of the stdlib is disallowed. pub(super) fn low_level_function_outside_stdlib( - func: &NoirFunction, + func: &FuncMeta, + modifiers: &FunctionModifiers, crate_id: CrateId, ) -> Option { let is_low_level_function = - func.attributes().function.as_ref().map_or(false, |func| func.is_low_level()); + modifiers.attributes.function.as_ref().map_or(false, |func| func.is_low_level()); if !crate_id.is_stdlib() && is_low_level_function { - Some(ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() }) + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::LowLevelFunctionOutsideOfStdlib { ident }) } else { None } } /// Oracle definitions (functions with the `#[oracle]` attribute) must be marked as unconstrained. -pub(super) fn oracle_not_marked_unconstrained(func: &NoirFunction) -> Option { +pub(super) fn oracle_not_marked_unconstrained( + func: &FuncMeta, + modifiers: &FunctionModifiers, +) -> Option { let is_oracle_function = - func.attributes().function.as_ref().map_or(false, |func| func.is_oracle()); - if is_oracle_function && !func.def.is_unconstrained { - Some(ResolverError::OracleMarkedAsConstrained { ident: func.name_ident().clone() }) + modifiers.attributes.function.as_ref().map_or(false, |func| func.is_oracle()); + if is_oracle_function && !modifiers.is_unconstrained { + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::OracleMarkedAsConstrained { ident }) } else { None } @@ -106,12 +112,13 @@ pub(super) fn oracle_called_from_constrained_function( } /// `pub` is required on return types for entry point functions -pub(super) fn missing_pub(func: &NoirFunction, is_entry_point: bool) -> Option { - if is_entry_point - && func.return_type().typ != UnresolvedTypeData::Unit - && func.def.return_visibility == Visibility::Private +pub(super) fn missing_pub(func: &FuncMeta, modifiers: &FunctionModifiers) -> Option { + if func.is_entry_point + && func.return_type() != &Type::Unit + && func.return_visibility == Visibility::Private { - Some(ResolverError::NecessaryPub { ident: func.name_ident().clone() }) + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::NecessaryPub { ident }) } else { None } @@ -119,11 +126,12 @@ pub(super) fn missing_pub(func: &NoirFunction, is_entry_point: bool) -> Option Option { - if !is_entry_point && func.kind == FunctionKind::Recursive { - Some(ResolverError::MisplacedRecursiveAttribute { ident: func.name_ident().clone() }) + if !func.is_entry_point && func.kind == FunctionKind::Recursive { + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::MisplacedRecursiveAttribute { ident }) } else { None } @@ -163,14 +171,13 @@ pub(super) fn unconstrained_function_return( /// /// Application of `pub` to other functions is not meaningful and is a mistake. pub(super) fn unnecessary_pub_return( - func: &NoirFunction, + func: &FuncMeta, + modifiers: &FunctionModifiers, is_entry_point: bool, ) -> Option { - if !is_entry_point && func.def.return_visibility == Visibility::Public { - Some(ResolverError::UnnecessaryPub { - ident: func.name_ident().clone(), - position: PubPosition::ReturnType, - }) + if !is_entry_point && func.return_visibility == Visibility::Public { + let ident = func_meta_name_ident(func, modifiers); + Some(ResolverError::UnnecessaryPub { ident, position: PubPosition::ReturnType }) } else { None } @@ -252,3 +259,7 @@ pub(crate) fn overflowing_int( errors } + +fn func_meta_name_ident(func: &FuncMeta, modifiers: &FunctionModifiers) -> Ident { + Ident(Spanned::from(func.name.location.span, modifiers.name.clone())) +} diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 161742029f6..6b23336b5ea 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -26,7 +26,8 @@ use crate::{ SecondaryAttribute, StructId, }, node_interner::{ - DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, + DefinitionKind, DependencyId, ExprId, FuncId, FunctionModifiers, GlobalId, ReferenceId, + TraitId, TypeAliasId, }, token::CustomAtrribute, Shared, Type, TypeVariable, @@ -417,6 +418,10 @@ impl<'context> Elaborator<'context> { self.trait_bounds = func_meta.trait_constraints.clone(); self.function_context.push(FunctionContext::default()); + let modifiers = self.interner.function_modifiers(&id).clone(); + + self.run_function_lints(&func_meta, &modifiers); + self.introduce_generics_into_scope(func_meta.all_generics.clone()); // The DefinitionIds for each parameter were already created in define_function_meta @@ -731,20 +736,6 @@ impl<'context> Elaborator<'context> { 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, in_contract)) - .map(Into::into) - }); - self.run_lint(|_| lints::oracle_not_marked_unconstrained(func).map(Into::into)); - self.run_lint(|elaborator| { - lints::low_level_function_outside_stdlib(func, elaborator.crate_id).map(Into::into) - }); - self.run_lint(|_| { - lints::recursive_non_entrypoint_function(func, is_entry_point).map(Into::into) - }); - // Both the #[fold] and #[no_predicates] alter a function's inline type and code generation in similar ways. // In certain cases such as type checking (for which the following flag will be used) both attributes // indicate we should code generate in the same way. Thus, we unify the attributes into one flag here. @@ -858,6 +849,23 @@ impl<'context> Elaborator<'context> { self.current_item = None; } + fn run_function_lints(&mut self, func: &FuncMeta, modifiers: &FunctionModifiers) { + self.run_lint(|_| lints::inlining_attributes(func, modifiers).map(Into::into)); + self.run_lint(|_| lints::missing_pub(func, modifiers).map(Into::into)); + self.run_lint(|_| { + let pub_allowed = func.is_entry_point || modifiers.attributes.is_foldable(); + lints::unnecessary_pub_return(func, modifiers, pub_allowed).map(Into::into) + }); + self.run_lint(|_| lints::oracle_not_marked_unconstrained(func, modifiers).map(Into::into)); + self.run_lint(|elaborator| { + lints::low_level_function_outside_stdlib(func, modifiers, elaborator.crate_id) + .map(Into::into) + }); + self.run_lint(|_| { + lints::recursive_non_entrypoint_function(func, modifiers).map(Into::into) + }); + } + /// Only sized types are valid to be used as main's parameters or the parameters to a contract /// function. If the given type is not sized (e.g. contains a slice or NamedGeneric type), an /// error is issued. diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index a1bb6d09cc9..a1bb99dbb46 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -6,7 +6,7 @@ use std::{ use acvm::{AcirField, FieldElement}; use builtin_helpers::{ block_expression_to_value, check_argument_count, check_function_not_yet_resolved, - check_one_argument, check_three_arguments, check_two_arguments, get_expr, get_field, + check_one_argument, check_three_arguments, check_two_arguments, get_bool, get_expr, get_field, get_format_string, get_function_def, get_module, get_quoted, get_slice, get_struct, get_trait_constraint, get_trait_def, get_trait_impl, get_tuple, get_type, get_typed_expr, get_u32, get_unresolved_type, hir_pattern_to_tokens, mutate_func_meta_type, parse, @@ -110,6 +110,9 @@ impl<'local, 'context> Interpreter<'local, 'context> { "function_def_set_return_type" => { function_def_set_return_type(self, arguments, location) } + "function_def_set_return_public" => { + function_def_set_return_public(self, arguments, location) + } "module_functions" => module_functions(self, arguments, location), "module_has_named_attribute" => module_has_named_attribute(self, arguments, location), "module_is_contract" => module_is_contract(self, arguments, location), @@ -1935,6 +1938,25 @@ fn function_def_set_return_type( Ok(Value::Unit) } +// fn set_return_public(self, public: bool) +fn function_def_set_return_public( + interpreter: &mut Interpreter, + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let (self_argument, public) = check_two_arguments(arguments, location)?; + + let func_id = get_function_def(self_argument)?; + check_function_not_yet_resolved(interpreter, func_id, location)?; + + let public = get_bool(public)?; + + let func_meta = interpreter.elaborator.interner.function_meta_mut(&func_id); + func_meta.return_visibility = if public { Visibility::Public } else { Visibility::Private }; + + Ok(Value::Unit) +} + // fn functions(self) -> [FunctionDefinition] fn module_functions( interpreter: &Interpreter, diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index ff3da6d253f..163b74efbe6 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -90,6 +90,13 @@ pub(crate) fn get_array( } } +pub(crate) fn get_bool((value, location): (Value, Location)) -> IResult { + match value { + Value::Bool(value) => Ok(value), + value => type_mismatch(value, Type::Bool, location), + } +} + pub(crate) fn get_slice( interner: &NodeInterner, (value, location): (Value, Location), diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 596d15176bc..2995e90ab01 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -26,7 +26,8 @@ use noirc_errors::Span; pub use parser::path::path_no_turbofish; pub use parser::traits::trait_bound; pub use parser::{ - block, expression, fresh_statement, lvalue, parse_program, parse_type, pattern, top_level_items, + block, expression, fresh_statement, lvalue, parse_program, parse_type, pattern, + top_level_items, visibility, }; #[derive(Debug, Clone)] diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 2bc7a88c6c5..1aee697aa88 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -28,7 +28,8 @@ use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable} use self::types::{generic_type_args, maybe_comp_time}; use attributes::{attributes, inner_attribute, validate_secondary_attributes}; pub use types::parse_type; -use visibility::visibility_modifier; +use visibility::item_visibility; +pub use visibility::visibility; use super::{ foldl_with_span, labels::ParsingRuleLabel, parameter_name_recovery, parameter_recovery, @@ -459,7 +460,7 @@ fn module_declaration() -> impl NoirParser { } fn use_statement() -> impl NoirParser { - visibility_modifier() + item_visibility() .then_ignore(keyword(Keyword::Use)) .then(use_tree()) .map(|(visibility, use_tree)| TopLevelStatement::Import(use_tree, visibility)) @@ -737,15 +738,6 @@ fn call_data() -> impl NoirParser { }) } -fn optional_visibility() -> impl NoirParser { - keyword(Keyword::Pub) - .map(|_| Visibility::Public) - .or(call_data()) - .or(keyword(Keyword::ReturnData).map(|_| Visibility::ReturnData)) - .or_not() - .map(|opt| opt.unwrap_or(Visibility::Private)) -} - pub fn expression() -> impl ExprParser { recursive(|expr| { expression_with_precedence( diff --git a/compiler/noirc_frontend/src/parser/parser/function.rs b/compiler/noirc_frontend/src/parser/parser/function.rs index 9328c882e54..05138bfffd9 100644 --- a/compiler/noirc_frontend/src/parser/parser/function.rs +++ b/compiler/noirc_frontend/src/parser/parser/function.rs @@ -1,10 +1,10 @@ use super::{ attributes::{attributes, validate_attributes}, - block, fresh_statement, ident, keyword, maybe_comp_time, nothing, optional_visibility, - parameter_name_recovery, parameter_recovery, parenthesized, parse_type, pattern, + block, fresh_statement, ident, keyword, maybe_comp_time, nothing, parameter_name_recovery, + parameter_recovery, parenthesized, parse_type, pattern, primitives::token_kind, self_parameter, - visibility::visibility_modifier, + visibility::{item_visibility, visibility}, where_clause, NoirParser, }; use crate::token::{Keyword, Token, TokenKind}; @@ -79,13 +79,9 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser impl NoirParser<(bool, ItemVisibility, bool)> { - keyword(Keyword::Unconstrained) - .or_not() - .then(visibility_modifier()) - .then(maybe_comp_time()) - .map(|((unconstrained, visibility), comptime)| { - (unconstrained.is_some(), visibility, comptime) - }) + keyword(Keyword::Unconstrained).or_not().then(item_visibility()).then(maybe_comp_time()).map( + |((unconstrained, visibility), comptime)| (unconstrained.is_some(), visibility, comptime), + ) } pub(super) fn numeric_generic() -> impl NoirParser { @@ -142,14 +138,12 @@ pub(super) fn generics() -> impl NoirParser { pub(super) fn function_return_type() -> impl NoirParser<(Visibility, FunctionReturnType)> { #[allow(deprecated)] - just(Token::Arrow) - .ignore_then(optional_visibility()) - .then(spanned(parse_type())) - .or_not() - .map_with_span(|ret, span| match ret { + just(Token::Arrow).ignore_then(visibility()).then(spanned(parse_type())).or_not().map_with_span( + |ret, span| match ret { Some((visibility, (ty, _))) => (visibility, FunctionReturnType::Ty(ty)), None => (Visibility::Private, FunctionReturnType::Default(span)), - }) + }, + ) } fn function_parameters<'a>(allow_self: bool) -> impl NoirParser> + 'a { @@ -158,7 +152,7 @@ fn function_parameters<'a>(allow_self: bool) -> impl NoirParser> + 'a let full_parameter = pattern() .recover_via(parameter_name_recovery()) .then_ignore(just(Token::Colon)) - .then(optional_visibility()) + .then(visibility()) .then(typ) .map_with_span(|((pattern, visibility), typ), span| Param { visibility, diff --git a/compiler/noirc_frontend/src/parser/parser/visibility.rs b/compiler/noirc_frontend/src/parser/parser/visibility.rs index d9c1abf2123..ea46becc932 100644 --- a/compiler/noirc_frontend/src/parser/parser/visibility.rs +++ b/compiler/noirc_frontend/src/parser/parser/visibility.rs @@ -4,15 +4,15 @@ use chumsky::{ }; use crate::{ - ast::ItemVisibility, + ast::{ItemVisibility, Visibility}, parser::NoirParser, token::{Keyword, Token}, }; -use super::primitives::keyword; +use super::{call_data, primitives::keyword}; /// visibility_modifier: 'pub(crate)'? 'pub'? '' -pub(crate) fn visibility_modifier() -> impl NoirParser { +pub(crate) fn item_visibility() -> impl NoirParser { let is_pub_crate = (keyword(Keyword::Pub) .then_ignore(just(Token::LeftParen)) .then_ignore(keyword(Keyword::Crate)) @@ -25,3 +25,12 @@ pub(crate) fn visibility_modifier() -> impl NoirParser { choice((is_pub_crate, is_pub, is_private)) } + +pub fn visibility() -> impl NoirParser { + keyword(Keyword::Pub) + .map(|_| Visibility::Public) + .or(call_data()) + .or(keyword(Keyword::ReturnData).map(|_| Visibility::ReturnData)) + .or_not() + .map(|opt| opt.unwrap_or(Visibility::Private)) +} diff --git a/docs/docs/noir/standard_library/meta/function_def.md b/docs/docs/noir/standard_library/meta/function_def.md index 3789fd82866..7c7531fb24a 100644 --- a/docs/docs/noir/standard_library/meta/function_def.md +++ b/docs/docs/noir/standard_library/meta/function_def.md @@ -73,3 +73,11 @@ each parameter pattern to be a syntactically valid parameter. Mutates the function's return type to a new type. This is only valid on functions in the current crate which have not yet been resolved. This means any functions called at compile-time are invalid targets for this method. + +### set_return_public + +#include_code set_return_public noir_stdlib/src/meta/function_def.nr rust + +Mutates the function's return visibility to public (if `true` is given) or private (if `false` is given). +This is only valid on functions in the current crate which have not yet been resolved. +This means any functions called at compile-time are invalid targets for this method. \ No newline at end of file diff --git a/noir_stdlib/src/meta/function_def.nr b/noir_stdlib/src/meta/function_def.nr index c1f2c026d57..0bff86ef102 100644 --- a/noir_stdlib/src/meta/function_def.nr +++ b/noir_stdlib/src/meta/function_def.nr @@ -43,4 +43,9 @@ impl FunctionDefinition { // docs:start:set_return_type fn set_return_type(self, return_type: Type) {} // docs:end:set_return_type + + #[builtin(function_def_set_return_public)] + // docs:start:set_return_public + fn set_return_public(self, public: bool) {} + // docs:end:set_return_public } diff --git a/test_programs/compile_success_empty/comptime_function_definition/src/main.nr b/test_programs/compile_success_empty/comptime_function_definition/src/main.nr index 8528bbce153..48651022b31 100644 --- a/test_programs/compile_success_empty/comptime_function_definition/src/main.nr +++ b/test_programs/compile_success_empty/comptime_function_definition/src/main.nr @@ -3,7 +3,7 @@ use std::meta::type_of; struct Foo { x: Field, field: Field } #[function_attr] -fn foo(w: i32, y: Field, Foo { x, field: some_field }: Foo, mut a: bool, (b, c): (i32, i32)) -> i32 { +pub fn foo(w: i32, y: Field, Foo { x, field: some_field }: Foo, mut a: bool, (b, c): (i32, i32)) -> i32 { let _ = (w, y, x, some_field, a, b, c); 1 } @@ -57,3 +57,15 @@ comptime fn mutate_add_one(f: FunctionDefinition) { fn main() { assert_eq(add_one(41), 42); } + +contract some_contract { + // No pub on the return type is an error + #[super::set_pub_return] + pub fn bar() -> Field { + 1 + } +} + +fn set_pub_return(f: FunctionDefinition) { + f.set_return_public(true); +}