Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: Remove open keyword from Noir #4967

Merged
merged 13 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions avm-transpiler/src/transpile_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use regex::Regex;
use serde::{Deserialize, Serialize};

use acvm::acir::circuit::Circuit;
use noirc_driver::ContractFunctionType;

use crate::transpile::brillig_to_avm;
use crate::utils::extract_brillig_from_acir;
Expand Down Expand Up @@ -40,8 +39,8 @@ pub struct CompiledAcirContract {
#[derive(Debug, Serialize, Deserialize)]
pub struct AvmContractFunction {
pub name: String,
pub function_type: ContractFunctionType,
pub is_internal: bool,
pub is_unconstrained: bool,
pub custom_attributes: Vec<String>,
pub abi: serde_json::Value,
pub bytecode: String, // base64
pub debug_symbols: serde_json::Value,
Expand All @@ -52,8 +51,8 @@ pub struct AvmContractFunction {
#[derive(Debug, Serialize, Deserialize)]
pub struct AcirContractFunction {
pub name: String,
pub function_type: ContractFunctionType,
pub is_internal: bool,
pub is_unconstrained: bool,
pub custom_attributes: Vec<String>,
pub abi: serde_json::Value,
#[serde(
serialize_with = "Circuit::serialize_circuit_base64",
Expand Down Expand Up @@ -82,7 +81,9 @@ impl From<CompiledAcirContract> for TranspiledContract {
let re = Regex::new(r"avm_.*$").unwrap();
for function in contract.functions {
// TODO(4269): once functions are tagged for transpilation to AVM, check tag
if function.function_type == ContractFunctionType::Open
if function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that open is public vm.

.custom_attributes
.contains(&"aztec(public-vm)".to_string())
&& re.is_match(function.name.as_str())
{
info!(
Expand All @@ -99,8 +100,8 @@ impl From<CompiledAcirContract> for TranspiledContract {
// Push modified function entry to ABI
functions.push(AvmOrAcirContractFunction::Avm(AvmContractFunction {
name: function.name,
function_type: function.function_type,
is_internal: function.is_internal,
is_unconstrained: function.is_unconstrained,
custom_attributes: function.custom_attributes,
abi: function.abi,
bytecode: base64::prelude::BASE64_STANDARD.encode(avm_bytecode),
debug_symbols: function.debug_symbols,
Expand Down
5 changes: 1 addition & 4 deletions noir/noir-repo/aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@ fn transform_module(
transform_vm_function(func, storage_defined)
.map_err(|err| (err, crate_graph.root_file_id))?;
has_transformed_module = true;
}

// Add the storage struct to the beginning of the function if it is unconstrained in an aztec contract
if storage_defined && func.def.is_unconstrained {
} else if storage_defined && func.def.is_unconstrained {
transform_unconstrained(func);
has_transformed_module = true;
}
Expand Down
7 changes: 3 additions & 4 deletions noir/noir-repo/aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ pub fn transform_function(
if is_internal {
let is_internal_check = create_internal_check(func.name());
func.def.body.0.insert(0, is_internal_check);
func.def.is_internal = true;
}

// Add initialization check
Expand Down Expand Up @@ -87,10 +86,10 @@ pub fn transform_function(
func.def.return_visibility = Visibility::Public;

// Distinct return types are only required for private functions
// Public functions should have open auto-inferred
// Public functions should have unconstrained auto-inferred
match ty {
"Private" => func.def.return_distinctness = Distinctness::Distinct,
"Public" => func.def.is_open = true,
"Public" => func.def.is_unconstrained = true,
_ => (),
}

Expand All @@ -113,7 +112,7 @@ pub fn transform_vm_function(
func.def.body.0.insert(0, create_context);

// We want the function to be seen as a public function
func.def.is_open = true;
func.def.is_unconstrained = true;

// NOTE: the line below is a temporary hack to trigger external transpilation tools
// It will be removed once the transpiler is integrated into the Noir compiler
Expand Down
31 changes: 2 additions & 29 deletions noir/noir-repo/compiler/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,6 @@ use noirc_evaluator::errors::SsaReport;

use super::debug::DebugFile;

/// 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(Serialize, Deserialize, Debug, Copy, Clone, PartialEq, Eq)]
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(Clone, Debug, Serialize, Deserialize)]
pub struct CompiledContract {
pub noir_version: String,
Expand Down Expand Up @@ -55,9 +38,9 @@ pub struct CompiledContract {
pub struct ContractFunction {
pub name: String,

pub function_type: ContractFunctionType,
pub is_unconstrained: bool,

pub is_internal: bool,
pub custom_attributes: Vec<String>,

pub abi: Abi,

Expand All @@ -69,13 +52,3 @@ pub struct ContractFunction {

pub debug: DebugInfo,
}

impl ContractFunctionType {
pub(super) 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,
}
}
}
20 changes: 13 additions & 7 deletions noir/noir-repo/compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use noirc_frontend::hir::Context;
use noirc_frontend::macros_api::MacroProcessor;
use noirc_frontend::monomorphization::{monomorphize, monomorphize_debug, MonomorphizationError};
use noirc_frontend::node_interner::FuncId;
use noirc_frontend::token::SecondaryAttribute;
use std::path::Path;
use thiserror::Error;
use tracing::info;
Expand All @@ -30,7 +31,7 @@ mod stdlib;

use debug::filter_relevant_files;

pub use contract::{CompiledContract, ContractFunction, ContractFunctionType};
pub use contract::{CompiledContract, ContractFunction};
pub use debug::DebugFile;
pub use program::CompiledProgram;

Expand Down Expand Up @@ -404,19 +405,24 @@ fn compile_contract_inner(
};
warnings.extend(function.warnings);
let modifiers = context.def_interner.function_modifiers(&function_id);
let func_type = modifiers
.contract_function_type
.expect("Expected contract function to have a contract visibility");

let function_type = ContractFunctionType::new(func_type, modifiers.is_unconstrained);
let custom_attributes = modifiers
.attributes
.secondary
.iter()
.filter_map(
|attr| if let SecondaryAttribute::Custom(tag) = attr { Some(tag) } else { None },
)
.cloned()
.collect();

functions.push(ContractFunction {
name,
function_type,
is_internal: modifiers.is_internal.unwrap_or(false),
custom_attributes,
abi: function.abi,
bytecode: function.circuit,
debug: function.debug,
is_unconstrained: modifiers.is_unconstrained,
});
}

Expand Down
19 changes: 0 additions & 19 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,6 @@ pub struct FunctionDefinition {
// and `secondary` attributes (ones that do not change the function kind)
pub attributes: Attributes,

/// 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,

Expand Down Expand Up @@ -406,18 +401,6 @@ pub enum FunctionReturnType {
Ty(UnresolvedType),
}

/// Describes the types of smart contract functions that are allowed.
/// - All Noir programs in the non-contract context can be seen as `Secret`.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ContractFunctionType {
/// This function will be executed in a private
/// context.
Secret,
/// This function will be executed in a public
/// context.
Open,
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub enum ArrayLiteral {
Standard(Vec<Expression>),
Expand Down Expand Up @@ -674,8 +657,6 @@ impl FunctionDefinition {
FunctionDefinition {
name: name.clone(),
attributes: Attributes::empty(),
is_open: false,
is_internal: false,
is_unconstrained: false,
visibility: FunctionVisibility::Private,
generics: generics.clone(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,6 @@ impl<'a> ModCollector<'a> {
// TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
attributes: crate::token::Attributes::empty(),
is_unconstrained: false,
contract_function_type: None,
is_internal: None,
};

let location = Location::new(name.span(), self.file_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,10 @@ pub enum ResolverError {
IncorrectGenericCount { span: Span, item_name: String, actual: usize, expected: usize },
#[error("{0}")]
ParserError(Box<ParserError>),
#[error("Function is not defined in a contract yet sets its contract visibility")]
ContractFunctionTypeInNormalFunction { span: Span },
#[error("Cannot create a mutable reference to {variable}, it was declared to be immutable")]
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 },
#[error("Numeric constants should be printed without formatting braces")]
NumericConstantInFormatString { name: String, span: Span },
#[error("Closure environment must be a tuple or unit type")]
Expand Down Expand Up @@ -278,22 +274,12 @@ impl From<ResolverError> for Diagnostic {
)
}
ResolverError::ParserError(error) => (*error).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,
),
ResolverError::MutableReferenceToImmutableVariable { variable, span } => {
Diagnostic::simple_error(format!("Cannot mutably reference the immutable variable {variable}"), format!("{variable} is immutable"), span)
},
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,
),
ResolverError::NumericConstantInFormatString { name, span } => Diagnostic::simple_error(
format!("cannot find `{name}` in this scope "),
"Numeric constants should be printed without formatting braces".to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ use crate::{
StatementKind,
};
use crate::{
ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition,
FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param,
Path, PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind,
UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
ArrayLiteral, Distinctness, ForRange, FunctionDefinition, FunctionReturnType,
FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param, Path, PathKind,
Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind, UnaryOp,
UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression, Visibility, ERROR_IDENT,
};
use fm::FileId;
Expand Down Expand Up @@ -233,8 +233,6 @@ impl<'a> Resolver<'a> {
let def = FunctionDefinition {
name: name.clone(),
attributes: Attributes::empty(),
is_open: false,
is_internal: false,
is_unconstrained: false,
visibility: FunctionVisibility::Public, // Trait functions are always public
generics: Vec::new(), // self.generics should already be set
Expand Down Expand Up @@ -976,9 +974,6 @@ impl<'a> Resolver<'a> {

self.interner.push_definition_type(name_ident.id, typ.clone());

self.handle_function_type(&func_id);
self.handle_is_function_internal(&func_id);

FuncMeta {
name: name_ident,
kind: func.kind,
Expand Down Expand Up @@ -1018,34 +1013,14 @@ impl<'a> Resolver<'a> {
/// True if the `distinct` keyword is allowed on a function's return type
fn distinct_allowed(&self, func: &NoirFunction) -> bool {
if self.in_contract {
// "open" and "unconstrained" functions are compiled to brillig and thus duplication of
// "unconstrained" functions are compiled to brillig and thus duplication of
// witness indices in their abis is not a concern.
!func.def.is_unconstrained && !func.def.is_open
!func.def.is_unconstrained
} else {
func.name() == MAIN_FUNCTION
}
}

fn handle_function_type(&mut self, function: &FuncId) {
let function_type = self.interner.function_modifiers(function).contract_function_type;

if !self.in_contract && function_type == Some(ContractFunctionType::Open) {
let span = self.interner.function_ident(function).span();
self.errors.push(ResolverError::ContractFunctionTypeInNormalFunction { span });
self.interner.function_modifiers_mut(function).contract_function_type = None;
}
}

fn handle_is_function_internal(&mut self, function: &FuncId) {
if !self.in_contract {
if self.interner.function_modifiers(function).is_internal == Some(true) {
let span = self.interner.function_ident(function).span();
self.push_err(ResolverError::ContractFunctionInternalInNormalFunction { span });
}
self.interner.function_modifiers_mut(function).is_internal = None;
}
}

fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) {
if self.generics.is_empty() {
return;
Expand Down
3 changes: 0 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,6 @@ pub enum Keyword {
Let,
Mod,
Mut,
Open,
Pub,
Return,
ReturnData,
Expand Down Expand Up @@ -706,7 +705,6 @@ impl fmt::Display for Keyword {
Keyword::Let => write!(f, "let"),
Keyword::Mod => write!(f, "mod"),
Keyword::Mut => write!(f, "mut"),
Keyword::Open => write!(f, "open"),
Keyword::Pub => write!(f, "pub"),
Keyword::Return => write!(f, "return"),
Keyword::ReturnData => write!(f, "return_data"),
Expand Down Expand Up @@ -751,7 +749,6 @@ impl Keyword {
"let" => Keyword::Let,
"mod" => Keyword::Mod,
"mut" => Keyword::Mut,
"open" => Keyword::Open,
"pub" => Keyword::Pub,
"return" => Keyword::Return,
"return_data" => Keyword::ReturnData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use crate::{
},
node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitImplKind, TraitMethodId},
token::FunctionAttribute,
ContractFunctionType, FunctionKind, IntegerBitSize, Signedness, Type, TypeBinding,
TypeBindings, TypeVariable, TypeVariableKind, UnaryOp, Visibility,
FunctionKind, IntegerBitSize, Signedness, Type, TypeBinding, TypeBindings, TypeVariable,
TypeVariableKind, UnaryOp, Visibility,
};

use self::ast::{Definition, FuncId, Function, LocalId, Program};
Expand Down Expand Up @@ -310,8 +310,7 @@ impl<'interner> Monomorphizer<'interner> {
Type::TraitAsType(..) => &body_return_type,
_ => meta.return_type(),
});
let unconstrained = modifiers.is_unconstrained
|| matches!(modifiers.contract_function_type, Some(ContractFunctionType::Open));
let unconstrained = modifiers.is_unconstrained;

let parameters = self.parameters(&meta.parameters);
let body = self.expr(body_expr_id)?;
Expand Down
Loading
Loading