Skip to content

Commit

Permalink
Merge branch 'master' into sync-noir
Browse files Browse the repository at this point in the history
* master:
  chore!: Remove open keyword from Noir (#4967)
  • Loading branch information
TomAFrench committed Mar 11, 2024
2 parents c638c07 + 401557e commit 8c3a664
Show file tree
Hide file tree
Showing 26 changed files with 107 additions and 3,702 deletions.
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
.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 @@ -398,19 +399,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 @@ -276,22 +272,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 @@ -234,8 +234,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: generics.clone(),
Expand Down Expand Up @@ -973,9 +971,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);

let direct_generics = func.def.generics.iter();
let direct_generics = direct_generics
.filter_map(|generic| self.find_generic(&generic.0.contents))
Expand Down Expand Up @@ -1023,34 +1018,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 @@ -312,8 +312,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

0 comments on commit 8c3a664

Please sign in to comment.