Skip to content

Commit

Permalink
Fix merge conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher committed Apr 26, 2023
2 parents a0b6f53 + 0dc2cac commit 27c98c2
Show file tree
Hide file tree
Showing 18 changed files with 208 additions and 18 deletions.
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn main() -> eyre::Result<()> {
// Register a panic hook to display more readable panic messages to end-users
let (panic_hook, _) = HookBuilder::default()
.display_env_section(false)
.panic_section("This is a bug. Consider opening an issue at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml")
.panic_section("This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.\nIf there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml")
.into_hooks();
panic_hook.install();

Expand Down
5 changes: 5 additions & 0 deletions crates/nargo_cli/tests/test_data/assert/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
1 change: 1 addition & 0 deletions crates/nargo_cli/tests/test_data/assert/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = "1"
3 changes: 3 additions & 0 deletions crates/nargo_cli/tests/test_data/assert/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main(x: Field) {
assert(x == 1);
}
24 changes: 24 additions & 0 deletions crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 18 additions & 2 deletions crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Witness>,
// 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<AcirOpcode>,
}
Expand Down Expand Up @@ -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.
//
Expand All @@ -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
Expand All @@ -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);

Expand Down
12 changes: 11 additions & 1 deletion crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use acvm::acir::native_types::Expression;

use crate::{
errors::RuntimeErrorKind,
ssa::{
Expand Down Expand Up @@ -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);
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions crates/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -104,8 +107,6 @@ impl CrateDefMap {

/// Find the main function for this crate
pub fn main_function(&self) -> Option<FuncId> {
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
Expand Down
14 changes: 14 additions & 0 deletions crates/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -176,6 +178,18 @@ impl From<ResolverError> 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(),
Expand Down
22 changes: 20 additions & 2 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
}
}
Expand All @@ -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
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion crates/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions crates/noirc_frontend/src/lexer/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ fn test_basic_language_syntax() {
x * y;
};
constrain mul(five, ten) == 50;
assert(ten + five == 15);
";

let expected = vec![
Expand Down Expand Up @@ -601,6 +602,15 @@ fn test_basic_language_syntax() {
Token::Equal,
Token::Int(50_i128.into()),
Token::Semicolon,
Token::Keyword(Keyword::Assert),
Token::LeftParen,
Token::Ident("ten".to_string()),
Token::Plus,
Token::Ident("five".to_string()),
Token::Equal,
Token::Int(15_i128.into()),
Token::RightParen,
Token::Semicolon,
Token::EOF,
];
let mut lexer = Lexer::new(input);
Expand Down
6 changes: 6 additions & 0 deletions crates/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,15 @@ impl AsRef<str> for Attribute {
#[cfg_attr(test, derive(strum_macros::EnumIter))]
pub enum Keyword {
As,
Assert,
Bool,
Char,
CompTime,
Constrain,
Contract,
Crate,
Dep,
Distinct,
Else,
Field,
Fn,
Expand All @@ -447,13 +449,15 @@ impl fmt::Display for Keyword {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Keyword::As => write!(f, "as"),
Keyword::Assert => write!(f, "assert"),
Keyword::Bool => write!(f, "bool"),
Keyword::Char => write!(f, "char"),
Keyword::CompTime => write!(f, "comptime"),
Keyword::Constrain => write!(f, "constrain"),
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"),
Expand Down Expand Up @@ -483,13 +487,15 @@ impl Keyword {
pub(crate) fn lookup_keyword(word: &str) -> Option<Token> {
let keyword = match word {
"as" => Keyword::As,
"assert" => Keyword::Assert,
"bool" => Keyword::Bool,
"char" => Keyword::Char,
"comptime" => Keyword::CompTime,
"constrain" => Keyword::Constrain,
"contract" => Keyword::Contract,
"crate" => Keyword::Crate,
"dep" => Keyword::Dep,
"distinct" => Keyword::Distinct,
"else" => Keyword::Else,
"Field" => Keyword::Field,
"fn" => Keyword::Fn,
Expand Down
13 changes: 11 additions & 2 deletions crates/noirc_frontend/src/monomorphization/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,20 @@ impl Type {
pub struct Program {
pub functions: Vec<Function>,
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<Function>, main_function_signature: FunctionSignature) -> Program {
Program { functions, main_function_signature }
pub fn new(
functions: Vec<Function>,
main_function_signature: FunctionSignature,
return_distinctness: noirc_abi::AbiDistinctness,
) -> Program {
Program { functions, main_function_signature, return_distinctness }
}

pub fn main(&self) -> &Function {
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 @@ -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},
Expand Down Expand Up @@ -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> {
Expand Down
Loading

0 comments on commit 27c98c2

Please sign in to comment.