Code review

crates/noirc_driver/src/lib.rs

@@ -205,8 +205,11 @@ impl Driver {
         let functions = try_btree_map(&contract.functions, |function_id| {
             let function_name = self.function_name(*function_id).to_owned();
             let function = self.compile_no_check(options, *function_id)?;
-            let func_type =
-                self.context.def_interner.function_meta(function_id).contract_visibility;
+            let func_meta = self.context.def_interner.function_meta(function_id);
+            let func_type = func_meta
+                .contract_visibility
+                .expect("Expected contract function to have a contract visibility");
+
             Ok((function_name, ContractFunction { func_type, function }))
         })?;
 

crates/noirc_frontend/src/ast/expression.rs

@@ -309,8 +309,14 @@ pub struct Lambda {
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct FunctionDefinition {
     pub name: Ident,
-    pub attribute: Option<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
-    pub contract_visibility: ContractVisibility,
+
+    // 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>,
+
     pub generics: UnresolvedGenerics,
     pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>,
     pub body: BlockExpression,
@@ -321,7 +327,7 @@ 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 `unconstrained` functions in regular Noir programs.
+/// - 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")]
@@ -334,7 +340,7 @@ pub enum ContractVisibility {
     Open,
     /// A function which is non-deterministic
     /// and does not require any constraints.
-    Unconstrained,
+    Unsafe,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]

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

@@ -246,7 +246,7 @@ 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 'unconstrained'".into(),
+                "Non-contract functions cannot be 'open' or 'unsafe'".into(),
                 span,
             ),
         }

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

@@ -644,14 +644,21 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    fn handle_contract_visibility(&mut self, func: &NoirFunction) -> ContractVisibility {
+    fn handle_contract_visibility(&mut self, func: &NoirFunction) -> Option<ContractVisibility> {
         let mut contract_visibility = func.def.contract_visibility;
-        if !self.in_contract() && contract_visibility != ContractVisibility::Secret {
-            contract_visibility = ContractVisibility::Secret;
+
+        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(),
             })
         }
+
         contract_visibility
     }
 
@@ -1268,7 +1275,7 @@ mod test {
         def_maps.insert(
             CrateId::dummy_id(),
             CrateDefMap {
-                root: LocalModuleId(arena::Index::from_raw_parts(0, 0)),
+                root: path_resolver.local_module_id(),
                 modules,
                 krate: CrateId::dummy_id(),
                 extern_prelude: HashMap::new(),

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

@@ -293,7 +293,7 @@ mod test {
         def_maps.insert(
             CrateId::dummy_id(),
             CrateDefMap {
-                root: LocalModuleId(arena::Index::from_raw_parts(0, 0)),
+                root: path_resolver.local_module_id(),
                 modules,
                 krate: CrateId::dummy_id(),
                 extern_prelude: HashMap::new(),

crates/noirc_frontend/src/hir_def/function.rs

@@ -123,7 +123,7 @@ 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: ContractVisibility,
+    pub contract_visibility: Option<ContractVisibility>,
 
     pub parameters: Parameters,
 

crates/noirc_frontend/src/lexer/token.rs

@@ -437,7 +437,7 @@ pub enum Keyword {
     String,
     Return,
     Struct,
-    Unconstrained,
+    Unsafe,
     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::Unconstrained => write!(f, "unconstrained"),
+            Keyword::Unsafe => write!(f, "unsafe"),
             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,
-            "unconstrained" => Keyword::Unconstrained,
+            "unsafe" => Keyword::Unsafe,
             "use" => Keyword::Use,
             "while" => Keyword::While,
 

crates/noirc_frontend/src/parser/parser.rs

@@ -183,13 +183,13 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
         )
 }
 
-/// contract_visibility: 'open' | 'unconstrained' | %empty
-fn contract_visibility() -> impl NoirParser<ContractVisibility> {
-    keyword(Keyword::Open).or(keyword(Keyword::Unconstrained)).or_not().map(|token| match token {
-        Some(Token::Keyword(Keyword::Open)) => ContractVisibility::Open,
-        Some(Token::Keyword(Keyword::Unconstrained)) => ContractVisibility::Unconstrained,
-        None => ContractVisibility::Secret,
-        _ => unreachable!("Only open and unconstrained keywords are parsed here"),
+/// 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"),
     })
 }