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: Separate frontend Visibility and Distinctness from the ABI #2369

Merged
merged 1 commit into from
Aug 18, 2023
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
18 changes: 0 additions & 18 deletions crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,6 @@ pub enum AbiVisibility {
Private,
}

impl std::fmt::Display for AbiVisibility {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
AbiVisibility::Public => write!(f, "pub"),
AbiVisibility::Private => write!(f, "priv"),
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
/// Represents whether the return value should compromise of unique witness indices such that no
Expand All @@ -101,15 +92,6 @@ pub enum AbiDistinctness {
DuplicationAllowed,
}

impl std::fmt::Display for AbiDistinctness {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
AbiDistinctness::Distinct => write!(f, "distinct"),
AbiDistinctness::DuplicationAllowed => write!(f, "duplication-allowed"),
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Sign {
Expand Down
8 changes: 4 additions & 4 deletions crates/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use acvm::{
FieldElement,
};
use iter_extended::{try_vecmap, vecmap};
use noirc_abi::AbiDistinctness;
use noirc_frontend::Distinctness;

/// Context struct for the acir generation pass.
/// May be similar to the Evaluator struct in the current SSA IR.
Expand Down Expand Up @@ -112,14 +112,14 @@ impl Ssa {
pub(crate) fn into_acir(
self,
brillig: Brillig,
abi_distinctness: AbiDistinctness,
abi_distinctness: Distinctness,
last_array_uses: &HashMap<ValueId, InstructionId>,
) -> Result<GeneratedAcir, RuntimeError> {
let context = Context::new();
let mut generated_acir = context.convert_ssa(self, brillig, last_array_uses)?;

match abi_distinctness {
AbiDistinctness::Distinct => {
Distinctness::Distinct => {
// Create a witness for each return witness we have
// to guarantee that the return witnesses are distinct
let distinct_return_witness: Vec<_> = generated_acir
Expand All @@ -135,7 +135,7 @@ impl Ssa {
generated_acir.return_witnesses = distinct_return_witness;
Ok(generated_acir)
}
AbiDistinctness::DuplicationAllowed => Ok(generated_acir),
Distinctness::DuplicationAllowed => Ok(generated_acir),
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::fmt::Display;

use crate::token::{Attribute, Token};
use crate::{Ident, Path, Pattern, Recoverable, Statement, TraitConstraint, UnresolvedType};
use crate::{
Distinctness, Ident, Path, Pattern, Recoverable, Statement, TraitConstraint, UnresolvedType,
Visibility,
};
use acvm::FieldElement;
use iter_extended::vecmap;
use noirc_errors::{Span, Spanned};
Expand Down Expand Up @@ -360,13 +363,13 @@ pub struct FunctionDefinition {
pub is_unconstrained: bool,

pub generics: UnresolvedGenerics,
pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>,
pub parameters: Vec<(Pattern, UnresolvedType, Visibility)>,
pub body: BlockExpression,
pub span: Span,
pub where_clause: Vec<TraitConstraint>,
pub return_type: FunctionReturnType,
pub return_visibility: noirc_abi::AbiVisibility,
pub return_distinctness: noirc_abi::AbiDistinctness,
pub return_visibility: Visibility,
pub return_distinctness: Distinctness,
}

#[derive(Debug, PartialEq, Eq, Clone)]
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::fmt::Display;

use crate::{token::Attribute, FunctionReturnType, Ident, Pattern};
use crate::{token::Attribute, FunctionReturnType, Ident, Pattern, Visibility};

use super::{FunctionDefinition, UnresolvedType};

Expand Down Expand Up @@ -52,7 +52,7 @@ impl NoirFunction {
pub fn name_ident(&self) -> &Ident {
&self.def.name
}
pub fn parameters(&self) -> &Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)> {
pub fn parameters(&self) -> &Vec<(Pattern, UnresolvedType, Visibility)> {
&self.def.parameters
}
pub fn attribute(&self) -> Option<&Attribute> {
Expand Down
62 changes: 62 additions & 0 deletions crates/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod type_alias;
pub use expression::*;
pub use function::*;

use noirc_abi::AbiVisibility;
use noirc_errors::Span;
pub use statement::*;
pub use structure::*;
Expand Down Expand Up @@ -214,3 +215,64 @@ impl UnresolvedTypeExpression {
)
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
/// Represents whether the parameter is public or known only to the prover.
pub enum Visibility {
Public,
// Constants are not allowed in the ABI for main at the moment.
// Constant,
Private,
}

impl std::fmt::Display for Visibility {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Public => write!(f, "pub"),
Self::Private => write!(f, "priv"),
}
}
}

// TODO: Move this into noirc_abi when it depends upon noirc_frontend (instead of other way around)
impl From<Visibility> for AbiVisibility {
fn from(value: Visibility) -> Self {
match value {
Visibility::Public => AbiVisibility::Public,
Visibility::Private => AbiVisibility::Private,
}
}
}

// TODO: Move this into noirc_abi when it depends upon noirc_frontend (instead of other way around)
impl From<&Visibility> for AbiVisibility {
fn from(value: &Visibility) -> Self {
match value {
Visibility::Public => AbiVisibility::Public,
Visibility::Private => AbiVisibility::Private,
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
/// Represents whether the return value should compromise of unique witness indices such that no
/// index occurs within the program's abi more than once.
///
/// This is useful for application stacks that require an uniform abi across across multiple
/// circuits. When index duplication is allowed, the compiler may identify that a public input
/// reaches the output unaltered and is thus referenced directly, causing the input and output
/// witness indices to overlap. Similarly, repetitions of copied values in the output may be
/// optimized away.
pub enum Distinctness {
Distinct,
DuplicationAllowed,
}

impl std::fmt::Display for Distinctness {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Distinct => write!(f, "distinct"),
Self::DuplicationAllowed => write!(f, "duplication-allowed"),
}
}
}
15 changes: 7 additions & 8 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
Statement,
};
use crate::{
ArrayLiteral, ContractFunctionType, Generics, LValue, NoirStruct, NoirTypeAlias, Path, Pattern,
Shared, StructType, Trait, Type, TypeAliasType, TypeBinding, TypeVariable, UnaryOp,
UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT,
ArrayLiteral, ContractFunctionType, Distinctness, Generics, LValue, NoirStruct, NoirTypeAlias,
Path, Pattern, Shared, StructType, Trait, Type, TypeAliasType, TypeBinding, TypeVariable,
UnaryOp, UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, Visibility, ERROR_IDENT,
};
use fm::FileId;
use iter_extended::vecmap;
Expand Down Expand Up @@ -661,7 +661,7 @@
let mut parameter_types = vec![];

for (pattern, typ, visibility) in func.parameters().iter().cloned() {
if visibility == noirc_abi::AbiVisibility::Public && !self.pub_allowed(func) {
if visibility == Visibility::Public && !self.pub_allowed(func) {
self.push_err(ResolverError::UnnecessaryPub {
ident: func.name_ident().clone(),
position: PubPosition::Parameter,
Expand All @@ -679,8 +679,7 @@

self.declare_numeric_generics(&parameter_types, &return_type);

if !self.pub_allowed(func) && func.def.return_visibility == noirc_abi::AbiVisibility::Public
{
if !self.pub_allowed(func) && func.def.return_visibility == Visibility::Public {
self.push_err(ResolverError::UnnecessaryPub {
ident: func.name_ident().clone(),
position: PubPosition::ReturnType,
Expand All @@ -690,13 +689,13 @@
// 'pub_allowed' also implies 'pub' is required on return types
if self.pub_allowed(func)
&& return_type.as_ref() != &Type::Unit
&& func.def.return_visibility != noirc_abi::AbiVisibility::Public
&& func.def.return_visibility != Visibility::Public
{
self.push_err(ResolverError::NecessaryPub { ident: func.name_ident().clone() });
}

if !self.distinct_allowed(func)
&& func.def.return_distinctness != noirc_abi::AbiDistinctness::DuplicationAllowed
&& func.def.return_distinctness != Distinctness::DuplicationAllowed
{
self.push_err(ResolverError::DistinctNotAllowed { ident: func.name_ident().clone() });
}
Expand Down Expand Up @@ -1935,7 +1934,7 @@
println(f"I want to print {0}");

let new_val = 10;
println(f"randomstring{new_val}{new_val}");

Check warning on line 1937 in crates/noirc_frontend/src/hir/resolution/resolver.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (randomstring)
}
fn println<T>(x : T) -> T {
x
Expand Down
10 changes: 5 additions & 5 deletions crates/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ mod test {
},
parse_program, FunctionKind, Path,
};
use crate::{BinaryOpKind, FunctionReturnType};
use crate::{BinaryOpKind, Distinctness, FunctionReturnType, Visibility};

#[test]
fn basic_let() {
Expand Down Expand Up @@ -273,12 +273,12 @@ mod test {
Box::new(Type::Unit),
),
parameters: vec![
Param(Identifier(x), Type::FieldElement, noirc_abi::AbiVisibility::Private),
Param(Identifier(y), Type::FieldElement, noirc_abi::AbiVisibility::Private),
Param(Identifier(x), Type::FieldElement, Visibility::Private),
Param(Identifier(y), Type::FieldElement, Visibility::Private),
]
.into(),
return_visibility: noirc_abi::AbiVisibility::Private,
return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed,
return_visibility: Visibility::Private,
return_distinctness: Distinctness::DuplicationAllowed,
has_body: true,
return_type: FunctionReturnType::Default(Span::default()),
};
Expand Down
12 changes: 6 additions & 6 deletions crates/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use iter_extended::vecmap;
use noirc_abi::{AbiDistinctness, AbiParameter, AbiType, AbiVisibility};
use noirc_abi::{AbiParameter, AbiType};
use noirc_errors::{Location, Span};

use super::expr::{HirBlockExpression, HirExpression, HirIdent};
use super::stmt::HirPattern;
use crate::hir::def_map::ModuleId;
use crate::node_interner::{ExprId, NodeInterner};
use crate::{token::Attribute, FunctionKind};
use crate::{ContractFunctionType, FunctionReturnType, Type};
use crate::{ContractFunctionType, Distinctness, FunctionReturnType, Type, Visibility};

/// A Hir function is a block expression
/// with a list of statements
Expand Down Expand Up @@ -37,7 +37,7 @@ impl HirFunction {

/// An interned function parameter from a function definition
#[derive(Debug, Clone)]
pub struct Param(pub HirPattern, pub Type, pub noirc_abi::AbiVisibility);
pub struct Param(pub HirPattern, pub Type, pub Visibility);

/// Attempts to retrieve the name of this parameter. Returns None
/// if this parameter is a tuple or struct pattern.
Expand All @@ -60,7 +60,7 @@ impl Parameters {
.expect("Abi for tuple and struct parameters is unimplemented")
.to_owned();
let as_abi = param.1.as_abi_type();
AbiParameter { name: param_name, typ: as_abi, visibility: param.2 }
AbiParameter { name: param_name, typ: as_abi, visibility: param.2.into() }
})
}

Expand Down Expand Up @@ -139,9 +139,9 @@ pub struct FuncMeta {

pub return_type: FunctionReturnType,

pub return_visibility: AbiVisibility,
pub return_visibility: Visibility,

pub return_distinctness: AbiDistinctness,
pub return_distinctness: Distinctness,

/// The type of this function. Either a Type::Function
/// or a Type::Forall for generic functions.
Expand Down
6 changes: 3 additions & 3 deletions crates/noirc_frontend/src/monomorphization/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use noirc_abi::FunctionSignature;
use noirc_errors::Location;

use crate::{BinaryOpKind, Signedness};
use crate::{BinaryOpKind, Distinctness, Signedness};

/// The monomorphized AST is expression-based, all statements are also
/// folded into this expression enum. Compared to the HIR, the monomorphized
Expand Down Expand Up @@ -241,14 +241,14 @@
///
/// Note: this has no impact on monomorphization, and is simply attached here for ease of
/// forwarding to the next phase.
pub return_distinctness: noirc_abi::AbiDistinctness,
pub return_distinctness: Distinctness,
}

impl Program {
pub fn new(
functions: Vec<Function>,
main_function_signature: FunctionSignature,
return_distinctness: noirc_abi::AbiDistinctness,
return_distinctness: Distinctness,
) -> Program {
Program { functions, main_function_signature, return_distinctness }
}
Expand Down Expand Up @@ -325,7 +325,7 @@
Type::Bool => write!(f, "bool"),
Type::String(len) => write!(f, "str<{len}>"),
Type::FmtString(len, elements) => {
write!(f, "fmtstr<{len}, {elements}>")

Check warning on line 328 in crates/noirc_frontend/src/monomorphization/ast.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (fmtstr)
}
Type::Unit => write!(f, "()"),
Type::Tuple(elements) => {
Expand Down
5 changes: 3 additions & 2 deletions crates/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
node_interner::{self, DefinitionKind, NodeInterner, StmtId},
token::Attribute,
ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, TypeVariableKind,
Visibility,
};

use self::ast::{Definition, FuncId, Function, LocalId, Program};
Expand Down Expand Up @@ -1013,7 +1014,7 @@

// Manually convert to Parameters type so we can reuse the self.parameters method
let parameters = Parameters(vecmap(lambda.parameters, |(pattern, typ)| {
Param(pattern, typ, noirc_abi::AbiVisibility::Private)
Param(pattern, typ, Visibility::Private)
}));

let parameters = self.parameters(parameters);
Expand Down Expand Up @@ -1058,14 +1059,14 @@
// instead of detecting and extracting
// patterns in the resulting tree,
// which seems more fragile, we directly reuse the return parameters
// of this function in those cases

Check warning on line 1062 in crates/noirc_frontend/src/monomorphization/mod.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (callsites)
let ret_type = Self::convert_type(&lambda.return_type);
let lambda_name = "lambda";
let parameter_types = vecmap(&lambda.parameters, |(_, typ)| Self::convert_type(typ));

// Manually convert to Parameters type so we can reuse the self.parameters method
let parameters = Parameters(vecmap(lambda.parameters, |(pattern, typ)| {
Param(pattern, typ, noirc_abi::AbiVisibility::Private)
Param(pattern, typ, Visibility::Private)
}));

let mut converted_parameters = self.parameters(parameters);
Expand Down
Loading