chore: Separate frontend Visibility and Distinctness from the ABI (…

…#2369)

crates/noirc_abi/src/lib.rs

@@ -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
@@ -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 {

crates/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -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.
@@ -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
@@ -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),
         }
     }
 }

crates/noirc_frontend/src/ast/expression.rs

@@ -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};
@@ -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)]

crates/noirc_frontend/src/ast/function.rs

@@ -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};
 
@@ -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> {

crates/noirc_frontend/src/ast/mod.rs

@@ -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::*;
@@ -228,3 +229,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"),
+        }
+    }
+}

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

@@ -34,9 +34,9 @@ use crate::{
     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;
@@ -661,7 +661,7 @@ impl<'a> Resolver<'a> {
         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,
@@ -679,8 +679,7 @@ impl<'a> Resolver<'a> {
 
         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,
@@ -690,13 +689,13 @@ impl<'a> Resolver<'a> {
         // '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() });
         }

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

@@ -194,7 +194,7 @@ mod test {
         },
         parse_program, FunctionKind, Path,
     };
-    use crate::{BinaryOpKind, FunctionReturnType};
+    use crate::{BinaryOpKind, Distinctness, FunctionReturnType, Visibility};
 
     #[test]
     fn basic_let() {
@@ -268,12 +268,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()),
         };

crates/noirc_frontend/src/hir_def/function.rs

@@ -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
@@ -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.
@@ -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() }
         })
     }
 
@@ -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.

crates/noirc_frontend/src/monomorphization/ast.rs

@@ -3,7 +3,7 @@ use iter_extended::vecmap;
 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
@@ -241,14 +241,14 @@ pub struct Program {
     ///
     /// 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 }
     }

crates/noirc_frontend/src/monomorphization/mod.rs

@@ -23,6 +23,7 @@ use crate::{
     node_interner::{self, DefinitionKind, NodeInterner, StmtId},
     token::Attribute,
     ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, TypeVariableKind,
+    Visibility,
 };
 
 use self::ast::{Definition, FuncId, Function, LocalId, Program};
@@ -1025,7 +1026,7 @@ impl<'interner> Monomorphizer<'interner> {
 
         // 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);
@@ -1077,7 +1078,7 @@ impl<'interner> Monomorphizer<'interner> {
 
         // 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);