diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index dbd935dcde0..191128b9407 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -85,6 +85,30 @@ impl std::fmt::Display for AbiVisibility { } } +#[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 +/// 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 AbiDistinctness { + Distinct, + 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 { diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 8b3cbb009a9..64a02061b0f 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -54,6 +54,9 @@ pub struct Evaluator { // and increasing as for `public_parameters`. We then use a `Vec` rather // than a `BTreeSet` to preserve this order for the ABI. return_values: Vec, + // If true, indicates that the resulting ACIR should enforce that all inputs and outputs are + // comprised of unique witness indices by having extra constraints if necessary. + return_is_distinct: bool, opcodes: Vec, } @@ -102,6 +105,11 @@ pub fn create_circuit( } impl Evaluator { + // Returns true if the `witness_index` appears in the program's input parameters. + fn is_abi_input(&self, witness_index: Witness) -> bool { + witness_index.as_usize() <= self.num_witnesses_abi_len + } + // Returns true if the `witness_index` // was created in the ABI as a private input. // @@ -111,11 +119,17 @@ impl Evaluator { // If the `witness_index` is more than the `num_witnesses_abi_len` // then it was created after the ABI was processed and is therefore // an intermediate variable. - let is_intermediate_variable = witness_index.as_usize() > self.num_witnesses_abi_len; let is_public_input = self.public_parameters.contains(&witness_index); - !is_intermediate_variable && !is_public_input + self.is_abi_input(witness_index) && !is_public_input + } + + // True if the main function return has the `distinct` keyword and this particular witness + // index has already occurred elsewhere in the abi's inputs and outputs. + fn should_proxy_witness_for_abi_output(&self, witness_index: Witness) -> bool { + self.return_is_distinct + && (self.is_abi_input(witness_index) || self.return_values.contains(&witness_index)) } // Creates a new Witness index @@ -139,6 +153,8 @@ impl Evaluator { enable_logging: bool, show_output: bool, ) -> Result<(), RuntimeError> { + self.return_is_distinct = + program.return_distinctness == noirc_abi::AbiDistinctness::Distinct; let mut ir_gen = IrGenerator::new(program); self.parse_abi_alt(&mut ir_gen); diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs index 3269af06d16..6aaa3b2fbbd 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs @@ -1,3 +1,5 @@ +use acvm::acir::native_types::Expression; + use crate::{ errors::RuntimeErrorKind, ssa::{ @@ -46,7 +48,15 @@ pub(crate) fn evaluate( "we do not allow private ABI inputs to be returned as public outputs", ))); } - evaluator.return_values.push(witness); + // Check if the outputted witness needs separating from an existing occurrence in the + // abi. This behavior stems from usage of the `distinct` keyword. + let return_witness = if evaluator.should_proxy_witness_for_abi_output(witness) { + let proxy_constraint = Expression::from(witness); + evaluator.create_intermediate_variable(proxy_constraint) + } else { + witness + }; + evaluator.return_values.push(return_witness); } } diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index ac6161ddac1..9be6f715a14 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -325,6 +325,7 @@ pub struct FunctionDefinition { pub span: Span, pub return_type: UnresolvedType, pub return_visibility: noirc_abi::AbiVisibility, + pub return_distinctness: noirc_abi::AbiDistinctness, } /// Describes the types of smart contract functions that are allowed. diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index 25e0488a7b6..fdaf2dd3acc 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -18,6 +18,9 @@ pub use module_data::*; mod namespace; pub use namespace::*; +/// The name that is used for a non-contract program's entry-point function. +pub const MAIN_FUNCTION: &str = "main"; + // XXX: Ultimately, we want to constrain an index to be of a certain type just like in RA /// Lets first check if this is offered by any external crate /// XXX: RA has made this a crate on crates.io @@ -104,8 +107,6 @@ impl CrateDefMap { /// Find the main function for this crate pub fn main_function(&self) -> Option { - const MAIN_FUNCTION: &str = "main"; - let root_module = &self.modules()[self.root.0]; // This function accepts an Ident, so we attach a dummy span to diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 9406474a226..c57e4c890d2 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -32,6 +32,8 @@ pub enum ResolverError { UnnecessaryPub { ident: Ident }, #[error("Required 'pub', main function must return public value")] NecessaryPub { ident: Ident }, + #[error("'distinct' keyword can only be used with main method")] + DistinctNotAllowed { ident: Ident }, #[error("Expected const value where non-constant value was used")] ExpectedComptimeVariable { name: String, span: Span }, #[error("Missing expression for declared constant")] @@ -176,6 +178,18 @@ impl From for Diagnostic { diag.add_note("The `pub` keyword is mandatory for the entry-point function return type because the verifier cannot retrieve private witness and thus the function will not be able to return a 'priv' value".to_owned()); diag } + ResolverError::DistinctNotAllowed { ident } => { + let name = &ident.0.contents; + + let mut diag = Diagnostic::simple_error( + format!("Invalid `distinct` keyword on return type of function {name}"), + "Invalid distinct on return type".to_string(), + ident.0.span(), + ); + + diag.add_note("The `distinct` keyword is only valid when used on the main function of a program, as its only purpose is to ensure that all witness indices that occur in the abi are unique".to_owned()); + diag + } ResolverError::ExpectedComptimeVariable { name, span } => Diagnostic::simple_error( format!("expected constant variable where non-constant variable {name} was used"), "expected const variable".to_string(), diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index cfb354498ab..98cf5993edf 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -22,7 +22,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::rc::Rc; use crate::graph::CrateId; -use crate::hir::def_map::{ModuleDefId, TryFromModuleDefId}; +use crate::hir::def_map::{ModuleDefId, TryFromModuleDefId, MAIN_FUNCTION}; use crate::hir_def::stmt::{HirAssignStatement, HirLValue, HirPattern}; use crate::node_interner::{ DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, @@ -637,6 +637,12 @@ impl<'a> Resolver<'a> { self.push_err(ResolverError::NecessaryPub { ident: func.name_ident().clone() }); } + if !self.distinct_allowed(func) + && func.def.return_distinctness != noirc_abi::AbiDistinctness::DuplicationAllowed + { + self.push_err(ResolverError::DistinctNotAllowed { ident: func.name_ident().clone() }); + } + if attributes == Some(Attribute::Test) && !parameters.is_empty() { self.push_err(ResolverError::TestFunctionHasParameters { span: func.name_ident().span(), @@ -661,6 +667,7 @@ impl<'a> Resolver<'a> { typ, parameters: parameters.into(), return_visibility: func.def.return_visibility, + return_distinctness: func.def.return_distinctness, has_body: !func.def.body.is_empty(), } } @@ -670,7 +677,18 @@ impl<'a> Resolver<'a> { if self.in_contract() { !func.def.is_unconstrained && !func.def.is_open } else { - func.name() == "main" + func.name() == MAIN_FUNCTION + } + } + + /// 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 + // witness indices in their abis is not a concern. + !func.def.is_unconstrained && !func.def.is_open + } else { + func.name() == MAIN_FUNCTION } } diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index 97b1c71a0bc..5ebac6de9a3 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -219,6 +219,7 @@ mod test { ] .into(), return_visibility: noirc_abi::AbiVisibility::Private, + return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed, has_body: true, }; interner.push_fn_meta(func_meta, func_id); diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index a9fafffe159..1f7399e5547 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -1,5 +1,5 @@ use iter_extended::vecmap; -use noirc_abi::{AbiParameter, AbiType, AbiVisibility}; +use noirc_abi::{AbiDistinctness, AbiParameter, AbiType, AbiVisibility}; use noirc_errors::{Location, Span}; use super::expr::{HirBlockExpression, HirExpression, HirIdent}; @@ -131,6 +131,8 @@ pub struct FuncMeta { pub return_visibility: AbiVisibility, + pub return_distinctness: AbiDistinctness, + /// The type of this function. Either a Type::Function /// or a Type::Forall for generic functions. pub typ: Type, diff --git a/crates/noirc_frontend/src/lexer/token.rs b/crates/noirc_frontend/src/lexer/token.rs index 0df1fc39938..6b021a3dcbb 100644 --- a/crates/noirc_frontend/src/lexer/token.rs +++ b/crates/noirc_frontend/src/lexer/token.rs @@ -421,6 +421,7 @@ pub enum Keyword { Contract, Crate, Dep, + Distinct, Else, Field, Fn, @@ -454,6 +455,7 @@ impl fmt::Display for Keyword { Keyword::Contract => write!(f, "contract"), Keyword::Crate => write!(f, "crate"), Keyword::Dep => write!(f, "dep"), + Keyword::Distinct => write!(f, "distinct"), Keyword::Else => write!(f, "else"), Keyword::Field => write!(f, "Field"), Keyword::Fn => write!(f, "fn"), @@ -490,6 +492,7 @@ impl Keyword { "contract" => Keyword::Contract, "crate" => Keyword::Crate, "dep" => Keyword::Dep, + "distinct" => Keyword::Distinct, "else" => Keyword::Else, "Field" => Keyword::Field, "fn" => Keyword::Fn, diff --git a/crates/noirc_frontend/src/monomorphization/ast.rs b/crates/noirc_frontend/src/monomorphization/ast.rs index e4339c8e367..04aec9a6726 100644 --- a/crates/noirc_frontend/src/monomorphization/ast.rs +++ b/crates/noirc_frontend/src/monomorphization/ast.rs @@ -221,11 +221,20 @@ impl Type { pub struct Program { pub functions: Vec, pub main_function_signature: FunctionSignature, + /// Indicates whether witness indices are allowed to reoccur in the ABI of the resulting ACIR. + /// + /// 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, } impl Program { - pub fn new(functions: Vec, main_function_signature: FunctionSignature) -> Program { - Program { functions, main_function_signature } + pub fn new( + functions: Vec, + main_function_signature: FunctionSignature, + return_distinctness: noirc_abi::AbiDistinctness, + ) -> Program { + Program { functions, main_function_signature, return_distinctness } } pub fn main(&self) -> &Function { diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index bfce292d2eb..79c9bab7d8a 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -17,7 +17,7 @@ use std::collections::{BTreeMap, HashMap, VecDeque}; use crate::{ hir_def::{ expr::*, - function::{Param, Parameters}, + function::{FuncMeta, Param, Parameters}, stmt::{HirAssignStatement, HirLValue, HirLetStatement, HirPattern, HirStatement}, }, node_interner::{self, DefinitionKind, NodeInterner, StmtId}, @@ -88,7 +88,8 @@ pub fn monomorphize(main: node_interner::FuncId, interner: &NodeInterner) -> Pro } let functions = vecmap(monomorphizer.finished_functions, |(_, f)| f); - Program::new(functions, function_sig) + let FuncMeta { return_distinctness, .. } = interner.function_meta(&main); + Program::new(functions, function_sig, return_distinctness) } impl<'interner> Monomorphizer<'interner> { diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index f4793d06368..065b6362fb4 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -40,7 +40,7 @@ use crate::{ use chumsky::prelude::*; use iter_extended::vecmap; -use noirc_abi::AbiVisibility; +use noirc_abi::{AbiDistinctness, AbiVisibility}; use noirc_errors::{CustomDiagnostic, Span, Spanned}; /// Entry function for the parser - also handles lexing internally. @@ -162,7 +162,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser { |( ( ((((attribute, (is_unconstrained, is_open)), name), generics), parameters), - (return_visibility, return_type), + ((return_distinctness, return_visibility), return_type), ), body, )| { @@ -177,6 +177,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser { body, return_type, return_visibility, + return_distinctness, } .into() }, @@ -235,12 +236,18 @@ fn lambda_return_type() -> impl NoirParser { .map(|ret| ret.unwrap_or(UnresolvedType::Unspecified)) } -fn function_return_type() -> impl NoirParser<(AbiVisibility, UnresolvedType)> { +fn function_return_type() -> impl NoirParser<((AbiDistinctness, AbiVisibility), UnresolvedType)> { just(Token::Arrow) - .ignore_then(optional_visibility()) + .ignore_then(optional_distinctness()) + .then(optional_visibility()) .then(parse_type()) .or_not() - .map(|ret| ret.unwrap_or((AbiVisibility::Private, UnresolvedType::Unit))) + .map(|ret| { + ret.unwrap_or(( + (AbiDistinctness::DuplicationAllowed, AbiVisibility::Private), + UnresolvedType::Unit, + )) + }) } fn attribute() -> impl NoirParser { @@ -554,6 +561,13 @@ fn optional_visibility() -> impl NoirParser { }) } +fn optional_distinctness() -> impl NoirParser { + keyword(Keyword::Distinct).or_not().map(|opt| match opt { + Some(_) => AbiDistinctness::Distinct, + None => AbiDistinctness::DuplicationAllowed, + }) +} + fn maybe_comp_time() -> impl NoirParser { keyword(Keyword::CompTime).or_not().map(|opt| match opt { Some(_) => CompTime::Yes(None), @@ -1257,6 +1271,7 @@ mod test { "fn f(f: pub Field, y : Field, z : comptime Field) -> u8 { x + a }", "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) {}", "fn func_name(x: [Field], y : [Field;2],y : pub [Field;2], z : pub [u8;5]) {}", + "fn main(x: pub u8, y: pub u8) -> distinct pub [u8; 2] { [x, y] }" ], );