feat: Allow arbitrary noir functions to be unconstrained (#1044)

* Allow arbitrary noir functions to be unconstrained

* Fix test

* Remove all instances of old 'contract_visibility' naming

crates/noirc_driver/src/contract.rs

@@ -1,7 +1,5 @@
 use std::collections::BTreeMap;
 
-use noirc_frontend::ContractVisibility;
-
 use crate::CompiledProgram;
 
 /// Each function in the contract will be compiled
@@ -12,10 +10,28 @@ use crate::CompiledProgram;
 /// One of these being a function type.
 #[derive(serde::Serialize, serde::Deserialize)]
 pub struct ContractFunction {
-    pub func_type: ContractVisibility,
+    pub func_type: ContractFunctionType,
     pub function: CompiledProgram,
 }
 
+/// Describes the types of smart contract functions that are allowed.
+/// Unlike the similar enum in noirc_frontend, 'open' and 'unconstrained'
+/// are mutually exclusive here. In the case a function is both, 'unconstrained'
+/// takes precedence.
+#[derive(serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, Eq)]
+#[serde(rename_all = "snake_case")]
+pub enum ContractFunctionType {
+    /// This function will be executed in a private
+    /// context.
+    Secret,
+    /// This function will be executed in a public
+    /// context.
+    Open,
+    /// This function cannot constrain any values and can use nondeterministic features
+    /// like arrays of a dynamic size.
+    Unconstrained,
+}
+
 #[derive(serde::Serialize, serde::Deserialize)]
 pub struct CompiledContract {
     /// The name of the contract.
@@ -24,3 +40,13 @@ pub struct CompiledContract {
     /// stored in this `BTreeMap`.
     pub functions: BTreeMap<String, ContractFunction>,
 }
+
+impl ContractFunctionType {
+    pub fn new(kind: noirc_frontend::ContractFunctionType, is_unconstrained: bool) -> Self {
+        match (kind, is_unconstrained) {
+            (_, true) => Self::Unconstrained,
+            (noirc_frontend::ContractFunctionType::Secret, false) => Self::Secret,
+            (noirc_frontend::ContractFunctionType::Open, false) => Self::Open,
+        }
+    }
+}

crates/noirc_driver/src/lib.rs

@@ -4,7 +4,7 @@
 
 use acvm::Language;
 use clap::Args;
-use contract::ContractFunction;
+use contract::{ContractFunction, ContractFunctionType};
 use fm::FileType;
 use iter_extended::{try_btree_map, try_vecmap};
 use noirc_abi::FunctionSignature;
@@ -207,9 +207,11 @@ impl Driver {
             let function = self.compile_no_check(options, *function_id)?;
             let func_meta = self.context.def_interner.function_meta(function_id);
             let func_type = func_meta
-                .contract_visibility
+                .contract_function_type
                 .expect("Expected contract function to have a contract visibility");
 
+            let func_type = ContractFunctionType::new(func_type, func_meta.is_unconstrained);
+
             Ok((function_name, ContractFunction { func_type, function }))
         })?;
 

crates/noirc_frontend/src/ast/expression.rs

@@ -313,9 +313,11 @@ pub struct FunctionDefinition {
     // 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
     pub attribute: Option<Attribute>,
 
-    // The contract visibility is always None if the function is not in a contract.
-    // Otherwise, it is 'secret' (by default), 'open', or 'unsafe'.
-    pub contract_visibility: Option<ContractVisibility>,
+    /// True if this function was defined with the 'open' keyword
+    pub is_open: bool,
+
+    /// True if this function was defined with the 'unconstrained' keyword
+    pub is_unconstrained: bool,
 
     pub generics: UnresolvedGenerics,
     pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>,
@@ -327,20 +329,15 @@ pub struct FunctionDefinition {
 
 /// Describes the types of smart contract functions that are allowed.
 /// - All Noir programs in the non-contract context can be seen as `Secret`.
-/// - It may be possible to have `unsafe` functions in regular Noir programs.
-///   For now we leave it as a property of only contract functions.
 #[derive(serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, Eq)]
 #[serde(rename_all = "snake_case")]
-pub enum ContractVisibility {
+pub enum ContractFunctionType {
     /// This function will be executed in a private
     /// context.
     Secret,
     /// This function will be executed in a public
     /// context.
     Open,
-    /// A function which is non-deterministic
-    /// and does not require any constraints.
-    Unsafe,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]

crates/noirc_frontend/src/hir/resolution/errors.rs

@@ -62,7 +62,7 @@ pub enum ResolverError {
     #[error("{0}")]
     ParserError(ParserError),
     #[error("Function is not defined in a contract yet sets its contract visibility")]
-    ContractVisibilityInNormalFunction { span: Span },
+    ContractFunctionTypeInNormalFunction { span: Span },
 }
 
 impl ResolverError {
@@ -244,9 +244,9 @@ impl From<ResolverError> for Diagnostic {
                 )
             }
             ResolverError::ParserError(error) => error.into(),
-            ResolverError::ContractVisibilityInNormalFunction { span } => Diagnostic::simple_error(
-                "Only functions defined within contracts can set their contract visibility".into(),
-                "Non-contract functions cannot be 'open' or 'unsafe'".into(),
+            ResolverError::ContractFunctionTypeInNormalFunction { span } => Diagnostic::simple_error(
+                "Only functions defined within contracts can set their contract function type".into(),
+                "Non-contract functions cannot be 'open'".into(),
                 span,
             ),
         }

crates/noirc_frontend/src/hir/resolution/resolver.rs

@@ -33,7 +33,7 @@ use crate::{
     Statement,
 };
 use crate::{
-    ArrayLiteral, ContractVisibility, Generics, LValue, NoirStruct, Path, Pattern, Shared,
+    ArrayLiteral, ContractFunctionType, Generics, LValue, NoirStruct, Path, Pattern, Shared,
     StructType, Type, TypeBinding, TypeVariable, UnresolvedGenerics, UnresolvedType,
     UnresolvedTypeExpression, ERROR_IDENT,
 };
@@ -635,7 +635,8 @@ impl<'a> Resolver<'a> {
             name: name_ident,
             kind: func.kind,
             attributes,
-            contract_visibility: self.handle_contract_visibility(func),
+            contract_function_type: self.handle_function_type(func),
+            is_unconstrained: func.def.is_unconstrained,
             location,
             typ,
             parameters: parameters.into(),
@@ -644,22 +645,19 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    fn handle_contract_visibility(&mut self, func: &NoirFunction) -> Option<ContractVisibility> {
-        let mut contract_visibility = func.def.contract_visibility;
-
-        if self.in_contract() && contract_visibility.is_none() {
-            // The default visibility is 'secret' for contract functions without visibility modifiers
-            contract_visibility = Some(ContractVisibility::Secret);
-        }
-
-        if !self.in_contract() && contract_visibility.is_some() {
-            contract_visibility = None;
-            self.push_err(ResolverError::ContractVisibilityInNormalFunction {
-                span: func.name_ident().span(),
-            })
+    fn handle_function_type(&mut self, func: &NoirFunction) -> Option<ContractFunctionType> {
+        if func.def.is_open {
+            if self.in_contract() {
+                Some(ContractFunctionType::Open)
+            } else {
+                self.push_err(ResolverError::ContractFunctionTypeInNormalFunction {
+                    span: func.name_ident().span(),
+                });
+                None
+            }
+        } else {
+            Some(ContractFunctionType::Secret)
         }
-
-        contract_visibility
     }
 
     fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) {

crates/noirc_frontend/src/hir/type_check/mod.rs

@@ -158,7 +158,8 @@ mod test {
             kind: FunctionKind::Normal,
             attributes: None,
             location,
-            contract_visibility: None,
+            contract_function_type: None,
+            is_unconstrained: false,
             typ: Type::Function(vec![Type::field(None), Type::field(None)], Box::new(Type::Unit)),
             parameters: vec![
                 Param(Identifier(x), Type::field(None), noirc_abi::AbiVisibility::Private),

crates/noirc_frontend/src/hir_def/function.rs

@@ -6,7 +6,7 @@ use super::expr::{HirBlockExpression, HirExpression, HirIdent};
 use super::stmt::HirPattern;
 use crate::node_interner::{ExprId, NodeInterner};
 use crate::{token::Attribute, FunctionKind};
-use crate::{ContractVisibility, Type};
+use crate::{ContractFunctionType, Type};
 
 /// A Hir function is a block expression
 /// with a list of statements
@@ -123,7 +123,9 @@ pub struct FuncMeta {
 
     /// This function's visibility in its contract.
     /// If this function is not in a contract, this is always 'Secret'.
-    pub contract_visibility: Option<ContractVisibility>,
+    pub contract_function_type: Option<ContractFunctionType>,
+
+    pub is_unconstrained: bool,
 
     pub parameters: Parameters,
 

crates/noirc_frontend/src/lexer/token.rs

@@ -437,7 +437,7 @@ pub enum Keyword {
     String,
     Return,
     Struct,
-    Unsafe,
+    Unconstrained,
     Use,
     While,
 }
@@ -469,7 +469,7 @@ impl fmt::Display for Keyword {
             Keyword::String => write!(f, "str"),
             Keyword::Return => write!(f, "return"),
             Keyword::Struct => write!(f, "struct"),
-            Keyword::Unsafe => write!(f, "unsafe"),
+            Keyword::Unconstrained => write!(f, "unconstrained"),
             Keyword::Use => write!(f, "use"),
             Keyword::While => write!(f, "while"),
         }
@@ -504,7 +504,7 @@ impl Keyword {
             "str" => Keyword::String,
             "return" => Keyword::Return,
             "struct" => Keyword::Struct,
-            "unsafe" => Keyword::Unsafe,
+            "unconstrained" => Keyword::Unconstrained,
             "use" => Keyword::Use,
             "while" => Keyword::While,
 

crates/noirc_frontend/src/parser/parser.rs

@@ -33,10 +33,9 @@ use crate::lexer::Lexer;
 use crate::parser::{force, ignore_then_commit, statement_recovery};
 use crate::token::{Attribute, Keyword, Token, TokenKind};
 use crate::{
-    BinaryOp, BinaryOpKind, BlockExpression, CompTime, ConstrainStatement, ContractVisibility,
-    FunctionDefinition, Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda,
-    NoirFunction, NoirImpl, NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp,
-    UnresolvedTypeExpression,
+    BinaryOp, BinaryOpKind, BlockExpression, CompTime, ConstrainStatement, FunctionDefinition,
+    Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda, NoirFunction, NoirImpl,
+    NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, UnresolvedTypeExpression,
 };
 
 use chumsky::prelude::*;
@@ -147,12 +146,12 @@ fn contract(module_parser: impl NoirParser<ParsedModule>) -> impl NoirParser<Top
         })
 }
 
-/// function_definition: attribute contract_visibility 'fn' ident generics '(' function_parameters ')' function_return_type block
-///                      contract_visibility 'fn' ident generics '(' function_parameters ')' function_return_type block
+/// function_definition: attribute function_modifiers 'fn' ident generics '(' function_parameters ')' function_return_type block
+///                      function_modifiers 'fn' ident generics '(' function_parameters ')' function_return_type block
 fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
     attribute()
         .or_not()
-        .then(contract_visibility())
+        .then(function_modifiers())
         .then_ignore(keyword(Keyword::Fn))
         .then(ident())
         .then(generics())
@@ -162,7 +161,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
         .map(
             |(
                 (
-                    ((((attribute, contract_visibility), name), generics), parameters),
+                    ((((attribute, (is_unconstrained, is_open)), name), generics), parameters),
                     (return_visibility, return_type),
                 ),
                 body,
@@ -171,7 +170,8 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
                     span: name.0.span(),
                     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
-                    contract_visibility,
+                    is_open,
+                    is_unconstrained,
                     generics,
                     parameters,
                     body,
@@ -183,14 +183,14 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
         )
 }
 
-/// contract_visibility: 'open' | 'unsafe' | %empty
-fn contract_visibility() -> impl NoirParser<Option<ContractVisibility>> {
-    keyword(Keyword::Open).or(keyword(Keyword::Unsafe)).or_not().map(|token| match token {
-        Some(Token::Keyword(Keyword::Open)) => Some(ContractVisibility::Open),
-        Some(Token::Keyword(Keyword::Unsafe)) => Some(ContractVisibility::Unsafe),
-        None => None,
-        _ => unreachable!("Only open and unsafe keywords are parsed here"),
-    })
+/// 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)> {
+    keyword(Keyword::Unconstrained)
+        .or_not()
+        .then(keyword(Keyword::Open).or_not())
+        .map(|(unconstrained, open)| (unconstrained.is_some(), open.is_some()))
 }
 
 /// non_empty_ident_list: ident ',' non_empty_ident_list