From e3bea13eb775b380d1243a60a0c87b9d6dbe3732 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 31 Jan 2023 11:42:23 +0000 Subject: [PATCH 01/14] Apply witness visibility on a parameter level rather than witness level (#712) * refactor: apply witness visibility on a parameter level * style: clippy --- crates/noirc_evaluator/src/lib.rs | 42 ++++++++++++++----------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index fc5921750cd..9269001c5be 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -135,7 +135,10 @@ impl Evaluator { ); } AbiType::Array { length, typ } => { - let witnesses = self.generate_array_witnesses(visibility, length, typ)?; + let witnesses = self.generate_array_witnesses(length, typ)?; + if *visibility == AbiVisibility::Public { + self.public_inputs.extend(witnesses.clone()); + } igen.abi_array(name, Some(def), typ.as_ref(), *length, witnesses); } AbiType::Integer { sign: _, width } => { @@ -157,17 +160,26 @@ impl Evaluator { igen.create_new_variable(name.to_owned(), Some(def), obj_type, Some(witness)); } AbiType::Struct { fields } => { - let mut struct_witnesses: BTreeMap> = BTreeMap::new(); let new_fields = btree_map(fields, |(inner_name, value)| { let new_name = format!("{name}.{inner_name}"); (new_name, value.clone()) }); - self.generate_struct_witnesses(&mut struct_witnesses, visibility, &new_fields)?; + + let mut struct_witnesses: BTreeMap> = BTreeMap::new(); + self.generate_struct_witnesses(&mut struct_witnesses, &new_fields)?; + if *visibility == AbiVisibility::Public { + let witnesses: Vec = + struct_witnesses.values().flatten().cloned().collect(); + self.public_inputs.extend(witnesses); + } igen.abi_struct(name, Some(def), fields, struct_witnesses); } AbiType::String { length } => { let typ = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 }; - let witnesses = self.generate_array_witnesses(visibility, length, &typ)?; + let witnesses = self.generate_array_witnesses(length, &typ)?; + if *visibility == AbiVisibility::Public { + self.public_inputs.extend(witnesses.clone()); + } igen.abi_array(name, Some(def), &typ, *length, witnesses); } } @@ -177,7 +189,6 @@ impl Evaluator { fn generate_struct_witnesses( &mut self, struct_witnesses: &mut BTreeMap>, - visibility: &AbiVisibility, fields: &BTreeMap, ) -> Result<(), RuntimeErrorKind> { for (name, typ) in fields { @@ -186,28 +197,18 @@ impl Evaluator { let witness = self.add_witness_to_cs(); struct_witnesses.insert(name.clone(), vec![witness]); ssa::acir_gen::range_constraint(witness, *width, self)?; - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } } AbiType::Boolean => { let witness = self.add_witness_to_cs(); struct_witnesses.insert(name.clone(), vec![witness]); ssa::acir_gen::range_constraint(witness, 1, self)?; - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } } AbiType::Field => { let witness = self.add_witness_to_cs(); struct_witnesses.insert(name.clone(), vec![witness]); - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } } AbiType::Array { length, typ } => { - let internal_arr_witnesses = - self.generate_array_witnesses(visibility, length, typ)?; + let internal_arr_witnesses = self.generate_array_witnesses(length, typ)?; struct_witnesses.insert(name.clone(), internal_arr_witnesses); } AbiType::Struct { fields, .. } => { @@ -216,12 +217,11 @@ impl Evaluator { let new_name = format!("{name}.{inner_name}"); new_fields.insert(new_name, value.clone()); } - self.generate_struct_witnesses(struct_witnesses, visibility, &new_fields)? + self.generate_struct_witnesses(struct_witnesses, &new_fields)? } AbiType::String { length } => { let typ = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 }; - let internal_str_witnesses = - self.generate_array_witnesses(visibility, length, &typ)?; + let internal_str_witnesses = self.generate_array_witnesses(length, &typ)?; struct_witnesses.insert(name.clone(), internal_str_witnesses); } } @@ -231,7 +231,6 @@ impl Evaluator { fn generate_array_witnesses( &mut self, - visibility: &AbiVisibility, length: &u64, typ: &AbiType, ) -> Result, RuntimeErrorKind> { @@ -246,9 +245,6 @@ impl Evaluator { if let Some(ww) = element_width { ssa::acir_gen::range_constraint(witness, ww, self)?; } - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } } Ok(witnesses) } From d78739cccee7f596fa96fef9172c3fe9fa076e02 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 31 Jan 2023 14:43:59 +0000 Subject: [PATCH 02/14] Replace `toml_map_to_field` and `toml_remap` with traits to map between `InputValue`s and `TomlTypes` (#677) * refactor: implement TomlTypes::from() * refactor: disallow passing empty strings to parse_str_to_field * refactor: implement InputValue::try_from_toml * refactor: remove panics from InputValue::try_from_toml * fix: pass in field types rather than the full struct type in InputValue::try_from_toml --- Cargo.lock | 1 + crates/iter-extended/src/lib.rs | 10 ++ crates/noirc_abi/Cargo.toml | 1 + crates/noirc_abi/src/input_parser/toml.rs | 190 +++++++++------------- 4 files changed, 92 insertions(+), 110 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 14497062098..0fd2b9d86bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1748,6 +1748,7 @@ version = "0.1.0" dependencies = [ "acvm", "blake2", + "iter-extended", "serde", "serde_derive", "serde_json", diff --git a/crates/iter-extended/src/lib.rs b/crates/iter-extended/src/lib.rs index 5ca5534570b..b840dc1c9b3 100644 --- a/crates/iter-extended/src/lib.rs +++ b/crates/iter-extended/src/lib.rs @@ -27,3 +27,13 @@ where { iterable.into_iter().map(f).collect() } + +/// Equivalent to .into_iter().map(f).collect::,_>>() +pub fn try_btree_map(iterable: T, f: F) -> Result, E> +where + T: IntoIterator, + K: std::cmp::Ord, + F: FnMut(T::Item) -> Result<(K, V), E>, +{ + iterable.into_iter().map(f).collect() +} diff --git a/crates/noirc_abi/Cargo.toml b/crates/noirc_abi/Cargo.toml index 9c82f87da84..2f4b0fdcaff 100644 --- a/crates/noirc_abi/Cargo.toml +++ b/crates/noirc_abi/Cargo.toml @@ -8,6 +8,7 @@ edition.workspace = true [dependencies] acvm.workspace = true +iter-extended.workspace = true toml.workspace = true serde.workspace = true serde_derive.workspace = true diff --git a/crates/noirc_abi/src/input_parser/toml.rs b/crates/noirc_abi/src/input_parser/toml.rs index 6500793097c..bcd2fab20b3 100644 --- a/crates/noirc_abi/src/input_parser/toml.rs +++ b/crates/noirc_abi/src/input_parser/toml.rs @@ -1,6 +1,7 @@ use super::InputValue; use crate::{errors::InputParserError, Abi, AbiType}; use acvm::FieldElement; +use iter_extended::{btree_map, try_btree_map, try_vecmap, vecmap}; use serde::Serialize; use serde_derive::Deserialize; use std::collections::BTreeMap; @@ -9,7 +10,7 @@ pub(crate) fn parse_toml( input_string: &str, abi: Abi, ) -> Result, InputParserError> { - // Parse input.toml into a BTreeMap, converting the argument to field elements + // Parse input.toml into a BTreeMap. let data: BTreeMap = toml::from_str(input_string)?; // The toml map is stored in an ordered BTreeMap. As the keys are strings the map is in alphanumerical order. @@ -18,30 +19,22 @@ pub(crate) fn parse_toml( // in the abi are already stored in a BTreeMap. let abi_map = abi.to_btree_map(); - let toml_map = toml_map_to_field(data, abi_map)?; - Ok(toml_map) + // Convert arguments to field elements. + try_btree_map(data, |(key, value)| { + InputValue::try_from_toml(value, &abi_map[&key]).map(|input_value| (key, input_value)) + }) } pub(crate) fn serialise_to_toml( w_map: &BTreeMap, ) -> Result { - let to_map = toml_remap(w_map); - // Toml requires that values be emitted before tables. Thus, we must reorder our map in case a TomlTypes::Table comes before any other values in the toml map // BTreeMap orders by key and we need the name of the input as our key, so we must split our maps in case a table type has a name that is alphanumerically less // than any other value type - let mut tables_map = BTreeMap::new(); - let to_map: BTreeMap = to_map - .into_iter() - .filter(|(k, v)| { - if matches!(v, TomlTypes::Table(_)) { - tables_map.insert(k.clone(), v.clone()); - false - } else { - true - } - }) - .collect(); + let (tables_map, to_map): (BTreeMap, BTreeMap) = w_map + .iter() + .map(|(key, value)| (key.to_owned(), TomlTypes::from(value.clone()))) + .partition(|(_, v)| matches!(v, TomlTypes::Table(_))); let mut toml_string = toml::to_string(&to_map)?; let toml_string_tables = toml::to_string(&tables_map)?; @@ -51,30 +44,63 @@ pub(crate) fn serialise_to_toml( Ok(toml_string) } -/// Converts the Toml mapping to the native representation that the compiler -/// understands for Inputs -fn toml_map_to_field( - toml_map: BTreeMap, - abi_map: BTreeMap, -) -> Result, InputParserError> { - let mut field_map = BTreeMap::new(); - for (parameter, value) in toml_map { - let mapped_value = match value { - TomlTypes::String(string) => { - let param_type = abi_map.get(¶meter).unwrap(); - match param_type { - AbiType::String { .. } => InputValue::String(string), - AbiType::Field | AbiType::Integer { .. } => { - let new_value = parse_str_to_field(&string)?; - if let Some(field_element) = new_value { - InputValue::Field(field_element) - } else { - InputValue::Undefined - } +#[derive(Debug, Deserialize, Serialize, Clone)] +#[serde(untagged)] +enum TomlTypes { + // This is most likely going to be a hex string + // But it is possible to support UTF-8 + String(String), + // Just a regular integer, that can fit in 128 bits + Integer(u64), + // Simple boolean flag + Bool(bool), + // Array of regular integers + ArrayNum(Vec), + // Array of hexadecimal integers + ArrayString(Vec), + // Struct of TomlTypes + Table(BTreeMap), +} + +impl From for TomlTypes { + fn from(value: InputValue) -> Self { + match value { + InputValue::Field(f) => { + let f_str = format!("0x{}", f.to_hex()); + TomlTypes::String(f_str) + } + InputValue::Vec(v) => { + let array = v.iter().map(|i| format!("0x{}", i.to_hex())).collect(); + TomlTypes::ArrayString(array) + } + InputValue::String(s) => TomlTypes::String(s), + InputValue::Struct(map) => { + let map_with_toml_types = + btree_map(map, |(key, value)| (key, TomlTypes::from(value))); + TomlTypes::Table(map_with_toml_types) + } + InputValue::Undefined => unreachable!(), + } + } +} + +impl InputValue { + fn try_from_toml( + value: TomlTypes, + param_type: &AbiType, + ) -> Result { + let input_value = match value { + TomlTypes::String(string) => match param_type { + AbiType::String { .. } => InputValue::String(string), + AbiType::Field | AbiType::Integer { .. } => { + if string.is_empty() { + InputValue::Undefined + } else { + InputValue::Field(parse_str_to_field(&string)?) } - _ => return Err(InputParserError::AbiTypeMismatch(param_type.clone())), } - } + _ => return Err(InputParserError::AbiTypeMismatch(param_type.clone())), + }, TomlTypes::Integer(integer) => { let new_value = FieldElement::from(i128::from(integer)); @@ -86,97 +112,41 @@ fn toml_map_to_field( InputValue::Field(new_value) } TomlTypes::ArrayNum(arr_num) => { - let array_elements: Vec<_> = arr_num - .into_iter() - .map(|elem_num| FieldElement::from(i128::from(elem_num))) - .collect(); + let array_elements = + vecmap(arr_num, |elem_num| FieldElement::from(i128::from(elem_num))); InputValue::Vec(array_elements) } TomlTypes::ArrayString(arr_str) => { - let array_elements: Vec<_> = arr_str - .into_iter() - .map(|elem_str| parse_str_to_field(&elem_str).unwrap().unwrap()) - .collect(); + let array_elements = try_vecmap(arr_str, |elem_str| parse_str_to_field(&elem_str))?; InputValue::Vec(array_elements) } TomlTypes::Table(table) => { - let param_type = abi_map.get(¶meter).unwrap(); let fields = match param_type { - AbiType::Struct { fields } => fields.clone(), + AbiType::Struct { fields } => fields, _ => return Err(InputParserError::AbiTypeMismatch(param_type.clone())), }; - let native_table = toml_map_to_field(table, fields)?; + let native_table = try_btree_map(table, |(key, value)| { + InputValue::try_from_toml(value, &fields[&key]) + .map(|input_value| (key, input_value)) + })?; InputValue::Struct(native_table) } }; - if field_map.insert(parameter.clone(), mapped_value).is_some() { - return Err(InputParserError::DuplicateVariableName(parameter)); - }; - } - - Ok(field_map) -} - -fn toml_remap(map: &BTreeMap) -> BTreeMap { - let mut toml_map = BTreeMap::new(); - for (parameter, value) in map { - let mapped_value = match value { - InputValue::Field(f) => { - let f_str = format!("0x{}", f.to_hex()); - TomlTypes::String(f_str) - } - InputValue::Vec(v) => { - let array = v.iter().map(|i| format!("0x{}", i.to_hex())).collect(); - TomlTypes::ArrayString(array) - } - InputValue::String(s) => TomlTypes::String(s.clone()), - InputValue::Struct(map) => { - let map_with_toml_types = toml_remap(map); - TomlTypes::Table(map_with_toml_types) - } - InputValue::Undefined => unreachable!(), - }; - toml_map.insert(parameter.clone(), mapped_value); + Ok(input_value) } - toml_map } -#[derive(Debug, Deserialize, Serialize, Clone)] -#[serde(untagged)] -enum TomlTypes { - // This is most likely going to be a hex string - // But it is possible to support UTF-8 - String(String), - // Just a regular integer, that can fit in 128 bits - Integer(u64), - // Simple boolean flag - Bool(bool), - // Array of regular integers - ArrayNum(Vec), - // Array of hexadecimal integers - ArrayString(Vec), - // Struct of TomlTypes - Table(BTreeMap), -} - -fn parse_str_to_field(value: &str) -> Result, InputParserError> { - if value.is_empty() { - Ok(None) - } else if value.starts_with("0x") { - let result = FieldElement::from_hex(value); - if result.is_some() { - Ok(result) - } else { - Err(InputParserError::ParseHexStr(value.to_owned())) - } +fn parse_str_to_field(value: &str) -> Result { + if value.starts_with("0x") { + FieldElement::from_hex(value).ok_or_else(|| InputParserError::ParseHexStr(value.to_owned())) } else { - let val: i128 = value + value .parse::() - .map_err(|err_msg| InputParserError::ParseStr(err_msg.to_string()))?; - Ok(Some(FieldElement::from(val))) + .map_err(|err_msg| InputParserError::ParseStr(err_msg.to_string())) + .map(FieldElement::from) } } From 1bc224553e2256ef3ca989a605f4e10b48a177cf Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Tue, 31 Jan 2023 16:12:31 +0100 Subject: [PATCH 03/14] Review some TODO in SSA (#698) * review a few todos * implement Eq for InternalVar * Code review: ICE error instead of assert --- crates/noirc_evaluator/src/ssa/acir_gen.rs | 351 ++++++++++----------- crates/noirc_evaluator/src/ssa/code_gen.rs | 3 +- crates/noirc_evaluator/src/ssa/context.rs | 26 +- crates/noirc_evaluator/src/ssa/flatten.rs | 2 +- crates/noirc_evaluator/src/ssa/inline.rs | 1 - crates/noirc_evaluator/src/ssa/integer.rs | 29 +- crates/noirc_evaluator/src/ssa/node.rs | 84 +++-- crates/noirc_evaluator/src/ssa/optim.rs | 10 + 8 files changed, 279 insertions(+), 227 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen.rs b/crates/noirc_evaluator/src/ssa/acir_gen.rs index 8858d048032..9fbe4969d0c 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen.rs @@ -73,6 +73,15 @@ impl InternalVar { } } +impl PartialEq for InternalVar { + fn eq(&self, other: &Self) -> bool { + self.expression == other.expression + || (self.witness.is_some() && self.witness == other.witness) + || (self.id.is_some() && self.id == other.id) + } +} +impl Eq for InternalVar {} + impl From for InternalVar { fn from(arith: Expression) -> InternalVar { let w = is_unit(&arith); @@ -107,7 +116,7 @@ impl Acir { } let var = match ctx.try_get_node(id) { Some(node::NodeObj::Const(c)) => { - let f_value = FieldElement::from_be_bytes_reduce(&c.value.to_bytes_be()); //TODO const should be a field + let f_value = FieldElement::from_be_bytes_reduce(&c.value.to_bytes_be()); let expr = Expression::from_field(f_value); InternalVar::new(expr, None, id) } @@ -353,9 +362,27 @@ impl Acir { BinaryOp::Lte => unimplemented!( "Field comparison is not implemented yet, try to cast arguments to integer type" ), - BinaryOp::And => InternalVar::from(evaluate_and(l_c, r_c, res_type.bits(), evaluator)), - BinaryOp::Or => InternalVar::from(evaluate_or(l_c, r_c, res_type.bits(), evaluator)), - BinaryOp::Xor => InternalVar::from(evaluate_xor(l_c, r_c, res_type.bits(), evaluator)), + BinaryOp::And => InternalVar::from(evaluate_bitwise( + l_c, + r_c, + res_type.bits(), + evaluator, + BinaryOp::And, + )), + BinaryOp::Or => InternalVar::from(evaluate_bitwise( + l_c, + r_c, + res_type.bits(), + evaluator, + BinaryOp::Or, + )), + BinaryOp::Xor => InternalVar::from(evaluate_bitwise( + l_c, + r_c, + res_type.bits(), + evaluator, + BinaryOp::Xor, + )), BinaryOp::Shl | BinaryOp::Shr => unreachable!(), i @ BinaryOp::Assign => unreachable!("Invalid Instruction: {:?}", i), } @@ -495,7 +522,7 @@ impl Acir { //Transform the arguments of intrinsic functions into witnesses pub fn prepare_inputs( - &self, + &mut self, args: &[NodeId], cfg: &SsaContext, evaluator: &mut Evaluator, @@ -505,42 +532,44 @@ impl Acir { for a in args { let l_obj = cfg.try_get_node(*a).unwrap(); match l_obj { - node::NodeObj::Obj(v) => { - match l_obj.get_type() { - node::ObjectType::Pointer(a) => { - let array = &cfg.mem[a]; - let num_bits = array.element_type.bits(); - for i in 0..array.len { - let address = array.adr + i; - if self.memory_map.contains_key(&address) { - if let Some(wit) = self.memory_map[&address].witness { - inputs.push(FunctionInput { witness: wit, num_bits }); - } else { - //TODO we should store the witnesses somewhere, else if the inputs are re-used - //we will duplicate the witnesses. + node::NodeObj::Obj(v) => match l_obj.get_type() { + node::ObjectType::Pointer(a) => { + let array = &cfg.mem[a]; + let num_bits = array.element_type.bits(); + for i in 0..array.len { + let address = array.adr + i; + if self.memory_map.contains_key(&address) { + if let Some(wit) = self.memory_map[&address].witness { + inputs.push(FunctionInput { witness: wit, num_bits }); + } else { + let mut var = self.memory_map[&address].clone(); + if var.expression.is_const() { let w = evaluator.create_intermediate_variable( self.memory_map[&address].expression.clone(), ); - inputs.push(FunctionInput { witness: w, num_bits }); + var.witness = Some(w); } - } else { - inputs.push(FunctionInput { - witness: array.values[i as usize].witness.unwrap(), - num_bits, - }); + let w = var.generate_witness(evaluator); + self.memory_map.insert(address, var); + + inputs.push(FunctionInput { witness: w, num_bits }); } - } - } - _ => { - if let Some(w) = v.witness { - inputs - .push(FunctionInput { witness: w, num_bits: v.size_in_bits() }); } else { - todo!("generate a witness"); + inputs.push(FunctionInput { + witness: array.values[i as usize].witness.unwrap(), + num_bits, + }); } } } - } + _ => { + if let Some(w) = v.witness { + inputs.push(FunctionInput { witness: w, num_bits: v.size_in_bits() }); + } else { + todo!("generate a witness"); + } + } + }, _ => { if self.arith_cache.contains_key(a) { let mut var = self.arith_cache[a].clone(); @@ -734,144 +763,136 @@ pub fn to_radix_base( result } -fn const_and( - var: InternalVar, - b: FieldElement, +fn simplify_bitwise( + lhs: &InternalVar, + rhs: &InternalVar, bit_size: u32, - evaluator: &mut Evaluator, -) -> Expression { - let a_bits = to_radix_base(&var, 2, bit_size, evaluator); - let mut result = Expression::default(); - let mut k = FieldElement::one(); - let two = FieldElement::from(2_i128); - for (a_iter, b_iter) in a_bits.into_iter().zip(b.bits().iter().rev()) { - if *b_iter { - result = add(&result, k, &from_witness(a_iter)); - } - k = k.mul(two); + opcode: &BinaryOp, +) -> Option { + if lhs == rhs { + //simplify bitwise operation of the form: a OP a + return Some(match opcode { + BinaryOp::And => lhs.clone(), + BinaryOp::Or => lhs.clone(), + BinaryOp::Xor => InternalVar::from(FieldElement::zero()), + _ => unreachable!(), + }); } - result -} -fn const_xor( - var: InternalVar, - b: FieldElement, - bit_size: u32, - evaluator: &mut Evaluator, -) -> Expression { - let a_bits = to_radix_base(&var, 2, bit_size, evaluator); - let mut result = Expression::default(); - let mut k = FieldElement::one(); - let two = FieldElement::from(2_i128); - for (a_iter, b_iter) in a_bits.into_iter().zip(b.bits().iter().rev()) { - if *b_iter { - let c = subtract(&Expression::one(), FieldElement::one(), &from_witness(a_iter)); - result = add(&result, k, &c); - } else { - result = add(&result, k, &from_witness(a_iter)); + assert!(bit_size < FieldElement::max_num_bits()); + let max = FieldElement::from((1_u128 << bit_size) - 1); + let mut field = None; + let mut var = lhs; + if let Some(l_c) = lhs.to_const() { + if l_c == FieldElement::zero() || l_c == max { + field = Some(l_c); + var = rhs } - k = k.mul(two); - } - result -} - -fn const_or( - var: InternalVar, - b: FieldElement, - bit_size: u32, - evaluator: &mut Evaluator, -) -> Expression { - if let Some(l_c) = var.to_const() { - return Expression { - mul_terms: Vec::new(), - linear_combinations: Vec::new(), - q_c: (l_c.to_u128() | b.to_u128()).into(), - }; - } - let a_bits = to_radix_base(&var, 2, bit_size, evaluator); - let mut result = Expression::default(); - let mut k = FieldElement::one(); - let two = FieldElement::from(2_i128); - let mut q_c = FieldElement::zero(); - for (a_iter, b_iter) in a_bits.into_iter().zip(b.bits().iter().rev()) { - if *b_iter { - q_c += k; - } else { - result = add(&result, k, &from_witness(a_iter)); + } else if let Some(r_c) = rhs.to_const() { + if r_c == FieldElement::zero() || r_c == max { + field = Some(r_c); } - k = k.mul(two); } - result.q_c = q_c; - result -} - -pub fn evaluate_and( - mut lhs: InternalVar, - mut rhs: InternalVar, - bit_size: u32, - evaluator: &mut Evaluator, -) -> Expression { - if let Some(r_c) = rhs.to_const() { - return const_and(lhs, r_c, bit_size, evaluator); - } - if let Some(l_c) = lhs.to_const() { - return const_and(rhs, l_c, bit_size, evaluator); - } - - let a_witness = lhs.generate_witness(evaluator); - let b_witness = rhs.generate_witness(evaluator); - //TODO checks the cost of the gate vs bit_size (cf. #164) - if bit_size == 1 { - return Expression { - mul_terms: vec![(FieldElement::one(), a_witness, b_witness)], - linear_combinations: Vec::new(), - q_c: FieldElement::zero(), - }; + if let Some(field) = field { + //simplify bitwise operation of the form: 0 OP var or 1 OP var + return Some(match opcode { + BinaryOp::And => { + if field.is_zero() { + InternalVar::from(field) + } else { + var.clone() + } + } + BinaryOp::Xor => { + if field.is_zero() { + var.clone() + } else { + InternalVar::from(subtract( + &Expression::from_field(field), + FieldElement::one(), + &var.expression, + )) + } + } + BinaryOp::Or => { + if field.is_zero() { + var.clone() + } else { + InternalVar::from(field) + } + } + _ => unreachable!(), + }); } - let result = evaluator.add_witness_to_cs(); - let bsize = if bit_size % 2 == 1 { bit_size + 1 } else { bit_size }; - assert!(bsize < FieldElement::max_num_bits() - 1); - let gate = AcirOpcode::BlackBoxFuncCall(BlackBoxFuncCall { - name: acvm::acir::BlackBoxFunc::AND, - inputs: vec![ - FunctionInput { witness: a_witness, num_bits: bsize }, - FunctionInput { witness: b_witness, num_bits: bsize }, - ], - outputs: vec![result], - }); - evaluator.opcodes.push(gate); - - Expression::from(Linear::from_witness(result)) + None } -pub fn evaluate_xor( +fn evaluate_bitwise( mut lhs: InternalVar, mut rhs: InternalVar, bit_size: u32, evaluator: &mut Evaluator, + opcode: BinaryOp, ) -> Expression { - if let Some(r_c) = rhs.to_const() { - return const_xor(lhs, r_c, bit_size, evaluator); + if let Some(var) = simplify_bitwise(&lhs, &rhs, bit_size, &opcode) { + return var.expression; } - if let Some(l_c) = lhs.to_const() { - return const_xor(rhs, l_c, bit_size, evaluator); - } - - //TODO checks the cost of the gate vs bit_size (cf. #164) if bit_size == 1 { - let sum = add(&lhs.expression, FieldElement::one(), &rhs.expression); - let mul = mul_with_witness(evaluator, &lhs.expression, &rhs.expression); - return subtract(&sum, FieldElement::from(2_i128), &mul); + match opcode { + BinaryOp::And => return mul_with_witness(evaluator, &lhs.expression, &rhs.expression), + BinaryOp::Xor => { + let sum = add(&lhs.expression, FieldElement::one(), &rhs.expression); + let mul = mul_with_witness(evaluator, &lhs.expression, &rhs.expression); + return subtract(&sum, FieldElement::from(2_i128), &mul); + } + BinaryOp::Or => { + let sum = add(&lhs.expression, FieldElement::one(), &rhs.expression); + let mul = mul_with_witness(evaluator, &lhs.expression, &rhs.expression); + return subtract(&sum, FieldElement::one(), &mul); + } + _ => unreachable!(), + } } + //We generate witness from const values in order to use the ACIR bitwise gates + // If the gate is implemented, it is expected to be better than going through bit decomposition, even if one of the operand is a constant + // If the gate is not implemented, we rely on the ACIR simplification to remove these witnesses + if rhs.to_const().is_some() && rhs.witness.is_none() { + rhs.witness = Some(evaluator.create_intermediate_variable(rhs.expression.clone())); + assert!(lhs.to_const().is_none()); + } else if lhs.to_const().is_some() && lhs.witness.is_none() { + assert!(rhs.to_const().is_none()); + lhs.witness = Some(evaluator.create_intermediate_variable(lhs.expression.clone())); + } + + let mut a_witness = lhs.generate_witness(evaluator); + let mut b_witness = rhs.generate_witness(evaluator); + let result = evaluator.add_witness_to_cs(); - let a_witness = lhs.generate_witness(evaluator); - let b_witness = rhs.generate_witness(evaluator); let bsize = if bit_size % 2 == 1 { bit_size + 1 } else { bit_size }; assert!(bsize < FieldElement::max_num_bits() - 1); + let max = FieldElement::from((1_u128 << bit_size) - 1); + let bit_gate = match opcode { + BinaryOp::And => acvm::acir::BlackBoxFunc::AND, + BinaryOp::Xor => acvm::acir::BlackBoxFunc::XOR, + BinaryOp::Or => { + a_witness = evaluator.create_intermediate_variable(subtract( + &Expression::from_field(max), + FieldElement::one(), + &lhs.expression, + )); + b_witness = evaluator.create_intermediate_variable(subtract( + &Expression::from_field(max), + FieldElement::one(), + &rhs.expression, + )); + acvm::acir::BlackBoxFunc::AND + } + _ => unreachable!(), + }; let gate = AcirOpcode::BlackBoxFuncCall(BlackBoxFuncCall { - name: acvm::acir::BlackBoxFunc::XOR, + name: bit_gate, inputs: vec![ FunctionInput { witness: a_witness, num_bits: bsize }, FunctionInput { witness: b_witness, num_bits: bsize }, @@ -880,39 +901,11 @@ pub fn evaluate_xor( }); evaluator.opcodes.push(gate); - from_witness(result) -} - -pub fn evaluate_or( - lhs: InternalVar, - rhs: InternalVar, - bit_size: u32, - evaluator: &mut Evaluator, -) -> Expression { - if let Some(r_c) = rhs.to_const() { - return const_or(lhs, r_c, bit_size, evaluator); - } - if let Some(l_c) = lhs.to_const() { - return const_or(rhs, l_c, bit_size, evaluator); - } - - if bit_size == 1 { - let sum = add(&lhs.expression, FieldElement::one(), &rhs.expression); - let mul = mul_with_witness(evaluator, &lhs.expression, &rhs.expression); - return subtract(&sum, FieldElement::one(), &mul); - } - - let lhs_bits = to_radix_base(&lhs, 2, bit_size, evaluator); - let rhs_bits = to_radix_base(&rhs, 2, bit_size, evaluator); - let mut result = Expression::default(); - let mut k = FieldElement::one(); - let two = FieldElement::from(2_i128); - for (l_bit, r_bit) in lhs_bits.into_iter().zip(rhs_bits) { - let l_or_r = evaluate_or(l_bit.into(), r_bit.into(), 1, evaluator); - result = add(&result, k, &l_or_r); - k = k.mul(two); + if opcode == BinaryOp::Or { + subtract(&Expression::from_field(max), FieldElement::one(), &from_witness(result)) + } else { + from_witness(result) } - result } //truncate lhs (a number whose value requires max_bits) into a rhs-bits number: i.e it returns b such that lhs mod 2^rhs is b diff --git a/crates/noirc_evaluator/src/ssa/code_gen.rs b/crates/noirc_evaluator/src/ssa/code_gen.rs index 43387ffdf40..0e3df891aec 100644 --- a/crates/noirc_evaluator/src/ssa/code_gen.rs +++ b/crates/noirc_evaluator/src/ssa/code_gen.rs @@ -694,7 +694,6 @@ impl IRGenerator { let end_idx = self.codegen_expression(&for_expr.end_range).unwrap().unwrap_id(); //We support only const range for now - //TODO how should we handle scope (cf. start/end_for_loop)? let iter_def = Definition::Local(for_expr.index_variable); let iter_type = self.context.convert_type(&for_expr.index_type); let index_name = for_expr.index_name.clone(); @@ -762,7 +761,7 @@ impl IRGenerator { //seal join ssa_form::seal_block(&mut self.context, join_idx, join_idx); - Ok(Value::Single(exit_first)) //TODO what should we return??? + Ok(Value::Single(exit_first)) } //Parse a block of AST statements into ssa form diff --git a/crates/noirc_evaluator/src/ssa/context.rs b/crates/noirc_evaluator/src/ssa/context.rs index deb1254e06c..6ae4f1bd399 100644 --- a/crates/noirc_evaluator/src/ssa/context.rs +++ b/crates/noirc_evaluator/src/ssa/context.rs @@ -8,6 +8,7 @@ use super::{block, builtin, flatten, inline, integer, node, optim}; use std::collections::{HashMap, HashSet}; use super::super::errors::RuntimeError; +use crate::errors::RuntimeErrorKind; use crate::ssa::acir_gen::Acir; use crate::ssa::function; use crate::ssa::node::{Mark, Node}; @@ -404,7 +405,7 @@ impl SsaContext { self[id].get_type() } - //Returns the object value if it is a constant, None if not. TODO: handle types + //Returns the object value if it is a constant, None if not. pub fn get_as_constant(&self, id: NodeId) -> Option { if let Some(node::NodeObj::Const(c)) = self.try_get_node(id) { return Some(FieldElement::from_be_bytes_reduce(&c.value.to_bytes_be())); @@ -434,25 +435,30 @@ impl SsaContext { None } - pub fn get_variable(&self, id: NodeId) -> Result<&node::Variable, &str> { - //TODO proper error handling + pub fn get_variable(&self, id: NodeId) -> Result<&node::Variable, RuntimeErrorKind> { match self.nodes.get(id.0) { Some(t) => match t { node::NodeObj::Obj(o) => Ok(o), - _ => Err("Not an object"), + _ => Err(RuntimeErrorKind::UnstructuredError { + message: "Not an object".to_string(), + }), }, - _ => Err("Invalid id"), + _ => Err(RuntimeErrorKind::UnstructuredError { message: "Invalid id".to_string() }), } } - pub fn get_mut_variable(&mut self, id: NodeId) -> Result<&mut node::Variable, &str> { - //TODO proper error handling + pub fn get_mut_variable( + &mut self, + id: NodeId, + ) -> Result<&mut node::Variable, RuntimeErrorKind> { match self.nodes.get_mut(id.0) { Some(t) => match t { node::NodeObj::Obj(o) => Ok(o), - _ => Err("Not an object"), + _ => Err(RuntimeErrorKind::UnstructuredError { + message: "Not an object".to_string(), + }), }, - _ => Err("Invalid id"), + _ => Err(RuntimeErrorKind::UnstructuredError { message: "Invalid id".to_string() }), } } @@ -585,7 +591,7 @@ impl SsaContext { // Retrieve the object conresponding to the const value given in argument // If such object does not exist, we create one pub fn get_or_create_const(&mut self, x: FieldElement, t: node::ObjectType) -> NodeId { - let value = BigUint::from_bytes_be(&x.to_be_bytes()); //TODO a const should be a field element + let value = BigUint::from_bytes_be(&x.to_be_bytes()); if let Some(prev_const) = self.find_const_with_type(&value, t) { return prev_const; } diff --git a/crates/noirc_evaluator/src/ssa/flatten.rs b/crates/noirc_evaluator/src/ssa/flatten.rs index db48d77ff69..2c67af06ec0 100644 --- a/crates/noirc_evaluator/src/ssa/flatten.rs +++ b/crates/noirc_evaluator/src/ssa/flatten.rs @@ -266,7 +266,7 @@ fn evaluate_conditional_jump( Operation::Jeq(cond_id, _) => (cond_id, |field| !field.is_zero()), Operation::Jne(cond_id, _) => (cond_id, |field| field.is_zero()), Operation::Jmp(_) => return Ok(true), - _ => panic!("loop without conditional statement!"), //TODO shouldn't we return false instead? + _ => panic!("loop without conditional statement!"), }; let cond = get_current_value(cond_id, value_array); diff --git a/crates/noirc_evaluator/src/ssa/inline.rs b/crates/noirc_evaluator/src/ssa/inline.rs index 41c92ac6eb1..5d6626b39da 100644 --- a/crates/noirc_evaluator/src/ssa/inline.rs +++ b/crates/noirc_evaluator/src/ssa/inline.rs @@ -314,7 +314,6 @@ pub fn inline_in_block( } Operation::Load { array_id, index } => { //Compute the new address: - //TODO use relative addressing, but that requires a few changes, mainly in acir_gen.rs and integer.rs let b = stack_frame.get_or_default(*array_id); let mut new_ins = Instruction::new( Operation::Load { array_id: b, index: *index }, diff --git a/crates/noirc_evaluator/src/ssa/integer.rs b/crates/noirc_evaluator/src/ssa/integer.rs index 7ea4ed7a9a5..ed4945dcbfc 100644 --- a/crates/noirc_evaluator/src/ssa/integer.rs +++ b/crates/noirc_evaluator/src/ssa/integer.rs @@ -283,8 +283,6 @@ fn block_overflow( match ins.operation { Operation::Load { array_id, index } => { - //TODO we use a local memory map for now but it should be used in arguments - //for instance, the join block of a IF should merge the two memorymaps using the condition value if let Some(val) = ctx.get_indexed_value(array_id, index) { //optimise static load ins.mark = Mark::ReplaceWith(*val); @@ -316,19 +314,22 @@ fn block_overflow( } if let Some(r_const) = ctx.get_as_constant(rhs) { let r_type = ctx[rhs].get_type(); - let rhs = - ctx.get_or_create_const(FieldElement::from(2_i128).pow(&r_const), r_type); - //todo checks that 2^rhs does not overflow - ins.operation = Operation::Binary(node::Binary { - lhs, - rhs, - operator: BinaryOp::Udiv, - predicate: None, - }); + if r_const.to_u128() > r_type.bits() as u128 { + ins.mark = Mark::ReplaceWith(ctx.zero_with_type(ins.res_type)) + } else { + let rhs = ctx + .get_or_create_const(FieldElement::from(2_i128).pow(&r_const), r_type); + ins.operation = Operation::Binary(node::Binary { + lhs, + rhs, + operator: BinaryOp::Udiv, + predicate: None, + }); + } } } Operation::Cast(value_id) => { - // TODO for now the types we support here are only all integer types (field, signed, unsigned, bool) + // For now the types we support here are only all integer types (field, signed, unsigned, bool) // so a cast would normally translate to a truncate. // if res_type and lhs have the same bit size (in a large sense, which includes field elements) // then either they have the same type and should have been simplified @@ -462,7 +463,9 @@ fn get_max_value(ins: &Instruction, max_map: &mut HashMap) -> B max_map[value].clone(), BigUint::from(2_u32).pow(*max_bit_size) - BigUint::from(1_u32), ), - Operation::Nop | Operation::Jne(..) | Operation::Jeq(..) | Operation::Jmp(_) => todo!(), + Operation::Nop | Operation::Jne(..) | Operation::Jeq(..) | Operation::Jmp(_) => { + unreachable!() + } Operation::Phi { root, block_args } => { let mut max = max_map[root].clone(); for (id, _block) in block_args { diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs index a15414013cb..760eef32c1b 100644 --- a/crates/noirc_evaluator/src/ssa/node.rs +++ b/crates/noirc_evaluator/src/ssa/node.rs @@ -148,12 +148,8 @@ pub struct Variable { pub id: NodeId, pub obj_type: ObjectType, pub name: String, - //pub cur_value: arena::Index, //for generating the SSA form, current value of the object during parsing of the AST pub root: Option, //when generating SSA, assignment of an object creates a new one which is linked to the original one - pub def: Option, //TODO redundant with root - should it be an option? - //TODO clarify where cur_value and root is stored, and also this: - // pub max_bits: u32, //max possible bit size of the expression - // pub max_value: Option, //maximum possible value of the expression, if less than max_bits + pub def: Option, //AST definition of the variable pub witness: Option, pub parent_block: BlockId, } @@ -367,8 +363,8 @@ impl Instruction { | Operation::Cond { .. } => false, Operation::Load { .. } => false, Operation::Store { .. } => true, - Operation::Intrinsic(_, _) => true, //TODO to check - Operation::Call { .. } => false, //return values are in the return statment, should we truncate function arguments? probably but not lhs and rhs anyways. + Operation::Intrinsic(_, _) => true, + Operation::Call { .. } => true, //return values are in the return statment Operation::Return(_) => true, Operation::Result { .. } => false, } @@ -796,9 +792,7 @@ impl Binary { return zero_div_error; } else if l_is_zero { return Ok(l_eval); //TODO should we ensure rhs != 0 ??? - } - //constant folding - TODO - else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { + } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { return Ok(NodeEval::Const(lhs / rhs, res_type)); } } @@ -807,21 +801,33 @@ impl Binary { return zero_div_error; } else if l_is_zero { return Ok(l_eval); //TODO should we ensure rhs != 0 ??? - } - //constant folding...TODO - else if lhs.is_some() && rhs.is_some() { - todo!("Constant folding for division"); + } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { + let a = field_to_signed(lhs, res_type.bits()); + let b = field_to_signed(rhs, res_type.bits()); + return Ok(NodeEval::Const(signed_to_field(a / b, res_type.bits())?, res_type)); } } - BinaryOp::Urem | BinaryOp::Srem => { + BinaryOp::Urem => { if r_is_zero { return zero_div_error; } else if l_is_zero { return Ok(l_eval); //TODO what is the correct result? + } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { + return Ok(NodeEval::Const(lhs - rhs * (lhs / rhs), res_type)); } - //constant folding - TODO - else if lhs.is_some() && rhs.is_some() { - todo!("divide lhs/rhs but take sign into account"); + } + BinaryOp::Srem => { + if r_is_zero { + return zero_div_error; + } else if l_is_zero { + return Ok(l_eval); //TODO what is the correct result? + } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { + let a = field_to_signed(lhs, res_type.bits()); + let b = field_to_signed(rhs, res_type.bits()); + return Ok(NodeEval::Const( + signed_to_field(a - b + (a / b), res_type.bits())?, + res_type, + )); } } BinaryOp::Ult => { @@ -894,8 +900,15 @@ impl Binary { return Ok(r_eval); } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { return Ok(wrapping(lhs, rhs, res_type, u128::bitand, field_op_not_allowed)); + } else { + let n = res_type.bits(); + let max = FieldElement::from(2_u128.pow(n) - 1); + if lhs == Some(max) { + return Ok(r_eval); + } else if rhs == Some(max) { + return Ok(l_eval); + } } - //TODO if boolean and not zero, also checks this is correct for field elements } BinaryOp::Or => { //Bitwise OR @@ -905,8 +918,13 @@ impl Binary { return Ok(l_eval); } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { return Ok(wrapping(lhs, rhs, res_type, u128::bitor, field_op_not_allowed)); + } else { + let n = res_type.bits(); + let max = FieldElement::from(2_u128.pow(n) - 1); + if lhs == Some(max) || rhs == Some(max) { + return Ok(NodeEval::Const(max, res_type)); + } } - //TODO if boolean and not zero, also checks this is correct for field elements } BinaryOp::Xor => { if self.lhs == self.rhs { @@ -918,7 +936,6 @@ impl Binary { } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { return Ok(wrapping(lhs, rhs, res_type, u128::bitxor, field_op_not_allowed)); } - //TODO handle case when lhs is one (or rhs is one) by generating 'not rhs' instruction (or 'not lhs' instruction) } BinaryOp::Shl => { if l_is_zero { @@ -1228,3 +1245,28 @@ impl BinaryOp { ) } } + +fn field_to_signed(f: FieldElement, n: u32) -> i128 { + assert!(n < 127); + let a = f.to_u128(); + let pow_2 = 2_u128.pow(n); + if a < pow_2 { + a as i128 + } else { + (a - 2 * pow_2) as i128 + } +} + +fn signed_to_field(a: i128, n: u32) -> Result { + if n >= 126 { + return Err(RuntimeErrorKind::UnstructuredError { + message: "ICE: cannot convert signed {n} bit size into field".to_string(), + })?; + } + if a >= 0 { + Ok(FieldElement::from(a)) + } else { + let b = (a + 2_i128.pow(n + 1)) as u128; + Ok(FieldElement::from(b)) + } +} diff --git a/crates/noirc_evaluator/src/ssa/optim.rs b/crates/noirc_evaluator/src/ssa/optim.rs index 72604d58a18..72c7d629c83 100644 --- a/crates/noirc_evaluator/src/ssa/optim.rs +++ b/crates/noirc_evaluator/src/ssa/optim.rs @@ -56,6 +56,16 @@ pub fn simplify(ctx: &mut SsaContext, ins: &mut Instruction) -> Result<(), Runti } } } + if let Operation::Binary(binary) = &ins.operation { + if binary.operator == BinaryOp::Xor { + let max = FieldElement::from(2_u128.pow(ins.res_type.bits()) - 1); + if NodeEval::from_id(ctx, binary.rhs).into_const_value() == Some(max) { + ins.operation = Operation::Not(binary.lhs); + } else if NodeEval::from_id(ctx, binary.lhs).into_const_value() == Some(max) { + ins.operation = Operation::Not(binary.rhs); + } + } + } Ok(()) } From a484e28ea2c9e516d8b7d73a98e30a1357a42289 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 31 Jan 2023 09:19:04 -0600 Subject: [PATCH 04/14] Implement numeric generics (#620) * Add binary operation to type representation * Change representation by adding a separate TypeExpression enum * Allow types to reference globals * Remove non-global used where only globals are expected * Parse and validate numeric type expressions * Fix typechecking bug * Change test to the passing test from the bug report * Fix bug in global_consts test * Improve error message * Fix bug with NamedGenerics being translated as TypeVariables within TypeExpressions * Add support for parsing numeric generics within regular <> type brackets * Fix subtle parsing bug * Fix previous fix, use one_of instead of 'just', just is only for sequences * Change type functions to be explicitly wrapping * Remove ability to reference non-globals --- .../tests/test_data/global_consts/src/main.nr | 5 +- .../test_data/numeric_generics/Nargo.toml | 7 + .../test_data/numeric_generics/Prover.toml | 0 .../test_data/numeric_generics/src/main.nr | 28 +++ crates/noirc_frontend/src/ast/mod.rs | 100 ++++++++- .../src/hir/resolution/errors.rs | 7 + .../src/hir/resolution/resolver.rs | 82 +++++++- .../noirc_frontend/src/hir/type_check/expr.rs | 10 +- crates/noirc_frontend/src/hir_def/types.rs | 114 ++++++---- .../src/monomorphisation/mod.rs | 11 +- crates/noirc_frontend/src/parser/mod.rs | 3 +- crates/noirc_frontend/src/parser/parser.rs | 102 ++++----- noir_stdlib/src/hash.nr | 195 +++++++++--------- 13 files changed, 439 insertions(+), 225 deletions(-) create mode 100644 crates/nargo/tests/test_data/numeric_generics/Nargo.toml create mode 100644 crates/nargo/tests/test_data/numeric_generics/Prover.toml create mode 100644 crates/nargo/tests/test_data/numeric_generics/src/main.nr diff --git a/crates/nargo/tests/test_data/global_consts/src/main.nr b/crates/nargo/tests/test_data/global_consts/src/main.nr index 2eb785d27fd..fb48eb2b798 100644 --- a/crates/nargo/tests/test_data/global_consts/src/main.nr +++ b/crates/nargo/tests/test_data/global_consts/src/main.nr @@ -4,6 +4,7 @@ mod baz; global M: Field = 32; global L: Field = 10; // Unused globals currently allowed global N: Field = 5; +global T_LEN = 2; // Type inference is allowed on globals //global N: Field = 5; // Uncomment to see duplicate globals error struct Dummy { @@ -40,9 +41,7 @@ fn main(a: [Field; M + N - N], b: [Field; 30 + N / 2], c : pub [Field; foo::MAGI arrays_neq(a, b); - //let mut L: Field = 2; // Uncomment to show expected comptime error for array annotations - let L: comptime Field = 2; - let t: [Field; L] = [N, M]; + let t: [Field; T_LEN] = [N, M]; constrain t[1] == 32; constrain 15 == mysubmodule::my_helper(); diff --git a/crates/nargo/tests/test_data/numeric_generics/Nargo.toml b/crates/nargo/tests/test_data/numeric_generics/Nargo.toml new file mode 100644 index 00000000000..d9434868157 --- /dev/null +++ b/crates/nargo/tests/test_data/numeric_generics/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + authors = [""] + compiler_version = "0.1" + + [dependencies] + \ No newline at end of file diff --git a/crates/nargo/tests/test_data/numeric_generics/Prover.toml b/crates/nargo/tests/test_data/numeric_generics/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/crates/nargo/tests/test_data/numeric_generics/src/main.nr b/crates/nargo/tests/test_data/numeric_generics/src/main.nr new file mode 100644 index 00000000000..7d73eabe2be --- /dev/null +++ b/crates/nargo/tests/test_data/numeric_generics/src/main.nr @@ -0,0 +1,28 @@ +fn main() { + let a = id([1, 2]); + let b = id([1, 2, 3]); + + let itWorks1 = MyStruct { data: a }; + constrain itWorks1.data[1] == 2; + let itWorks2 = MyStruct { data: b }; + constrain itWorks2.data[1] == 2; + + let c = [1, 2]; + let itAlsoWorks = MyStruct { data: c }; + constrain itAlsoWorks.data[1] == 2; + + constrain foo(itWorks2).data[0] == itWorks2.data[0] + 1; +} + +fn id(x: [Field; I]) -> [Field; I] { + x +} + +struct MyStruct { + data: [Field; S] +} + +fn foo(mut s: MyStruct<2+1>) -> MyStruct<10/2-2> { + s.data[0] = s.data[0] + 1; + s +} diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index c404b014044..f9c01ff6380 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -12,7 +12,7 @@ use noirc_errors::Span; pub use statement::*; pub use structure::*; -use crate::{token::IntType, Comptime}; +use crate::{parser::ParserError, token::IntType, BinaryTypeOperator, Comptime}; use iter_extended::vecmap; /// The parser parses types as 'UnresolvedType's which @@ -21,10 +21,11 @@ use iter_extended::vecmap; #[derive(Debug, PartialEq, Eq, Clone)] pub enum UnresolvedType { FieldElement(Comptime), - Array(Option, Box), // [4]Witness = Array(4, Witness) - Integer(Comptime, Signedness, u32), // u32 = Integer(unsigned, 32) + Array(Option, Box), // [4]Witness = Array(4, Witness) + Integer(Comptime, Signedness, u32), // u32 = Integer(unsigned, 32) Bool(Comptime), - String(Option), + Expression(UnresolvedTypeExpression), + String(Option), Unit, /// A Named UnresolvedType can be a struct type or a type variable @@ -39,6 +40,21 @@ pub enum UnresolvedType { Error, } +/// The precursor to TypeExpression, this is the type that the parser allows +/// to be used in the length position of an array type. Only constants, variables, +/// and numeric binary operators are allowed here. +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum UnresolvedTypeExpression { + Variable(Path), + Constant(u64, Span), + BinaryOperation( + Box, + BinaryTypeOperator, + Box, + Span, + ), +} + impl Recoverable for UnresolvedType { fn error(_: Span) -> Self { UnresolvedType::Error @@ -70,6 +86,7 @@ impl std::fmt::Display for UnresolvedType { let elements = vecmap(elements, ToString::to_string); write!(f, "({})", elements.join(", ")) } + Expression(expression) => expression.fmt(f), Bool(is_const) => write!(f, "{is_const}bool"), String(len) => match len { None => write!(f, "str[]"), @@ -86,6 +103,18 @@ impl std::fmt::Display for UnresolvedType { } } +impl std::fmt::Display for UnresolvedTypeExpression { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + UnresolvedTypeExpression::Variable(name) => name.fmt(f), + UnresolvedTypeExpression::Constant(x, _) => x.fmt(f), + UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, _) => { + write!(f, "({lhs} {op} {rhs})") + } + } + } +} + impl UnresolvedType { pub fn from_int_token(token: (Comptime, IntType)) -> UnresolvedType { use {IntType::*, UnresolvedType::Integer}; @@ -101,3 +130,66 @@ pub enum Signedness { Unsigned, Signed, } + +impl UnresolvedTypeExpression { + pub fn from_expr( + expr: Expression, + span: Span, + ) -> Result { + Self::from_expr_helper(expr).map_err(|err| { + ParserError::with_reason( + format!("Expression is invalid in an array-length type: '{err}'. Only unsigned integer constants, globals, generics, +, -, *, /, and % may be used in this context."), + span, + ) + }) + } + + pub fn span(&self) -> Span { + match self { + UnresolvedTypeExpression::Variable(path) => path.span(), + UnresolvedTypeExpression::Constant(_, span) => *span, + UnresolvedTypeExpression::BinaryOperation(_, _, _, span) => *span, + } + } + + fn from_expr_helper(expr: Expression) -> Result { + match expr.kind { + ExpressionKind::Literal(Literal::Integer(int)) => match int.try_to_u64() { + Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)), + None => Err(expr), + }, + ExpressionKind::Variable(path) => Ok(UnresolvedTypeExpression::Variable(path)), + ExpressionKind::Prefix(prefix) if prefix.operator == UnaryOp::Minus => { + let lhs = Box::new(UnresolvedTypeExpression::Constant(0, expr.span)); + let rhs = Box::new(UnresolvedTypeExpression::from_expr_helper(prefix.rhs)?); + let op = BinaryTypeOperator::Subtraction; + Ok(UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, expr.span)) + } + ExpressionKind::Infix(infix) if Self::operator_allowed(infix.operator.contents) => { + let lhs = Box::new(UnresolvedTypeExpression::from_expr_helper(infix.lhs)?); + let rhs = Box::new(UnresolvedTypeExpression::from_expr_helper(infix.rhs)?); + let op = match infix.operator.contents { + BinaryOpKind::Add => BinaryTypeOperator::Addition, + BinaryOpKind::Subtract => BinaryTypeOperator::Subtraction, + BinaryOpKind::Multiply => BinaryTypeOperator::Multiplication, + BinaryOpKind::Divide => BinaryTypeOperator::Division, + BinaryOpKind::Modulo => BinaryTypeOperator::Modulo, + _ => unreachable!(), // impossible via operator_allowed check + }; + Ok(UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, expr.span)) + } + _ => Err(expr), + } + } + + fn operator_allowed(op: BinaryOpKind) -> bool { + matches!( + op, + BinaryOpKind::Add + | BinaryOpKind::Subtract + | BinaryOpKind::Multiply + | BinaryOpKind::Divide + | BinaryOpKind::Modulo + ) + } +} diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 8429bc50ab5..bc9449de438 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -38,6 +38,8 @@ pub enum ResolverError { InvalidArrayLengthExpr { span: Span }, #[error("Integer too large to be evaluated in an array length context")] IntegerTooLarge { span: Span }, + #[error("No global or generic type parameter found with the given name")] + NoSuchNumericTypeVariable { path: crate::Path }, #[error("Closures cannot capture mutable variables")] CapturedMutableVariable { span: Span }, } @@ -189,6 +191,11 @@ impl ResolverError { "Array-lengths may be a maximum size of usize::MAX, including intermediate calculations".into(), span, ), + ResolverError::NoSuchNumericTypeVariable { path } => Diagnostic::simple_error( + format!("Cannot find a global or generic type parameter named `{path}`"), + "Only globals or generic type parameters are allowed to be used as an array type's length".to_string(), + path.span(), + ), ResolverError::CapturedMutableVariable { span } => Diagnostic::simple_error( "Closures cannot capture mutable variables".into(), "Mutable variable".into(), diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index c989c8c6e18..0845ee4aee1 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -39,7 +39,7 @@ use crate::{ }; use crate::{ ArrayLiteral, Generics, LValue, NoirStruct, Path, Pattern, Shared, StructType, Type, - TypeBinding, TypeVariable, UnresolvedType, ERROR_IDENT, + TypeBinding, TypeVariable, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, }; use fm::FileId; use iter_extended::vecmap; @@ -302,6 +302,7 @@ impl<'a> Resolver<'a> { let elem = Box::new(self.resolve_type_inner(*elem, new_variables)); Type::Array(Box::new(resolved_size), elem) } + UnresolvedType::Expression(expr) => self.convert_expression_type(expr), UnresolvedType::Integer(comptime, sign, bits) => Type::Integer(comptime, sign, bits), UnresolvedType::Bool(comptime) => Type::Bool(comptime), UnresolvedType::String(size) => { @@ -313,7 +314,7 @@ impl<'a> Resolver<'a> { UnresolvedType::Error => Type::Error, UnresolvedType::Named(path, args) => { // Check if the path is a type variable first. We currently disallow generics on type - // variables since this is what rust does. + // variables since we do not support higher-kinded types. if args.is_empty() && path.segments.len() == 1 { let name = &path.last_segment().0.contents; if let Some((name, (var, _))) = self.generics.get_key_value(name) { @@ -342,10 +343,10 @@ impl<'a> Resolver<'a> { fn resolve_array_size( &mut self, - size: Option, + length: Option, new_variables: &mut Generics, ) -> Type { - match &size { + match length { None => { let id = self.interner.next_type_variable_id(); let typevar = Shared::new(TypeBinding::Unbound(id)); @@ -356,9 +357,47 @@ impl<'a> Resolver<'a> { // require users to explicitly be generic over array lengths. Type::NamedGeneric(typevar, Rc::new("".into())) } - Some(expr) => { - let len = self.eval_array_length(expr); - Type::ArrayLength(len) + Some(length) => self.convert_expression_type(length), + } + } + + fn convert_expression_type(&mut self, length: UnresolvedTypeExpression) -> Type { + match length { + UnresolvedTypeExpression::Variable(path) => { + if path.segments.len() == 1 { + let name = &path.last_segment().0.contents; + if let Some((name, (var, _))) = self.generics.get_key_value(name) { + return Type::NamedGeneric(var.clone(), name.clone()); + } + } + + // If we cannot find a local generic of the same name, try to look up a global + if let Ok(ModuleDefId::GlobalId(id)) = + self.path_resolver.resolve(self.def_maps, path.clone()) + { + Type::Constant(self.eval_global_as_array_length(id)) + } else { + self.push_err(ResolverError::NoSuchNumericTypeVariable { path }); + Type::Constant(0) + } + } + UnresolvedTypeExpression::Constant(int, _) => Type::Constant(int), + UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, _) => { + let (lhs_span, rhs_span) = (lhs.span(), rhs.span()); + let lhs = self.convert_expression_type(*lhs); + let rhs = self.convert_expression_type(*rhs); + + match (lhs, rhs) { + (Type::Constant(lhs), Type::Constant(rhs)) => { + Type::Constant(op.function()(lhs, rhs)) + } + (lhs, _) => { + let span = + if !matches!(lhs, Type::Constant(_)) { lhs_span } else { rhs_span }; + self.push_err(ResolverError::InvalidArrayLengthExpr { span }); + Type::Constant(0) + } + } } } } @@ -914,11 +953,32 @@ impl<'a> Resolver<'a> { } fn eval_array_length(&mut self, length: &Expression) -> u64 { - match self.try_eval_array_length(length).map(|length| length.try_into()) { - Ok(Ok(length_value)) => return length_value, - Ok(Err(_cast_err)) => { - self.push_err(ResolverError::IntegerTooLarge { span: length.span }) + let result = self.try_eval_array_length(length); + self.unwrap_array_length_eval_result(result, length.span) + } + + fn eval_global_as_array_length(&mut self, global: StmtId) -> u64 { + let stmt = match self.interner.statement(&global) { + HirStatement::Let(let_expr) => let_expr, + other => { + unreachable!("Expected global while evaluating array length, found {:?}", other) } + }; + + let length = stmt.expression; + let span = self.interner.expr_span(&length); + let result = self.try_eval_array_length_id(length, span); + self.unwrap_array_length_eval_result(result, span) + } + + fn unwrap_array_length_eval_result( + &mut self, + result: Result>, + span: Span, + ) -> u64 { + match result.map(|length| length.try_into()) { + Ok(Ok(length_value)) => return length_value, + Ok(Err(_cast_err)) => self.push_err(ResolverError::IntegerTooLarge { span }), Err(Some(error)) => self.push_err(error), Err(None) => (), } diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index c139cf3531b..b5ad956ed45 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -31,16 +31,13 @@ pub(crate) fn type_check_expression( HirExpression::Literal(literal) => { match literal { HirLiteral::Array(arr) => { - // Type check the contents of the array let elem_types = vecmap(&arr, |arg| type_check_expression(interner, arg, errors)); let first_elem_type = elem_types.get(0).cloned().unwrap_or(Type::Error); - // Specify the type of the Array - // Note: This assumes that the array is homogeneous, which will be checked next let arr_type = Type::Array( - Box::new(Type::ArrayLength(arr.len() as u64)), + Box::new(Type::Constant(arr.len() as u64)), Box::new(first_elem_type.clone()), ); @@ -72,7 +69,8 @@ pub(crate) fn type_check_expression( ) } HirLiteral::Str(string) => { - Type::String(Box::new(Type::ArrayLength(string.len() as u64))) + let len = Type::Constant(string.len() as u64); + Type::String(Box::new(len)) } } } @@ -783,7 +781,7 @@ pub fn comparator_operand_type_rules( x_size.unify(y_size, op.location.span, errors, || { TypeCheckError::Unstructured { - msg: format!("Can only compare arrays of the same length. Here LHS is of length {x_size}, and RHS is {y_size} "), + msg: format!("Can only compare arrays of the same length. Here LHS is of length {x_size}, and RHS is {y_size}"), span: op.location.span, } }); diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index a127d583ad6..2f7bdd250df 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -217,11 +217,22 @@ pub enum Type { /// A type-level integer. Included to let an Array's size type variable /// bind to an integer without special checks to bind it to a non-type. - ArrayLength(u64), + Constant(u64), Error, } +/// A restricted subset of binary operators useable on +/// type level integers for use in the array length positions of types. +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum BinaryTypeOperator { + Addition, + Subtraction, + Multiplication, + Division, + Modulo, +} + pub type TypeVariable = Shared; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -230,6 +241,12 @@ pub enum TypeBinding { Unbound(TypeVariableId), } +impl TypeBinding { + pub fn is_unbound(&self) -> bool { + matches!(self, TypeBinding::Unbound(_)) + } +} + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct TypeVariableId(pub usize); @@ -469,10 +486,7 @@ impl std::fmt::Display for Type { Type::FieldElement(comptime) => { write!(f, "{comptime}Field") } - Type::Array(len, typ) => match len.array_length() { - Some(len) => write!(f, "[{typ}; {len}]"), - None => write!(f, "[{typ}]"), - }, + Type::Array(len, typ) => write!(f, "[{typ}; {len}]"), Type::Integer(comptime, sign, num_bits) => match sign { Signedness::Signed => write!(f, "{comptime}i{num_bits}"), Signedness::Unsigned => write!(f, "{comptime}u{num_bits}"), @@ -500,10 +514,7 @@ impl std::fmt::Display for Type { write!(f, "({})", elements.join(", ")) } Type::Bool(comptime) => write!(f, "{comptime}bool"), - Type::String(len) => match len.array_length() { - Some(len) => write!(f, "str[{len}]"), - None => write!(f, "str[]]"), - }, + Type::String(len) => write!(f, "str<{len}>"), Type::Unit => write!(f, "()"), Type::Error => write!(f, "error"), Type::TypeVariable(id) => write!(f, "{}", id.borrow()), @@ -512,7 +523,7 @@ impl std::fmt::Display for Type { TypeBinding::Unbound(_) if name.is_empty() => write!(f, "_"), TypeBinding::Unbound(_) => write!(f, "{name}"), }, - Type::ArrayLength(n) => n.fmt(f), + Type::Constant(x) => x.fmt(f), Type::Forall(typevars, typ) => { let typevars = vecmap(typevars, |(var, _)| var.to_string()); write!(f, "forall {}. {}", typevars.join(" "), typ) @@ -525,6 +536,18 @@ impl std::fmt::Display for Type { } } +impl std::fmt::Display for BinaryTypeOperator { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + BinaryTypeOperator::Addition => write!(f, "+"), + BinaryTypeOperator::Subtraction => write!(f, "-"), + BinaryTypeOperator::Multiplication => write!(f, "*"), + BinaryTypeOperator::Division => write!(f, "/"), + BinaryTypeOperator::Modulo => write!(f, "%"), + } + } +} + impl std::fmt::Display for TypeVariableId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "_") @@ -664,7 +687,7 @@ impl Type { TypeBinding::Unbound(id) => *id, }; - if let Type::TypeVariable(binding) = self { + if let Some(binding) = self.get_inner_typevariable() { match &*binding.borrow() { TypeBinding::Bound(typ) => return typ.try_bind_to(var), // Don't recursively bind the same id to itself @@ -683,6 +706,15 @@ impl Type { } } + fn get_inner_typevariable(&self) -> Option> { + match self { + Type::PolymorphicInteger(_, var) + | Type::TypeVariable(var) + | Type::NamedGeneric(var, _) => Some(var.clone()), + _ => None, + } + } + fn is_comptime(&self) -> bool { match self { Type::FieldElement(comptime) => comptime.is_comptime(), @@ -758,7 +790,7 @@ impl Type { return link.try_unify(other, span); } - Ok(()) + other.try_bind_to(binding) } (Array(len_a, elem_a), Array(len_b, elem_b)) => { @@ -806,12 +838,10 @@ impl Type { (Bool(comptime_a), Bool(comptime_b)) => comptime_a.unify(comptime_b, span), (NamedGeneric(binding_a, name_a), NamedGeneric(binding_b, name_b)) => { - let is_unbound = |binding: &Shared| { - matches!(&*binding.borrow(), TypeBinding::Unbound(_)) - }; - // Ensure NamedGenerics are never bound during type checking - assert!(is_unbound(binding_a) && is_unbound(binding_b)); + assert!(binding_a.borrow().is_unbound()); + assert!(binding_b.borrow().is_unbound()); + if name_a == name_b { Ok(()) } else { @@ -825,7 +855,6 @@ impl Type { a.try_unify(b, span)?; } - // return types are contravariant, so this must be ret_b <: ret_a instead of the reverse ret_b.try_unify(ret_a, span) } else { Err(SpanKind::None) @@ -842,8 +871,8 @@ impl Type { } } - /// The `subtype` term here is somewhat loose, the only subtyping relations remaining are - /// between fixed and variable sized arrays, and Comptime tracking. + /// The `subtype` term here is somewhat loose, the only subtyping relations remaining + /// have to do with Comptime tracking. pub fn make_subtype_of( &self, expected: &Type, @@ -939,12 +968,10 @@ impl Type { (Bool(comptime_a), Bool(comptime_b)) => comptime_a.is_subtype_of(comptime_b, span), (NamedGeneric(binding_a, name_a), NamedGeneric(binding_b, name_b)) => { - let is_unbound = |binding: &Shared| { - matches!(&*binding.borrow(), TypeBinding::Unbound(_)) - }; - // Ensure NamedGenerics are never bound during type checking - assert!(is_unbound(binding_a) && is_unbound(binding_b)); + assert!(binding_a.borrow().is_unbound()); + assert!(binding_b.borrow().is_unbound()); + if name_a == name_b { Ok(()) } else { @@ -975,16 +1002,16 @@ impl Type { } } - pub fn array_length(&self) -> Option { + pub fn evaluate_to_u64(&self) -> Option { match self { Type::PolymorphicInteger(_, binding) | Type::NamedGeneric(binding, _) | Type::TypeVariable(binding) => match &*binding.borrow() { - TypeBinding::Bound(binding) => binding.array_length(), + TypeBinding::Bound(binding) => binding.evaluate_to_u64(), TypeBinding::Unbound(_) => None, }, - Type::Array(len, _elem) => len.array_length(), - Type::ArrayLength(size) => Some(*size), + Type::Array(len, _elem) => len.evaluate_to_u64(), + Type::Constant(x) => Some(*x), _ => None, } } @@ -995,10 +1022,10 @@ impl Type { match self { Type::FieldElement(_) => AbiType::Field, Type::Array(size, typ) => { - let size = size - .array_length() + let length = size + .evaluate_to_u64() .expect("Cannot have variable sized arrays as a parameter to main"); - AbiType::Array { length: size, typ: Box::new(typ.as_abi_type()) } + AbiType::Array { length, typ: Box::new(typ.as_abi_type()) } } Type::Integer(_, sign, bit_width) => { let sign = match sign { @@ -1015,13 +1042,13 @@ impl Type { Type::Bool(_) => AbiType::Boolean, Type::String(size) => { let size = size - .array_length() + .evaluate_to_u64() .expect("Cannot have variable sized strings as a parameter to main"); AbiType::String { length: size } } Type::Error => unreachable!(), Type::Unit => unreachable!(), - Type::ArrayLength(_) => unreachable!(), + Type::Constant(_) => unreachable!(), Type::Struct(def, args) => { let struct_type = def.borrow(); let fields = struct_type.get_fields(args); @@ -1150,7 +1177,7 @@ impl Type { Type::FieldElement(_) | Type::Integer(_, _, _) | Type::Bool(_) - | Type::ArrayLength(_) + | Type::Constant(_) | Type::Error | Type::Unit => self.clone(), } @@ -1180,7 +1207,7 @@ impl Type { Type::FieldElement(_) | Type::Integer(_, _, _) | Type::Bool(_) - | Type::ArrayLength(_) + | Type::Constant(_) | Type::Error | Type::Unit => false, } @@ -1221,9 +1248,22 @@ impl Type { // Expect that this function should only be called on instantiated types Forall(..) => unreachable!(), - FieldElement(_) | Integer(_, _, _) | Bool(_) | ArrayLength(_) | Unit | Error => { + FieldElement(_) | Integer(_, _, _) | Bool(_) | Constant(_) | Unit | Error => { self.clone() } } } } + +impl BinaryTypeOperator { + /// Return the actual rust numeric function associated with this operator + pub fn function(self) -> fn(u64, u64) -> u64 { + match self { + BinaryTypeOperator::Addition => |a, b| a.wrapping_add(b), + BinaryTypeOperator::Subtraction => |a, b| a.wrapping_sub(b), + BinaryTypeOperator::Multiplication => |a, b| a.wrapping_mul(b), + BinaryTypeOperator::Division => |a, b| a.wrapping_div(b), + BinaryTypeOperator::Modulo => |a, b| a.wrapping_rem(b), // % b, + } + } +} diff --git a/crates/noirc_frontend/src/monomorphisation/mod.rs b/crates/noirc_frontend/src/monomorphisation/mod.rs index 72a4ea83495..2f640866884 100644 --- a/crates/noirc_frontend/src/monomorphisation/mod.rs +++ b/crates/noirc_frontend/src/monomorphisation/mod.rs @@ -504,14 +504,11 @@ impl Monomorphiser { HirType::FieldElement(_) => ast::Type::Field, HirType::Integer(_, sign, bits) => ast::Type::Integer(*sign, *bits), HirType::Bool(_) => ast::Type::Bool, - HirType::String(size) => { - let size = size.array_length().unwrap_or(0); - ast::Type::String(size) - } + HirType::String(size) => ast::Type::String(size.evaluate_to_u64().unwrap_or(0)), HirType::Unit => ast::Type::Unit, HirType::Array(size, element) => { - let size = size.array_length().unwrap_or(0); + let size = size.evaluate_to_u64().unwrap_or(0); let element = Self::convert_type(element.as_ref()); ast::Type::Array(size, Box::new(element)) } @@ -552,7 +549,7 @@ impl Monomorphiser { ast::Type::Function(args, ret) } - HirType::Forall(_, _) | HirType::ArrayLength(_) | HirType::Error => { + HirType::Forall(_, _) | HirType::Constant(_) | HirType::Error => { unreachable!("Unexpected type {} found", typ) } } @@ -592,7 +589,7 @@ impl Monomorphiser { ast::Expression::Ident(ident) => match &ident.definition { Definition::Builtin(opcode) if opcode == "arraylen" => { let typ = self.interner.id_type(arguments[0]); - let len = typ.array_length().unwrap(); + let len = typ.evaluate_to_u64().unwrap(); Some(ast::Expression::Literal(ast::Literal::Integer( (len as u128).into(), ast::Type::Field, diff --git a/crates/noirc_frontend/src/parser/mod.rs b/crates/noirc_frontend/src/parser/mod.rs index c8c66114dac..7b57018d533 100644 --- a/crates/noirc_frontend/src/parser/mod.rs +++ b/crates/noirc_frontend/src/parser/mod.rs @@ -292,6 +292,7 @@ impl Precedence { Some(precedence) } + /// Return the next higher precedence. E.g. `Sum.next() == Product` fn next(self) -> Self { use Precedence::*; match self { @@ -307,7 +308,7 @@ impl Precedence { } } - /// Type expressions only contain basic arithmetic operators and + /// TypeExpressions only contain basic arithmetic operators and /// notably exclude `>` due to parsing conflicts with generic type brackets. fn next_type_precedence(self) -> Self { use Precedence::*; diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 9b02fb58ecf..22e0a352d83 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -10,7 +10,7 @@ use crate::token::{Attribute, Keyword, Token, TokenKind}; use crate::{ BinaryOp, BinaryOpKind, BlockExpression, Comptime, ConstrainStatement, FunctionDefinition, Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda, NoirFunction, NoirImpl, - NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, + NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, UnresolvedTypeExpression, }; use chumsky::prelude::*; @@ -95,7 +95,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser { .then_ignore(keyword(Keyword::Fn)) .then(ident()) .then(generics()) - .then(parenthesized(function_parameters(allow_self, expression()))) + .then(parenthesized(function_parameters(allow_self))) .then(function_return_type()) .then(block(expression())) .map( @@ -132,14 +132,14 @@ fn struct_definition() -> impl NoirParser { use self::Keyword::Struct; use Token::*; - let fields = struct_fields(expression()) - .delimited_by(just(LeftBrace), just(RightBrace)) - .recover_with(nested_delimiters( + let fields = struct_fields().delimited_by(just(LeftBrace), just(RightBrace)).recover_with( + nested_delimiters( LeftBrace, RightBrace, [(LeftParen, RightParen), (LeftBracket, RightBracket)], |_| vec![], - )); + ), + ); keyword(Struct).ignore_then(ident()).then(generics()).then(fields).map_with_span( |((name, generics), fields), span| { @@ -148,19 +148,17 @@ fn struct_definition() -> impl NoirParser { ) } -fn lambda_return_type<'a>( - expr_parser: impl NoirParser + 'a, -) -> impl NoirParser + 'a { +fn lambda_return_type() -> impl NoirParser { just(Token::Arrow) - .ignore_then(parse_type(expr_parser)) + .ignore_then(parse_type()) .or_not() .map(|ret| ret.unwrap_or(UnresolvedType::Unspecified)) } -fn function_return_type<'a>() -> impl NoirParser<(AbiVisibility, UnresolvedType)> + 'a { +fn function_return_type() -> impl NoirParser<(AbiVisibility, UnresolvedType)> { just(Token::Arrow) .ignore_then(optional_visibility()) - .then(parse_type(expression())) + .then(parse_type()) .or_not() .map(|ret| ret.unwrap_or((AbiVisibility::Private, UnresolvedType::Unit))) } @@ -172,20 +170,16 @@ fn attribute() -> impl NoirParser { }) } -fn struct_fields<'a>( - expr_parser: impl NoirParser + 'a, -) -> impl NoirParser> + 'a { +fn struct_fields() -> impl NoirParser> { ident() .then_ignore(just(Token::Colon)) - .then(parse_type(expr_parser)) + .then(parse_type()) .separated_by(just(Token::Comma)) .allow_trailing() } -fn lambda_parameters<'a>( - expr_parser: impl NoirParser + 'a, -) -> impl NoirParser> + 'a { - let typ = parse_type(expr_parser).recover_via(parameter_recovery()); +fn lambda_parameters() -> impl NoirParser> { + let typ = parse_type().recover_via(parameter_recovery()); let typ = just(Token::Colon).ignore_then(typ); let parameter = pattern() @@ -197,9 +191,8 @@ fn lambda_parameters<'a>( fn function_parameters<'a>( allow_self: bool, - expr_parser: impl NoirParser + 'a, ) -> impl NoirParser> + 'a { - let typ = parse_type(expr_parser).recover_via(parameter_recovery()); + let typ = parse_type().recover_via(parameter_recovery()); let full_parameter = pattern() .recover_via(parameter_name_recovery()) @@ -282,7 +275,7 @@ fn check_statements_require_semicolon( /// Parse an optional ': type' and implicitly add a 'comptime' to the type fn global_type_annotation() -> impl NoirParser { - ignore_then_commit(just(Token::Colon), parse_type(expression())) + ignore_then_commit(just(Token::Colon), parse_type()) .map(|r#type| match r#type { UnresolvedType::FieldElement(_) => UnresolvedType::FieldElement(Comptime::Yes(None)), UnresolvedType::Bool(_) => UnresolvedType::Bool(Comptime::Yes(None)), @@ -295,10 +288,8 @@ fn global_type_annotation() -> impl NoirParser { .map(|opt| opt.unwrap_or(UnresolvedType::Unspecified)) } -fn optional_type_annotation<'a>( - expr_parser: impl NoirParser + 'a, -) -> impl NoirParser + 'a { - ignore_then_commit(just(Token::Colon), parse_type(expr_parser)) +fn optional_type_annotation<'a>() -> impl NoirParser + 'a { + ignore_then_commit(just(Token::Colon), parse_type()) .or_not() .map(|r#type| r#type.unwrap_or(UnresolvedType::Unspecified)) } @@ -373,7 +364,7 @@ where P: ExprParser + 'a, { let p = ignore_then_commit(keyword(Keyword::Let).labelled("statement"), pattern()); - let p = p.then(optional_type_annotation(expr_parser.clone())); + let p = p.then(optional_type_annotation()); let p = then_commit_ignore(p, just(Token::Assign)); let p = then_commit(p, expr_parser); p.map(Statement::new_let) @@ -453,26 +444,18 @@ where }) } -fn parse_type<'a, P>(expr_parser: P) -> impl NoirParser + 'a -where - P: NoirParser + 'a, -{ - recursive(move |typ| parse_type_inner(typ, expr_parser)) +fn parse_type<'a>() -> impl NoirParser + 'a { + recursive(parse_type_inner) } -fn parse_type_inner( - recursive_type_parser: T, - expr_parser: P, -) -> impl NoirParser -where - T: NoirParser, - P: NoirParser, -{ +fn parse_type_inner( + recursive_type_parser: impl NoirParser, +) -> impl NoirParser { choice(( field_type(), int_type(), named_type(recursive_type_parser.clone()), - array_type(recursive_type_parser.clone(), expr_parser), + array_type(recursive_type_parser.clone()), tuple_type(recursive_type_parser.clone()), bool_type(), string_type(), @@ -531,6 +514,12 @@ fn generic_type_args( type_parser: impl NoirParser, ) -> impl NoirParser> { type_parser + // Without checking for a terminating ',' or '>' here we may incorrectly + // parse a generic `N * 2` as just the type `N` then fail when there is no + // separator afterward. Failing early here ensures we try the `type_expression` + // parser afterward. + .then_ignore(one_of([Token::Comma, Token::Greater]).rewind()) + .or(type_expression().map(UnresolvedType::Expression)) .separated_by(just(Token::Comma)) .allow_trailing() .at_least(1) @@ -539,23 +528,20 @@ fn generic_type_args( .map(Option::unwrap_or_default) } -fn type_expression() -> impl NoirParser { - recursive(|expr| expression_with_precedence(Precedence::lowest_type_precedence(), expr, true)) - .labelled("type expression") -} - -fn array_type(type_parser: T, expr_parser: P) -> impl NoirParser -where - T: NoirParser, - P: NoirParser, -{ +fn array_type(type_parser: impl NoirParser) -> impl NoirParser { just(Token::LeftBracket) .ignore_then(type_parser) - .then(just(Token::Semicolon).ignore_then(expr_parser).or_not()) + .then(just(Token::Semicolon).ignore_then(type_expression()).or_not()) .then_ignore(just(Token::RightBracket)) .map(|(element_type, size)| UnresolvedType::Array(size, Box::new(element_type))) } +fn type_expression() -> impl NoirParser { + recursive(|expr| expression_with_precedence(Precedence::lowest_type_precedence(), expr, true)) + .labelled("type expression") + .try_map(UnresolvedTypeExpression::from_expr) +} + fn tuple_type(type_parser: T) -> impl NoirParser where T: NoirParser, @@ -680,10 +666,8 @@ where .map(UnaryRhs::ArrayIndex); // `as Type` in `atom as Type` - let cast_rhs = keyword(Keyword::As) - .ignore_then(parse_type(expr_parser.clone())) - .map(UnaryRhs::Cast) - .labelled("cast"); + let cast_rhs = + keyword(Keyword::As).ignore_then(parse_type()).map(UnaryRhs::Cast).labelled("cast"); // `.foo` or `.foo(args)` in `atom.foo` or `atom.foo(args)` let member_rhs = just(Token::Dot) @@ -731,9 +715,9 @@ where fn lambda<'a>( expr_parser: impl NoirParser + 'a, ) -> impl NoirParser + 'a { - lambda_parameters(expr_parser.clone()) + lambda_parameters() .delimited_by(just(Token::Pipe), just(Token::Pipe)) - .then(lambda_return_type(expr_parser.clone())) + .then(lambda_return_type()) .then(expr_parser) .map(|((parameters, return_type), body)| { ExpressionKind::Lambda(Box::new(Lambda { parameters, return_type, body })) diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index 5a90df2069c..40e9ef4511e 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -27,110 +27,111 @@ fn mimc(x: Field, k: Field, constants: [Field], exp : Field) -> Field { h + k } +global MIMC_BN254_ROUNDS = 91; //mimc implementation with hardcoded parameters for BN254 curve. fn mimc_bn254(x: [Field]) -> Field { //mimc parameters - let ROUNDS: Field = 91; let exponent = 7; //generated from seed "mimc" using keccak256 - let constants :[Field; ROUNDS] = [0, - 20888961410941983456478427210666206549300505294776164667214940546594746570981, -15265126113435022738560151911929040668591755459209400716467504685752745317193, -8334177627492981984476504167502758309043212251641796197711684499645635709656, -1374324219480165500871639364801692115397519265181803854177629327624133579404, -11442588683664344394633565859260176446561886575962616332903193988751292992472, -2558901189096558760448896669327086721003508630712968559048179091037845349145, -11189978595292752354820141775598510151189959177917284797737745690127318076389, -3262966573163560839685415914157855077211340576201936620532175028036746741754, -17029914891543225301403832095880481731551830725367286980611178737703889171730, -4614037031668406927330683909387957156531244689520944789503628527855167665518, -19647356996769918391113967168615123299113119185942498194367262335168397100658, -5040699236106090655289931820723926657076483236860546282406111821875672148900, -2632385916954580941368956176626336146806721642583847728103570779270161510514, -17691411851977575435597871505860208507285462834710151833948561098560743654671, -11482807709115676646560379017491661435505951727793345550942389701970904563183, -8360838254132998143349158726141014535383109403565779450210746881879715734773, -12663821244032248511491386323242575231591777785787269938928497649288048289525, -3067001377342968891237590775929219083706800062321980129409398033259904188058, -8536471869378957766675292398190944925664113548202769136103887479787957959589, -19825444354178182240559170937204690272111734703605805530888940813160705385792, -16703465144013840124940690347975638755097486902749048533167980887413919317592, -13061236261277650370863439564453267964462486225679643020432589226741411380501, -10864774797625152707517901967943775867717907803542223029967000416969007792571, -10035653564014594269791753415727486340557376923045841607746250017541686319774, -3446968588058668564420958894889124905706353937375068998436129414772610003289, -4653317306466493184743870159523234588955994456998076243468148492375236846006, -8486711143589723036499933521576871883500223198263343024003617825616410932026, -250710584458582618659378487568129931785810765264752039738223488321597070280, -2104159799604932521291371026105311735948154964200596636974609406977292675173, -16313562605837709339799839901240652934758303521543693857533755376563489378839, -6032365105133504724925793806318578936233045029919447519826248813478479197288, -14025118133847866722315446277964222215118620050302054655768867040006542798474, -7400123822125662712777833064081316757896757785777291653271747396958201309118, -1744432620323851751204287974553233986555641872755053103823939564833813704825, -8316378125659383262515151597439205374263247719876250938893842106722210729522, -6739722627047123650704294650168547689199576889424317598327664349670094847386, -21211457866117465531949733809706514799713333930924902519246949506964470524162, -13718112532745211817410303291774369209520657938741992779396229864894885156527, -5264534817993325015357427094323255342713527811596856940387954546330728068658, -18884137497114307927425084003812022333609937761793387700010402412840002189451, -5148596049900083984813839872929010525572543381981952060869301611018636120248, -19799686398774806587970184652860783461860993790013219899147141137827718662674, -19240878651604412704364448729659032944342952609050243268894572835672205984837, -10546185249390392695582524554167530669949955276893453512788278945742408153192, -5507959600969845538113649209272736011390582494851145043668969080335346810411, -18177751737739153338153217698774510185696788019377850245260475034576050820091, -19603444733183990109492724100282114612026332366576932662794133334264283907557, -10548274686824425401349248282213580046351514091431715597441736281987273193140, -1823201861560942974198127384034483127920205835821334101215923769688644479957, -11867589662193422187545516240823411225342068709600734253659804646934346124945, -18718569356736340558616379408444812528964066420519677106145092918482774343613, -10530777752259630125564678480897857853807637120039176813174150229243735996839, -20486583726592018813337145844457018474256372770211860618687961310422228379031, -12690713110714036569415168795200156516217175005650145422920562694422306200486, -17386427286863519095301372413760745749282643730629659997153085139065756667205, -2216432659854733047132347621569505613620980842043977268828076165669557467682, -6309765381643925252238633914530877025934201680691496500372265330505506717193, -20806323192073945401862788605803131761175139076694468214027227878952047793390, -4037040458505567977365391535756875199663510397600316887746139396052445718861, -19948974083684238245321361840704327952464170097132407924861169241740046562673, -845322671528508199439318170916419179535949348988022948153107378280175750024, -16222384601744433420585982239113457177459602187868460608565289920306145389382, -10232118865851112229330353999139005145127746617219324244541194256766741433339, -6699067738555349409504843460654299019000594109597429103342076743347235369120, -6220784880752427143725783746407285094967584864656399181815603544365010379208, -6129250029437675212264306655559561251995722990149771051304736001195288083309, -10773245783118750721454994239248013870822765715268323522295722350908043393604, -4490242021765793917495398271905043433053432245571325177153467194570741607167, -19596995117319480189066041930051006586888908165330319666010398892494684778526, -837850695495734270707668553360118467905109360511302468085569220634750561083, -11803922811376367215191737026157445294481406304781326649717082177394185903907, -10201298324909697255105265958780781450978049256931478989759448189112393506592, -13564695482314888817576351063608519127702411536552857463682060761575100923924, -9262808208636973454201420823766139682381973240743541030659775288508921362724, -173271062536305557219323722062711383294158572562695717740068656098441040230, -18120430890549410286417591505529104700901943324772175772035648111937818237369, -20484495168135072493552514219686101965206843697794133766912991150184337935627, -19155651295705203459475805213866664350848604323501251939850063308319753686505, -11971299749478202793661982361798418342615500543489781306376058267926437157297, -18285310723116790056148596536349375622245669010373674803854111592441823052978, -7069216248902547653615508023941692395371990416048967468982099270925308100727, -6465151453746412132599596984628739550147379072443683076388208843341824127379, -16143532858389170960690347742477978826830511669766530042104134302796355145785, -19362583304414853660976404410208489566967618125972377176980367224623492419647, -1702213613534733786921602839210290505213503664731919006932367875629005980493, -10781825404476535814285389902565833897646945212027592373510689209734812292327, -4212716923652881254737947578600828255798948993302968210248673545442808456151, -7594017890037021425366623750593200398174488805473151513558919864633711506220, -18979889247746272055963929241596362599320706910852082477600815822482192194401, -13602139229813231349386885113156901793661719180900395818909719758150455500533,]; + let constants: [Field; MIMC_BN254_ROUNDS] = [ + 0, + 20888961410941983456478427210666206549300505294776164667214940546594746570981, + 15265126113435022738560151911929040668591755459209400716467504685752745317193, + 8334177627492981984476504167502758309043212251641796197711684499645635709656, + 1374324219480165500871639364801692115397519265181803854177629327624133579404, + 11442588683664344394633565859260176446561886575962616332903193988751292992472, + 2558901189096558760448896669327086721003508630712968559048179091037845349145, + 11189978595292752354820141775598510151189959177917284797737745690127318076389, + 3262966573163560839685415914157855077211340576201936620532175028036746741754, + 17029914891543225301403832095880481731551830725367286980611178737703889171730, + 4614037031668406927330683909387957156531244689520944789503628527855167665518, + 19647356996769918391113967168615123299113119185942498194367262335168397100658, + 5040699236106090655289931820723926657076483236860546282406111821875672148900, + 2632385916954580941368956176626336146806721642583847728103570779270161510514, + 17691411851977575435597871505860208507285462834710151833948561098560743654671, + 11482807709115676646560379017491661435505951727793345550942389701970904563183, + 8360838254132998143349158726141014535383109403565779450210746881879715734773, + 12663821244032248511491386323242575231591777785787269938928497649288048289525, + 3067001377342968891237590775929219083706800062321980129409398033259904188058, + 8536471869378957766675292398190944925664113548202769136103887479787957959589, + 19825444354178182240559170937204690272111734703605805530888940813160705385792, + 16703465144013840124940690347975638755097486902749048533167980887413919317592, + 13061236261277650370863439564453267964462486225679643020432589226741411380501, + 10864774797625152707517901967943775867717907803542223029967000416969007792571, + 10035653564014594269791753415727486340557376923045841607746250017541686319774, + 3446968588058668564420958894889124905706353937375068998436129414772610003289, + 4653317306466493184743870159523234588955994456998076243468148492375236846006, + 8486711143589723036499933521576871883500223198263343024003617825616410932026, + 250710584458582618659378487568129931785810765264752039738223488321597070280, + 2104159799604932521291371026105311735948154964200596636974609406977292675173, + 16313562605837709339799839901240652934758303521543693857533755376563489378839, + 6032365105133504724925793806318578936233045029919447519826248813478479197288, + 14025118133847866722315446277964222215118620050302054655768867040006542798474, + 7400123822125662712777833064081316757896757785777291653271747396958201309118, + 1744432620323851751204287974553233986555641872755053103823939564833813704825, + 8316378125659383262515151597439205374263247719876250938893842106722210729522, + 6739722627047123650704294650168547689199576889424317598327664349670094847386, + 21211457866117465531949733809706514799713333930924902519246949506964470524162, + 13718112532745211817410303291774369209520657938741992779396229864894885156527, + 5264534817993325015357427094323255342713527811596856940387954546330728068658, + 18884137497114307927425084003812022333609937761793387700010402412840002189451, + 5148596049900083984813839872929010525572543381981952060869301611018636120248, + 19799686398774806587970184652860783461860993790013219899147141137827718662674, + 19240878651604412704364448729659032944342952609050243268894572835672205984837, + 10546185249390392695582524554167530669949955276893453512788278945742408153192, + 5507959600969845538113649209272736011390582494851145043668969080335346810411, + 18177751737739153338153217698774510185696788019377850245260475034576050820091, + 19603444733183990109492724100282114612026332366576932662794133334264283907557, + 10548274686824425401349248282213580046351514091431715597441736281987273193140, + 1823201861560942974198127384034483127920205835821334101215923769688644479957, + 11867589662193422187545516240823411225342068709600734253659804646934346124945, + 18718569356736340558616379408444812528964066420519677106145092918482774343613, + 10530777752259630125564678480897857853807637120039176813174150229243735996839, + 20486583726592018813337145844457018474256372770211860618687961310422228379031, + 12690713110714036569415168795200156516217175005650145422920562694422306200486, + 17386427286863519095301372413760745749282643730629659997153085139065756667205, + 2216432659854733047132347621569505613620980842043977268828076165669557467682, + 6309765381643925252238633914530877025934201680691496500372265330505506717193, + 20806323192073945401862788605803131761175139076694468214027227878952047793390, + 4037040458505567977365391535756875199663510397600316887746139396052445718861, + 19948974083684238245321361840704327952464170097132407924861169241740046562673, + 845322671528508199439318170916419179535949348988022948153107378280175750024, + 16222384601744433420585982239113457177459602187868460608565289920306145389382, + 10232118865851112229330353999139005145127746617219324244541194256766741433339, + 6699067738555349409504843460654299019000594109597429103342076743347235369120, + 6220784880752427143725783746407285094967584864656399181815603544365010379208, + 6129250029437675212264306655559561251995722990149771051304736001195288083309, + 10773245783118750721454994239248013870822765715268323522295722350908043393604, + 4490242021765793917495398271905043433053432245571325177153467194570741607167, + 19596995117319480189066041930051006586888908165330319666010398892494684778526, + 837850695495734270707668553360118467905109360511302468085569220634750561083, + 11803922811376367215191737026157445294481406304781326649717082177394185903907, + 10201298324909697255105265958780781450978049256931478989759448189112393506592, + 13564695482314888817576351063608519127702411536552857463682060761575100923924, + 9262808208636973454201420823766139682381973240743541030659775288508921362724, + 173271062536305557219323722062711383294158572562695717740068656098441040230, + 18120430890549410286417591505529104700901943324772175772035648111937818237369, + 20484495168135072493552514219686101965206843697794133766912991150184337935627, + 19155651295705203459475805213866664350848604323501251939850063308319753686505, + 11971299749478202793661982361798418342615500543489781306376058267926437157297, + 18285310723116790056148596536349375622245669010373674803854111592441823052978, + 7069216248902547653615508023941692395371990416048967468982099270925308100727, + 6465151453746412132599596984628739550147379072443683076388208843341824127379, + 16143532858389170960690347742477978826830511669766530042104134302796355145785, + 19362583304414853660976404410208489566967618125972377176980367224623492419647, + 1702213613534733786921602839210290505213503664731919006932367875629005980493, + 10781825404476535814285389902565833897646945212027592373510689209734812292327, + 4212716923652881254737947578600828255798948993302968210248673545442808456151, + 7594017890037021425366623750593200398174488805473151513558919864633711506220, + 18979889247746272055963929241596362599320706910852082477600815822482192194401, + 13602139229813231349386885113156901793661719180900395818909719758150455500533, + ]; let mut r = 0; - for i in 0..crate::array::len(x) { + for i in 0 .. crate::array::len(x) { let h = mimc(x[i], r, constants, exponent); - r = r + x[i] +h; + r = r + x[i] + h; }; - r - -} \ No newline at end of file + r +} From a20e1c1cf26f6fbfbcafd35c0009803e66dc8354 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 31 Jan 2023 13:39:14 -0500 Subject: [PATCH 05/14] feat(std_lib)!: modulus bits/bytes methods, and to_bits -> to_le_bits (#697) * added modulus_bits command, have to add modulus * add modulus_bits and modulus_be_byte_array builtin funcs * cargo fm * cargo clippy * more clippy format fixes * slight comment fix * rename modulus to bytes methods * remove comments and dbg * rename to have le and be trailing func names * match rust with endianness before bytes/bits * tiny comments rename * to_le_bytes test naming * remove dbg! stmt --- .../tests/test_data/7_function/src/main.nr | 2 +- .../tests/test_data/9_conditional/src/main.nr | 4 +- .../nargo/tests/test_data/modulus/Nargo.toml | 5 +++ .../nargo/tests/test_data/modulus/Prover.toml | 3 ++ .../nargo/tests/test_data/modulus/src/main.nr | 27 ++++++++++++ .../tests/test_data/to_le_bytes/Prover.toml | 2 +- .../tests/test_data/to_le_bytes/src/main.nr | 2 +- crates/noirc_evaluator/src/ssa/builtin.rs | 4 +- crates/noirc_evaluator/src/ssa/optim.rs | 2 +- .../src/monomorphisation/mod.rs | 43 ++++++++++++++++++- noir_stdlib/src/field.nr | 26 +++++++++++ noir_stdlib/src/lib.nr | 18 +------- noir_stdlib/src/merkle.nr | 2 +- 13 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 crates/nargo/tests/test_data/modulus/Nargo.toml create mode 100644 crates/nargo/tests/test_data/modulus/Prover.toml create mode 100644 crates/nargo/tests/test_data/modulus/src/main.nr create mode 100644 noir_stdlib/src/field.nr diff --git a/crates/nargo/tests/test_data/7_function/src/main.nr b/crates/nargo/tests/test_data/7_function/src/main.nr index bc45610ff22..a57889838b7 100644 --- a/crates/nargo/tests/test_data/7_function/src/main.nr +++ b/crates/nargo/tests/test_data/7_function/src/main.nr @@ -33,7 +33,7 @@ fn test2(z: Field, t: u32 ) { fn pow(base: Field, exponent: Field) -> Field { let mut r = 1 as Field; - let b = std::to_bits(exponent, 32 as u32); + let b = std::field::to_le_bits(exponent, 32 as u32); for i in 1..33 { r = r*r; r = (b[32-i] as Field) * (r * base) + (1 - b[32-i] as Field) * r; diff --git a/crates/nargo/tests/test_data/9_conditional/src/main.nr b/crates/nargo/tests/test_data/9_conditional/src/main.nr index 8819955c758..6afc79d4be2 100644 --- a/crates/nargo/tests/test_data/9_conditional/src/main.nr +++ b/crates/nargo/tests/test_data/9_conditional/src/main.nr @@ -59,12 +59,12 @@ fn main(a: u32, mut c: [u32; 4], x: [u8; 5], result: pub [u8; 32]){ } } - //Regression for to_bits() constant evaluation + //Regression for to_le_bits() constant evaluation // binary array representation of u8 1 let as_bits_hardcode_1 = [1, 0]; let mut c1 = 0; for i in 0..2 { - let mut as_bits = std::to_bits(arr[i] as Field, 2); + let mut as_bits = std::field::to_le_bits(arr[i] as Field, 2); c1 = c1 + as_bits[0] as Field; if i == 0 { diff --git a/crates/nargo/tests/test_data/modulus/Nargo.toml b/crates/nargo/tests/test_data/modulus/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo/tests/test_data/modulus/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo/tests/test_data/modulus/Prover.toml b/crates/nargo/tests/test_data/modulus/Prover.toml new file mode 100644 index 00000000000..d46300b8a5b --- /dev/null +++ b/crates/nargo/tests/test_data/modulus/Prover.toml @@ -0,0 +1,3 @@ +bn254_modulus_be_bytes = [48, 100, 78, 114, 225, 49, 160, 41, 184, 80, 69, 182, 129, 129, 88, 93, 40, 51, 232, 72, 121, 185, 112, 145, 67, 225, 245, 147, 240, 0, 0, 1] +bn254_modulus_be_bits = [1, 1, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 0, 0, 1, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1, 1, 0, 1, 1, 0, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 1, 1, 0, 0, 0, 0, 1, 0, 1, 1, 1, 0, 1, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 1, 0, 1, 1, 0, 0, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] +return = "" \ No newline at end of file diff --git a/crates/nargo/tests/test_data/modulus/src/main.nr b/crates/nargo/tests/test_data/modulus/src/main.nr new file mode 100644 index 00000000000..070d934976d --- /dev/null +++ b/crates/nargo/tests/test_data/modulus/src/main.nr @@ -0,0 +1,27 @@ +use dep::std; + +fn main(bn254_modulus_be_bytes : [u8; 32], bn254_modulus_be_bits : [u1; 254]) -> pub Field { + let modulus_size = std::field::modulus_num_bits(); + // NOTE: The constraints used in this circuit will only work when testing nargo with the plonk bn254 backend + constrain modulus_size == 254; + + let modulus_be_byte_array = std::field::modulus_be_bytes(); + for i in 0..32 { + constrain modulus_be_byte_array[i] == bn254_modulus_be_bytes[i]; + } + let modulus_le_byte_array = std::field::modulus_le_bytes(); + for i in 0..32 { + constrain modulus_le_byte_array[i] == bn254_modulus_be_bytes[31-i]; + } + + let modulus_be_bits = std::field::modulus_be_bits(); + for i in 0..254 { + constrain modulus_be_bits[i] == bn254_modulus_be_bits[i]; + } + let modulus_le_bits = std::field::modulus_le_bits(); + for i in 0..254 { + constrain modulus_le_bits[i] == bn254_modulus_be_bits[253-i]; + } + + modulus_size +} \ No newline at end of file diff --git a/crates/nargo/tests/test_data/to_le_bytes/Prover.toml b/crates/nargo/tests/test_data/to_le_bytes/Prover.toml index 54cfe16a841..f5f4f367d1c 100644 --- a/crates/nargo/tests/test_data/to_le_bytes/Prover.toml +++ b/crates/nargo/tests/test_data/to_le_bytes/Prover.toml @@ -1,2 +1,2 @@ x = "2040124" -return = [0x3c, 0x21, 0x1f, 0x00] +return = [0x3c, 0x21, 0x1f, 0x00] \ No newline at end of file diff --git a/crates/nargo/tests/test_data/to_le_bytes/src/main.nr b/crates/nargo/tests/test_data/to_le_bytes/src/main.nr index f2fbb915ddd..2fb86170230 100644 --- a/crates/nargo/tests/test_data/to_le_bytes/src/main.nr +++ b/crates/nargo/tests/test_data/to_le_bytes/src/main.nr @@ -2,7 +2,7 @@ use dep::std; fn main(x : Field) -> pub [u8; 4] { // The result of this byte array will be little-endian - let byte_array = std::to_le_bytes(x, 31); + let byte_array = std::field::to_le_bytes(x, 31); let mut first_four_bytes = [0; 4]; for i in 0..4 { first_four_bytes[i] = byte_array[i]; diff --git a/crates/noirc_evaluator/src/ssa/builtin.rs b/crates/noirc_evaluator/src/ssa/builtin.rs index b6e4eebb772..23ae36d6133 100644 --- a/crates/noirc_evaluator/src/ssa/builtin.rs +++ b/crates/noirc_evaluator/src/ssa/builtin.rs @@ -20,7 +20,7 @@ impl std::fmt::Display for Opcode { impl Opcode { pub fn lookup(op_name: &str) -> Option { match op_name { - "to_bits" => Some(Opcode::ToBits), + "to_le_bits" => Some(Opcode::ToBits), "to_radix" => Some(Opcode::ToRadix), _ => BlackBoxFunc::lookup(op_name).map(Opcode::LowLevel), } @@ -29,7 +29,7 @@ impl Opcode { pub fn name(&self) -> &str { match self { Opcode::LowLevel(op) => op.name(), - Opcode::ToBits => "to_bits", + Opcode::ToBits => "to_le_bits", Opcode::ToRadix => "to_radix", } } diff --git a/crates/noirc_evaluator/src/ssa/optim.rs b/crates/noirc_evaluator/src/ssa/optim.rs index 72c7d629c83..edad49ce32e 100644 --- a/crates/noirc_evaluator/src/ssa/optim.rs +++ b/crates/noirc_evaluator/src/ssa/optim.rs @@ -362,7 +362,7 @@ fn cse_block_with_anchor( result?; } - //cannot simplify to_bits() in the previous call because it get replaced with multiple instructions + //cannot simplify to_le_bits() in the previous call because it get replaced with multiple instructions if let Operation::Intrinsic(opcode, args) = &update2.operation { let args = args.iter().map(|arg| { NodeEval::from_id(ctx, *arg).into_const_value().map(|f| f.to_u128()) diff --git a/crates/noirc_frontend/src/monomorphisation/mod.rs b/crates/noirc_frontend/src/monomorphisation/mod.rs index 2f640866884..d20a520c4f6 100644 --- a/crates/noirc_frontend/src/monomorphisation/mod.rs +++ b/crates/noirc_frontend/src/monomorphisation/mod.rs @@ -1,3 +1,4 @@ +use acvm::FieldElement; use iter_extended::{btree_map, vecmap}; use noirc_abi::Abi; use std::collections::{BTreeMap, HashMap, VecDeque}; @@ -574,7 +575,7 @@ impl Monomorphiser { })) } - /// Try to evaluate certain builtin functions (currently only 'arraylen') + /// Try to evaluate certain builtin functions (currently only 'arraylen' and field modulus methods) /// at their callsite. /// NOTE: Evaluating at the callsite means we cannot track aliased functions. /// E.g. `let f = std::array::len; f(arr)` will fail to evaluate. @@ -595,12 +596,52 @@ impl Monomorphiser { ast::Type::Field, ))) } + Definition::Builtin(opcode) if opcode == "modulus_num_bits" => { + Some(ast::Expression::Literal(ast::Literal::Integer( + (FieldElement::max_num_bits() as u128).into(), + ast::Type::Field, + ))) + } + Definition::Builtin(opcode) if opcode == "modulus_le_bits" => { + let modulus = FieldElement::modulus(); + let bits = modulus.to_radix_le(2); + Some(self.modulus_array_literal(bits, 1)) + } + Definition::Builtin(opcode) if opcode == "modulus_be_bits" => { + let modulus = FieldElement::modulus(); + let bits = modulus.to_radix_be(2); + Some(self.modulus_array_literal(bits, 1)) + } + Definition::Builtin(opcode) if opcode == "modulus_be_bytes" => { + let modulus = FieldElement::modulus(); + let bytes = modulus.to_bytes_be(); + Some(self.modulus_array_literal(bytes, 8)) + } + Definition::Builtin(opcode) if opcode == "modulus_le_bytes" => { + let modulus = FieldElement::modulus(); + let bytes = modulus.to_bytes_le(); + Some(self.modulus_array_literal(bytes, 8)) + } _ => None, }, _ => None, } } + fn modulus_array_literal(&self, bytes: Vec, arr_elem_bits: u32) -> ast::Expression { + let bytes_as_expr = vecmap(bytes, |byte| { + ast::Expression::Literal(ast::Literal::Integer( + (byte as u128).into(), + ast::Type::Integer(crate::Signedness::Unsigned, arr_elem_bits), + )) + }); + let arr_literal = ast::ArrayLiteral { + contents: bytes_as_expr, + element_type: ast::Type::Integer(crate::Signedness::Unsigned, arr_elem_bits), + }; + ast::Expression::Literal(ast::Literal::Array(arr_literal)) + } + fn queue_function( &mut self, id: node_interner::FuncId, diff --git a/noir_stdlib/src/field.nr b/noir_stdlib/src/field.nr new file mode 100644 index 00000000000..0e8a5637842 --- /dev/null +++ b/noir_stdlib/src/field.nr @@ -0,0 +1,26 @@ +#[builtin(to_le_bits)] +fn to_le_bits(_x : Field, _bit_size: u32) -> [u1] {} + +fn to_le_bytes(x : Field, byte_size: u32) -> [u8] { + to_radix(x, 256, byte_size) +} + +#[builtin(to_radix)] +//decompose _x into a _result_len vector over the _radix basis +//_radix must be less than 256 +fn to_radix(_x : Field, _radix: u32, _result_len: u32) -> [u8] {} + +#[builtin(modulus_num_bits)] +fn modulus_num_bits() -> comptime Field {} + +#[builtin(modulus_be_bits)] +fn modulus_be_bits() -> [u1] {} + +#[builtin(modulus_le_bits)] +fn modulus_le_bits() -> [u1] {} + +#[builtin(modulus_be_bytes)] +fn modulus_be_bytes() -> [u8] {} + +#[builtin(modulus_le_bytes)] +fn modulus_le_bytes() -> [u8] {} \ No newline at end of file diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index dd78ccc621c..3435dfc0f5a 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -6,21 +6,7 @@ mod ecdsa_secp256k1; mod scalar_mul; mod sha256; mod sha512; - - - #[builtin(to_bits)] -fn to_bits(_x : Field, _bit_size: u32) -> [u1] {} - -fn to_le_bytes(x : Field, byte_size: u32) -> [u8] { - to_radix(x, 256, byte_size) -} - -#[builtin(to_radix)] -//decompose _x into a _result_len vector over the _radix basis -//_radix must be less than 256 -fn to_radix(_x : Field, _radix: u32, _result_len: u32) -> [u8] {} - - +mod field; // Returns base^exponent. // ^ means to the power of and not xor @@ -28,7 +14,7 @@ fn to_radix(_x : Field, _radix: u32, _result_len: u32) -> [u8] {} // using a bigger bit size impacts negatively the performance and should be done only if the exponent does not fit in 32 bits fn pow_32(base: Field, exponent: Field) -> Field { let mut r = 1 as Field; - let b = crate::to_bits(exponent, 32); + let b = field::to_le_bits(exponent, 32); for i in 1..33 { r = r*r; diff --git a/noir_stdlib/src/merkle.nr b/noir_stdlib/src/merkle.nr index 188c7a011a5..7b6db21d755 100644 --- a/noir_stdlib/src/merkle.nr +++ b/noir_stdlib/src/merkle.nr @@ -17,7 +17,7 @@ fn check_membership_in_noir(root : Field, leaf : Field, index : Field, hash_path // Returns the root of the tree from the provided leaf and its hashpath, using pedersen hash fn compute_root_from_leaf(leaf : Field, index : Field, hash_path: [Field]) -> Field { let n = crate::array::len(hash_path); - let index_bits = crate::to_bits(index, n as u32); + let index_bits = crate::field::to_le_bits(index, n as u32); let mut current = leaf; for i in 0..n { let path_bit = index_bits[i] as bool; From a9662791fe476a1748a959c69efe8a0baba66eb6 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Tue, 31 Jan 2023 19:05:43 +0000 Subject: [PATCH 06/14] Remove unused dependencies and only use workspace inheritance on shared deps (#671) --- Cargo.lock | 10 -------- Cargo.toml | 39 +------------------------------ crates/arena/Cargo.toml | 2 +- crates/fm/Cargo.toml | 6 +++-- crates/nargo/Cargo.toml | 13 +++++------ crates/noirc_abi/Cargo.toml | 1 - crates/noirc_driver/Cargo.toml | 1 - crates/noirc_evaluator/Cargo.toml | 5 ++-- crates/noirc_frontend/Cargo.toml | 3 +-- crates/wasm/Cargo.toml | 12 ++++------ 10 files changed, 20 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0fd2b9d86bd..be5a14b8730 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1747,7 +1747,6 @@ name = "noirc_abi" version = "0.1.0" dependencies = [ "acvm", - "blake2", "iter-extended", "serde", "serde_derive", @@ -1767,7 +1766,6 @@ dependencies = [ "noirc_errors", "noirc_evaluator", "noirc_frontend", - "pathdiff", "serde", ] @@ -1789,7 +1787,6 @@ dependencies = [ "arena", "fm", "iter-extended", - "lazy_static", "noirc_abi", "noirc_errors", "noirc_frontend", @@ -1809,7 +1806,6 @@ dependencies = [ "iter-extended", "noirc_abi", "noirc_errors", - "pathdiff", "rustc-hash", "smol_str", "thiserror", @@ -1995,12 +1991,6 @@ version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d01a5bd0424d00070b0098dd17ebca6f961a959dead1dbcbbbc1d1cd8d3deeba" -[[package]] -name = "pathdiff" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8835116a5c179084a830efb3adc117ab007512b535bc1a21c991d3b32a6b44dd" - [[package]] name = "peeking_take_while" version = "0.1.2" diff --git a/Cargo.toml b/Cargo.toml index 77ed566392c..4b469306568 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,6 @@ members = [ "crates/iter-extended", "crates/wasm", ] -exclude = ["examples/9/merkle_tree_processor"] default-members = ["crates/nargo"] [workspace.package] @@ -34,51 +33,15 @@ noirc_evaluator = { path = "crates/noirc_evaluator" } noirc_frontend = { path = "crates/noirc_frontend" } noir_wasm = { path = "crates/wasm" } -ark-bn254 = { version = "^0.3.0", default-features = false, features = [ - "curve", -] } -ark-bls12-381 = { version = "^0.3.0", default-features = false, features = [ - "curve", -] } -ark-ff = { version = "^0.3.0", default-features = false } -blake2 = "0.9.1" -chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" } cfg-if = "1.0.0" -clap = "2.33.3" codespan = "0.9.5" codespan-reporting = "0.9.5" -console_error_panic_hook = "*" +chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" } dirs = "4" -flate2 = "1.0.24" -generational-arena = "0.2.8" -getrandom = { version = "0.2.4", features = ["js"] } -gloo-utils = { version = "0.1", features = ["serde"] } -hex = "0.4.2" -indexmap = "1.7.0" -js-sys = "0.3.55" -lazy_static = "1.4.0" -k256 = { version = "0.7.2", features = [ - "ecdsa", - "ecdsa-core", - "sha256", - "digest", - "arithmetic", -] } -pathdiff = "0.2" -num-bigint = "0.4" -num-traits = "0.2.8" -rmp-serde = "1.1.0" -rustc-hash = "1.1.0" -rustc_version = "0.4.0" serde = { version = "1.0.136", features = ["derive"] } serde_derive = "1.0.123" serde_json = "1.0" -sled = "0.34.6" -sha2 = "0.9.3" smol_str = "0.1.17" -tempdir = "0.3.7" -tempfile = "3.2.0" -termcolor = "1.1.2" thiserror = "1.0.21" toml = "0.5.8" url = "2.2.0" diff --git a/crates/arena/Cargo.toml b/crates/arena/Cargo.toml index 6b8e9ff4947..04e46791ce0 100644 --- a/crates/arena/Cargo.toml +++ b/crates/arena/Cargo.toml @@ -7,4 +7,4 @@ edition.workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -generational-arena.workspace = true +generational-arena = "0.2.8" diff --git a/crates/fm/Cargo.toml b/crates/fm/Cargo.toml index a9ea7e3806a..1a4c4dd7f1c 100644 --- a/crates/fm/Cargo.toml +++ b/crates/fm/Cargo.toml @@ -9,7 +9,9 @@ edition.workspace = true [dependencies] codespan-reporting.workspace = true cfg-if.workspace = true + [target.'cfg(target_arch = "wasm32")'.dependencies] -wasm-bindgen = { version = "*", features = ["serde-serialize"] } +wasm-bindgen.workspace = true + [dev-dependencies] -tempfile.workspace = true +tempfile = "3.2.0" diff --git a/crates/nargo/Cargo.toml b/crates/nargo/Cargo.toml index aaed0a0ed35..ea018cd7ead 100644 --- a/crates/nargo/Cargo.toml +++ b/crates/nargo/Cargo.toml @@ -8,7 +8,7 @@ edition.workspace = true [build-dependencies] dirs.workspace = true -rustc_version.workspace = true +rustc_version = "0.4.0" [dependencies] dirs.workspace = true @@ -20,15 +20,14 @@ noirc_abi.workspace = true fm.workspace = true acvm.workspace = true cfg-if.workspace = true - toml.workspace = true serde_derive.workspace = true -serde = "1.0.123" -clap.workspace = true -termcolor.workspace = true -hex.workspace = true -tempdir.workspace = true thiserror.workspace = true +serde = "1.0.123" +clap = "2.33.3" +hex = "0.4.2" +termcolor = "1.1.2" +tempdir = "0.3.7" # Backends aztec_backend = { optional = true, package = "barretenberg_static_lib", git = "https://github.com/noir-lang/aztec_backend", rev = "c673a14be33e445d98b35969716ac59c682036d6" } diff --git a/crates/noirc_abi/Cargo.toml b/crates/noirc_abi/Cargo.toml index 2f4b0fdcaff..92ace9751c1 100644 --- a/crates/noirc_abi/Cargo.toml +++ b/crates/noirc_abi/Cargo.toml @@ -12,7 +12,6 @@ iter-extended.workspace = true toml.workspace = true serde.workspace = true serde_derive.workspace = true -blake2.workspace = true thiserror.workspace = true [dev-dependencies] diff --git a/crates/noirc_driver/Cargo.toml b/crates/noirc_driver/Cargo.toml index b53e3ab5e0a..67cf385c8c4 100644 --- a/crates/noirc_driver/Cargo.toml +++ b/crates/noirc_driver/Cargo.toml @@ -15,4 +15,3 @@ acvm.workspace = true fm.workspace = true serde.workspace = true dirs.workspace = true -pathdiff.workspace = true diff --git a/crates/noirc_evaluator/Cargo.toml b/crates/noirc_evaluator/Cargo.toml index 4f1e10ea60b..d2a137afaa9 100644 --- a/crates/noirc_evaluator/Cargo.toml +++ b/crates/noirc_evaluator/Cargo.toml @@ -14,7 +14,6 @@ acvm.workspace = true arena.workspace = true fm.workspace = true iter-extended.workspace = true -lazy_static.workspace = true thiserror.workspace = true -num-bigint.workspace = true -num-traits.workspace = true +num-bigint = "0.4" +num-traits = "0.2.8" diff --git a/crates/noirc_frontend/Cargo.toml b/crates/noirc_frontend/Cargo.toml index 35046dd9f71..254cd6a3115 100644 --- a/crates/noirc_frontend/Cargo.toml +++ b/crates/noirc_frontend/Cargo.toml @@ -15,6 +15,5 @@ arena.workspace = true iter-extended.workspace = true chumsky.workspace = true thiserror.workspace = true -pathdiff.workspace = true smol_str.workspace = true -rustc-hash.workspace = true +rustc-hash = "1.1.0" diff --git a/crates/wasm/Cargo.toml b/crates/wasm/Cargo.toml index c0fd22eb709..f4a76bf8fc9 100644 --- a/crates/wasm/Cargo.toml +++ b/crates/wasm/Cargo.toml @@ -13,15 +13,13 @@ crate-type = ["cdylib"] [dependencies] acvm.workspace = true -noirc_driver = { path = "../noirc_driver" } - -console_error_panic_hook.workspace = true - +noirc_driver.workspace = true wasm-bindgen.workspace = true -gloo-utils.workspace = true -getrandom.workspace = true -js-sys.workspace = true +console_error_panic_hook = "*" +getrandom = { version = "0.2.4", features = ["js"] } +gloo-utils = { version = "0.1", features = ["serde"] } +js-sys = "0.3.55" [dev-dependencies] wasm-bindgen-test.workspace = true From 4872e9179fa4d85a111cda50d852a020b07cecd0 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Tue, 31 Jan 2023 20:07:10 +0100 Subject: [PATCH 07/14] Handle out-of-bound errors in CSE (#471) (#673) * Handle out-of-bound errors in CSE * Code review * Code review - removes the CseAction error * Code review * fix clippy error --- crates/noirc_evaluator/src/ssa/anchor.rs | 71 ++++++++++++++++++------ crates/noirc_evaluator/src/ssa/optim.rs | 20 +++---- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/anchor.rs b/crates/noirc_evaluator/src/ssa/anchor.rs index 17b10451fbe..c0ab28402ad 100644 --- a/crates/noirc_evaluator/src/ssa/anchor.rs +++ b/crates/noirc_evaluator/src/ssa/anchor.rs @@ -1,5 +1,7 @@ use std::collections::{HashMap, VecDeque}; +use crate::errors::{RuntimeError, RuntimeErrorKind}; + use super::{ context::SsaContext, mem::ArrayId, @@ -89,8 +91,8 @@ impl Anchor { None } - fn get_mem_map(&self, a: ArrayId) -> &Vec> { - &self.mem_map[&a] + fn get_mem_map(&self, a: &ArrayId) -> &Vec> { + &self.mem_map[a] } pub fn get_mem_all(&self, a: ArrayId) -> &VecDeque { @@ -105,7 +107,11 @@ impl Anchor { } } - pub fn push_mem_instruction(&mut self, ctx: &SsaContext, id: NodeId) { + pub fn push_mem_instruction( + &mut self, + ctx: &SsaContext, + id: NodeId, + ) -> Result<(), RuntimeError> { let ins = ctx.get_instruction(id); let (array_id, index, is_load) = Anchor::get_mem_op(&ins.operation); self.use_array(array_id, ctx.mem[array_id].len as usize); @@ -134,10 +140,12 @@ impl Anchor { len } }; - self.mem_map.get_mut(&array_id).unwrap()[mem_idx].push_front((item_pos, id)); + let anchor_list = self.get_anchor_list_mut(&array_id, mem_idx)?; + anchor_list.push_front((item_pos, id)); } else { prev_list.push_front(MemItem::NonConst(id)); } + Ok(()) } pub fn find_similar_mem_instruction( @@ -145,14 +153,14 @@ impl Anchor { ctx: &SsaContext, op: &Operation, prev_ins: &VecDeque, - ) -> CseAction { + ) -> Result { for iter in prev_ins.iter() { - if let Some(action) = self.match_mem_item(ctx, iter, op) { - return action; + if let Some(action) = self.match_mem_item(ctx, iter, op)? { + return Ok(action); } } - CseAction::Keep + Ok(CseAction::Keep) } fn get_mem_op(op: &Operation) -> (ArrayId, NodeId, bool) { @@ -168,41 +176,68 @@ impl Anchor { ctx: &SsaContext, item: &MemItem, op: &Operation, - ) -> Option { + ) -> Result, RuntimeErrorKind> { let (array_id, index, is_load) = Anchor::get_mem_op(op); if let Some(b_value) = ctx.get_as_constant(index) { match item { MemItem::Const(p) | MemItem::ConstLoad(p) => { - let a = self.get_mem_map(array_id); let b_idx = b_value.to_u128() as usize; - for (pos, id) in &a[b_idx] { + let anchor_list = self.get_anchor_list(&array_id, b_idx)?; + for (pos, id) in anchor_list { if pos == p { let action = Anchor::match_mem_id(ctx, *id, index, is_load); if action.is_some() { - return action; + return Ok(action); } } } - None + Ok(None) } - MemItem::NonConst(id) => Anchor::match_mem_id(ctx, *id, index, is_load), + MemItem::NonConst(id) => Ok(Anchor::match_mem_id(ctx, *id, index, is_load)), } } else { match item { - MemItem::Const(_) => Some(CseAction::Keep), + MemItem::Const(_) => Ok(Some(CseAction::Keep)), MemItem::ConstLoad(_) => { if is_load { - None + Ok(None) } else { - Some(CseAction::Keep) + Ok(Some(CseAction::Keep)) } } - MemItem::NonConst(id) => Anchor::match_mem_id(ctx, *id, index, is_load), + MemItem::NonConst(id) => Ok(Anchor::match_mem_id(ctx, *id, index, is_load)), } } } + //Returns the anchor list of memory instructions for the array_id at the provided index + // It issues an out-of-bound error when the list does not exist at this index. + fn get_anchor_list( + &self, + array_id: &ArrayId, + index: usize, + ) -> Result<&VecDeque<(usize, NodeId)>, RuntimeErrorKind> { + let memory_map = self.get_mem_map(array_id); + memory_map.get(index).ok_or(RuntimeErrorKind::ArrayOutOfBounds { + index: index as u128, + bound: memory_map.len() as u128, + }) + } + + //Same as get_anchor_list() but returns a mutable anchor + fn get_anchor_list_mut( + &mut self, + array_id: &ArrayId, + index: usize, + ) -> Result<&mut VecDeque<(usize, NodeId)>, RuntimeErrorKind> { + let memory_map = self.mem_map.get_mut(array_id).unwrap(); + let len = memory_map.len() as u128; + memory_map + .get_mut(index) + .ok_or(RuntimeErrorKind::ArrayOutOfBounds { index: index as u128, bound: len }) + } + fn match_mem_id( ctx: &SsaContext, a: NodeId, diff --git a/crates/noirc_evaluator/src/ssa/optim.rs b/crates/noirc_evaluator/src/ssa/optim.rs index edad49ce32e..33f539a7941 100644 --- a/crates/noirc_evaluator/src/ssa/optim.rs +++ b/crates/noirc_evaluator/src/ssa/optim.rs @@ -214,11 +214,11 @@ fn cse_block_with_anchor( //No CSE for arrays because they are not in SSA form //We could improve this in future by checking if the arrays are immutable or not modified in-between let id = ctx.get_dummy_load(a); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; if let ObjectType::Pointer(a) = ctx.get_object_type(binary.rhs) { let id = ctx.get_dummy_load(a); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; } new_list.push(*ins_id); @@ -240,9 +240,9 @@ fn cse_block_with_anchor( } anchor.use_array(*x, ctx.mem[*x].len as usize); let prev_ins = anchor.get_mem_all(*x); - match anchor.find_similar_mem_instruction(ctx, &operator, prev_ins) { + match anchor.find_similar_mem_instruction(ctx, &operator, prev_ins)? { CseAction::Keep => { - anchor.push_mem_instruction(ctx, *ins_id); + anchor.push_mem_instruction(ctx, *ins_id)?; new_list.push(*ins_id) } CseAction::ReplaceWith(new_id) => { @@ -250,7 +250,7 @@ fn cse_block_with_anchor( new_mark = Mark::ReplaceWith(new_id); } CseAction::Remove(id_to_remove) => { - anchor.push_mem_instruction(ctx, *ins_id); + anchor.push_mem_instruction(ctx, *ins_id)?; new_list.push(*ins_id); // TODO if not found, it should be removed from other blocks; we could keep a list of instructions to remove if let Some(id) = new_list.iter().position(|x| *x == id_to_remove) { @@ -288,13 +288,13 @@ fn cse_block_with_anchor( //Add dummy store for functions that modify arrays for a in returned_arrays { let id = ctx.get_dummy_store(a.0); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; } if let Some(f) = ctx.try_get_ssafunc(*func) { for typ in &f.result_types { if let ObjectType::Pointer(a) = typ { let id = ctx.get_dummy_store(*a); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; } } } @@ -303,7 +303,7 @@ fn cse_block_with_anchor( if let Some(obj) = ctx.try_get_node(*arg) { if let ObjectType::Pointer(a) = obj.get_type() { let id = ctx.get_dummy_load(a); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; } } } @@ -317,14 +317,14 @@ fn cse_block_with_anchor( if let Some(obj) = ctx.try_get_node(*arg) { if let ObjectType::Pointer(a) = obj.get_type() { let id = ctx.get_dummy_load(a); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; activate_cse = false; } } } if let ObjectType::Pointer(a) = ins.res_type { let id = ctx.get_dummy_store(a); - anchor.push_mem_instruction(ctx, id); + anchor.push_mem_instruction(ctx, id)?; activate_cse = false; } From d78c938cb260368bcf5c7a54ced9ae83bfc3d800 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 1 Feb 2023 12:34:25 +0000 Subject: [PATCH 08/14] chore: clean up serde-related dependencies (#722) * chore: move serde_json dependency down into noirc_abi only * chore: remove usage of serde_derive dependency We don't need this extra dependency as serde reexports the relevant macros with `derive` feature flag set * chore: always use workspace version of serde --- Cargo.lock | 2 -- Cargo.toml | 2 -- crates/nargo/Cargo.toml | 3 +-- crates/nargo/src/toml.rs | 2 +- crates/noirc_abi/Cargo.toml | 3 +-- crates/noirc_abi/src/input_parser/toml.rs | 3 +-- 6 files changed, 4 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be5a14b8730..e2dec523e7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1702,7 +1702,6 @@ dependencies = [ "noirc_frontend", "rustc_version 0.4.0", "serde", - "serde_derive", "tempdir", "termcolor", "thiserror", @@ -1749,7 +1748,6 @@ dependencies = [ "acvm", "iter-extended", "serde", - "serde_derive", "serde_json", "thiserror", "toml", diff --git a/Cargo.toml b/Cargo.toml index 4b469306568..9b40ce42794 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,8 +39,6 @@ codespan-reporting = "0.9.5" chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" } dirs = "4" serde = { version = "1.0.136", features = ["derive"] } -serde_derive = "1.0.123" -serde_json = "1.0" smol_str = "0.1.17" thiserror = "1.0.21" toml = "0.5.8" diff --git a/crates/nargo/Cargo.toml b/crates/nargo/Cargo.toml index ea018cd7ead..2453fbb2d6a 100644 --- a/crates/nargo/Cargo.toml +++ b/crates/nargo/Cargo.toml @@ -21,9 +21,8 @@ fm.workspace = true acvm.workspace = true cfg-if.workspace = true toml.workspace = true -serde_derive.workspace = true +serde.workspace = true thiserror.workspace = true -serde = "1.0.123" clap = "2.33.3" hex = "0.4.2" termcolor = "1.1.2" diff --git a/crates/nargo/src/toml.rs b/crates/nargo/src/toml.rs index 18dde53b0e9..4c9ab210f66 100644 --- a/crates/nargo/src/toml.rs +++ b/crates/nargo/src/toml.rs @@ -1,4 +1,4 @@ -use serde_derive::Deserialize; +use serde::Deserialize; use std::collections::BTreeMap; use std::path::Path; diff --git a/crates/noirc_abi/Cargo.toml b/crates/noirc_abi/Cargo.toml index 92ace9751c1..3d12afc8293 100644 --- a/crates/noirc_abi/Cargo.toml +++ b/crates/noirc_abi/Cargo.toml @@ -11,8 +11,7 @@ acvm.workspace = true iter-extended.workspace = true toml.workspace = true serde.workspace = true -serde_derive.workspace = true thiserror.workspace = true [dev-dependencies] -serde_json.workspace = true +serde_json = "1.0" diff --git a/crates/noirc_abi/src/input_parser/toml.rs b/crates/noirc_abi/src/input_parser/toml.rs index bcd2fab20b3..069658ce7be 100644 --- a/crates/noirc_abi/src/input_parser/toml.rs +++ b/crates/noirc_abi/src/input_parser/toml.rs @@ -2,8 +2,7 @@ use super::InputValue; use crate::{errors::InputParserError, Abi, AbiType}; use acvm::FieldElement; use iter_extended::{btree_map, try_btree_map, try_vecmap, vecmap}; -use serde::Serialize; -use serde_derive::Deserialize; +use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; pub(crate) fn parse_toml( From f3a936693def58bb6375932bef669cc179a51057 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 1 Feb 2023 12:35:25 +0000 Subject: [PATCH 09/14] Remove print to console for named proofs in `nargo prove` (#718) * feat: remove print to console for named proofs * chore: clippy --- crates/nargo/src/cli/mod.rs | 6 ++-- crates/nargo/src/cli/prove_cmd.rs | 60 +++++++++++++++++++------------ 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 2faf739585e..7efcc46aaab 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -64,7 +64,7 @@ pub fn start_cli() { .subcommand( App::new("prove") .about("Create proof for this program") - .arg(Arg::with_name("proof_name").help("The name of the proof").required(true)) + .arg(Arg::with_name("proof_name").help("The name of the proof")) .arg(show_ssa.clone()) .arg(allow_warnings.clone()), ) @@ -174,7 +174,7 @@ fn write_inputs_to_file>( pub fn prove_and_verify(proof_name: &str, prg_dir: &Path, show_ssa: bool) -> bool { let tmp_dir = TempDir::new("p_and_v_tests").unwrap(); let proof_path = match prove_cmd::prove_with_path( - proof_name, + Some(proof_name), prg_dir, &tmp_dir.into_path(), show_ssa, @@ -187,7 +187,7 @@ pub fn prove_and_verify(proof_name: &str, prg_dir: &Path, show_ssa: bool) -> boo } }; - verify_cmd::verify_with_path(prg_dir, &proof_path, show_ssa, false).unwrap() + verify_cmd::verify_with_path(prg_dir, &proof_path.unwrap(), show_ssa, false).unwrap() } fn add_std_lib(driver: &mut Driver) { diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index 81d19b0dccc..bc8667ff605 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -17,9 +17,10 @@ use crate::{ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("prove").unwrap(); - let proof_name = args.value_of("proof_name").unwrap(); + let proof_name = args.value_of("proof_name"); let show_ssa = args.is_present("show-ssa"); let allow_warnings = args.is_present("allow-warnings"); + prove(proof_name, show_ssa, allow_warnings) } @@ -27,14 +28,39 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { /// So when we add witness values, their index start from 1. const WITNESS_OFFSET: u32 = 1; -fn prove(proof_name: &str, show_ssa: bool, allow_warnings: bool) -> Result<(), CliError> { +fn prove(proof_name: Option<&str>, show_ssa: bool, allow_warnings: bool) -> Result<(), CliError> { let curr_dir = std::env::current_dir().unwrap(); - let mut proof_path = PathBuf::new(); - proof_path.push(PROOFS_DIR); - let result = prove_with_path(proof_name, curr_dir, proof_path, show_ssa, allow_warnings); - match result { - Ok(_) => Ok(()), - Err(e) => Err(e), + + let mut proof_dir = PathBuf::new(); + proof_dir.push(PROOFS_DIR); + + prove_with_path(proof_name, curr_dir, proof_dir, show_ssa, allow_warnings)?; + + Ok(()) +} + +pub fn prove_with_path>( + proof_name: Option<&str>, + program_dir: P, + proof_dir: P, + show_ssa: bool, + allow_warnings: bool, +) -> Result, CliError> { + let (compiled_program, solved_witness) = + compile_circuit_and_witness(program_dir, show_ssa, allow_warnings)?; + + let backend = crate::backends::ConcreteBackend; + let proof = backend.prove_with_meta(compiled_program.circuit, solved_witness); + + println!("Proof successfully created"); + if let Some(proof_name) = proof_name { + let proof_path = save_proof_to_dir(proof, proof_name, proof_dir)?; + + println!("Proof saved to {}", proof_path.display()); + Ok(Some(proof_path)) + } else { + println!("{}", hex::encode(&proof)); + Ok(None) } } @@ -112,28 +138,16 @@ fn solve_witness( Ok(solved_witness) } -pub fn prove_with_path>( +fn save_proof_to_dir>( + proof: Vec, proof_name: &str, - program_dir: P, proof_dir: P, - show_ssa: bool, - allow_warnings: bool, ) -> Result { - let (compiled_program, solved_witness) = - compile_circuit_and_witness(program_dir, show_ssa, allow_warnings)?; - - let backend = crate::backends::ConcreteBackend; - let proof = backend.prove_with_meta(compiled_program.circuit, solved_witness); - let mut proof_path = create_named_dir(proof_dir.as_ref(), "proof"); proof_path.push(proof_name); proof_path.set_extension(PROOF_EXT); - println!("proof : {}", hex::encode(&proof)); - - let path = write_to_file(hex::encode(&proof).as_bytes(), &proof_path); - println!("Proof successfully created and located at {path}"); - println!("{:?}", std::fs::canonicalize(&proof_path)); + write_to_file(hex::encode(proof).as_bytes(), &proof_path); Ok(proof_path) } From 6e01c2f8518e6a551acbc987b3117649d2e0ed8f Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 1 Feb 2023 12:36:22 +0000 Subject: [PATCH 10/14] feat(nargo): include short git commit in cli version output (#721) --- Cargo.lock | 50 +++++++++++++++++++++++++++++++++++++ crates/nargo/Cargo.toml | 2 ++ crates/nargo/src/cli/mod.rs | 7 +++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index e2dec523e7d..96c0a2bafa1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -633,6 +633,26 @@ version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f6b64db6932c7e49332728e3a6bd82c6b7e16016607d20923b537c3bc4c0d5f" +[[package]] +name = "const_format" +version = "0.2.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7309d9b4d3d2c0641e018d449232f2e28f1b22933c137f157d3dbc14228b8c0e" +dependencies = [ + "const_format_proc_macros", +] + +[[package]] +name = "const_format_proc_macros" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d897f47bf7270cf70d370f8f98c1abb6d2d4cf60a6845d30e05bfb90c6568650" +dependencies = [ + "proc-macro2", + "quote", + "unicode-xid", +] + [[package]] name = "core-foundation" version = "0.9.3" @@ -1256,6 +1276,28 @@ version = "0.27.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dec7af912d60cdbd3677c1af9352ebae6fb8394d165568a2234df0fa00f87793" +[[package]] +name = "git-version" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6b0decc02f4636b9ccad390dcbe77b722a77efedfa393caf8379a51d5c61899" +dependencies = [ + "git-version-macro", + "proc-macro-hack", +] + +[[package]] +name = "git-version-macro" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe69f1cbdb6e28af2bac214e943b99ce8a0a06b447d15d3e61161b0423139f3f" +dependencies = [ + "proc-macro-hack", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "glob" version = "0.3.1" @@ -1692,8 +1734,10 @@ dependencies = [ "barretenberg_wasm", "cfg-if 1.0.0", "clap 2.34.0", + "const_format", "dirs 4.0.0", "fm", + "git-version", "hex", "iter-extended", "marlin_arkworks_backend", @@ -2068,6 +2112,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "proc-macro-hack" +version = "0.5.20+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" + [[package]] name = "proc-macro2" version = "1.0.50" diff --git a/crates/nargo/Cargo.toml b/crates/nargo/Cargo.toml index 2453fbb2d6a..6691f604f73 100644 --- a/crates/nargo/Cargo.toml +++ b/crates/nargo/Cargo.toml @@ -24,6 +24,8 @@ toml.workspace = true serde.workspace = true thiserror.workspace = true clap = "2.33.3" +const_format = "0.2.30" +git-version = "0.3.5" hex = "0.4.2" termcolor = "1.1.2" tempdir = "0.3.7" diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 7efcc46aaab..269acbfbf6a 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -1,5 +1,7 @@ pub use check_cmd::check_from_path; use clap::{App, AppSettings, Arg}; +use const_format::formatcp; +use git_version::git_version; use noirc_abi::{ input_parser::{Format, InputValue}, Abi, @@ -25,6 +27,9 @@ mod new_cmd; mod prove_cmd; mod verify_cmd; +const SHORT_GIT_HASH: &str = git_version!(prefix = "git:"); +const VERSION_STRING: &str = formatcp!("{} ({})", env!("CARGO_PKG_VERSION"), SHORT_GIT_HASH); + pub fn start_cli() { let allow_warnings = Arg::with_name("allow-warnings") .long("allow-warnings") @@ -36,7 +41,7 @@ pub fn start_cli() { let matches = App::new("nargo") .about("Noir's package manager") - .version("0.1") + .version(VERSION_STRING) .author("Kevaundray Wedderburn ") .subcommand( App::new("check") From f18a760f19f7570bb21d6192e503113fec4706ec Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 1 Feb 2023 14:28:46 +0000 Subject: [PATCH 11/14] chore: readability improvements (#726) --- crates/nargo/src/cli/prove_cmd.rs | 4 +--- crates/noirc_evaluator/src/lib.rs | 8 ++++---- crates/noirc_evaluator/src/ssa/acir_gen.rs | 14 ++++++-------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index bc8667ff605..f9624ecc751 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -131,9 +131,7 @@ fn solve_witness( .collect(); let backend = crate::backends::ConcreteBackend; - let solver_res = backend.solve(&mut solved_witness, compiled_program.circuit.opcodes.clone()); - - solver_res.map_err(CliError::from)?; + backend.solve(&mut solved_witness, compiled_program.circuit.opcodes.clone())?; Ok(solved_witness) } diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 9269001c5be..ba98393f7cd 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -235,10 +235,10 @@ impl Evaluator { typ: &AbiType, ) -> Result, RuntimeErrorKind> { let mut witnesses = Vec::new(); - let mut element_width = None; - if let AbiType::Integer { width, .. } = typ { - element_width = Some(*width); - } + let element_width = match typ { + AbiType::Integer { width, .. } => Some(*width), + _ => None, + }; for _ in 0..*length { let witness = self.add_witness_to_cs(); witnesses.push(witness); diff --git a/crates/noirc_evaluator/src/ssa/acir_gen.rs b/crates/noirc_evaluator/src/ssa/acir_gen.rs index 9fbe4969d0c..679bf4de86c 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen.rs @@ -1402,16 +1402,14 @@ fn try_range_constraint(w: Witness, bits: u32, evaluator: &mut Evaluator) { } pub fn is_unit(arith: &Expression) -> Option { - if arith.mul_terms.is_empty() - && arith.linear_combinations.len() == 1 - && arith.linear_combinations[0].0 == FieldElement::one() - && arith.q_c == FieldElement::zero() - { - return Some(arith.linear_combinations[0].1); - } if arith.mul_terms.is_empty() && arith.linear_combinations.len() == 1 { - //todo!("should be simplified"); + if arith.linear_combinations[0].0.is_one() && arith.q_c.is_zero() { + return Some(arith.linear_combinations[0].1); + } else { + //todo!("should be simplified"); + } } + None } pub fn from_witness(witness: Witness) -> Expression { From 49433901c33d4768172c8b83d3daaff23b5d2cec Mon Sep 17 00:00:00 2001 From: Ethan-000 Date: Wed, 1 Feb 2023 16:48:40 +0000 Subject: [PATCH 12/14] chore: explicit versions for dependencies (#727) --- Cargo.toml | 4 ++-- crates/wasm/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9b40ce42794..81a6361b7b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,5 +43,5 @@ smol_str = "0.1.17" thiserror = "1.0.21" toml = "0.5.8" url = "2.2.0" -wasm-bindgen = { version = "*", features = ["serde-serialize"] } -wasm-bindgen-test = "*" +wasm-bindgen = { version = "0.2.83", features = ["serde-serialize"] } +wasm-bindgen-test = "0.3.33" diff --git a/crates/wasm/Cargo.toml b/crates/wasm/Cargo.toml index f4a76bf8fc9..98e166f5ac2 100644 --- a/crates/wasm/Cargo.toml +++ b/crates/wasm/Cargo.toml @@ -16,7 +16,7 @@ acvm.workspace = true noirc_driver.workspace = true wasm-bindgen.workspace = true -console_error_panic_hook = "*" +console_error_panic_hook = "0.1.7" getrandom = { version = "0.2.4", features = ["js"] } gloo-utils = { version = "0.1", features = ["serde"] } js-sys = "0.3.55" From 90b6036daf958f34b336219f1b5f99397a3250ef Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Feb 2023 11:35:18 +0000 Subject: [PATCH 13/14] feat(noir)!: Returned values are no longer required by the prover (#731) * acir - add public_outputs field to acir structure * monomorphisation: - Add a SetPub Expression - Instead of adding a constrain statement and adding a `return` parameter to main, we now specify that the return type should be set as public. This is then dealt with in the ssa pass. * evaluator - acir now has a `public_outputs` field and so we add one into the evaluator * ssa - Add SetPub operation and opcode * ssa - codegen for setpub * ssa - setpub acir_gen * Overwrite merge commit to revert change to `get_max_value` * ssa - acir_gen - `get_or_generate_witness` is not named `generate_witness` * revert: remove public_outputs field - This can be done as a separate PR. Removing it to make PR easier to review, an issue has been opened up for this * remove the return type from the abi when converting abi parameters to noir parameters * clippy * ssa - acir_gen : create witnesses for constants generate_witness will panic when trying to generate a witness for a constant * ssa - acir_gen : deref node_id when we have an array codegening an array and calling to_node_ids will produce a single node_id, whereas we want to fetch all elements of the array * nargo - remove duplicates - When a public input is return as a public output (return value), it is then added into the public input list twice. Using the current code, we need this duplication since we want to write the return values into the toml files. For proving and verifying we want to remove these duplicate values * noirc_abi - skip `return` when proving - The prover no longer supplies a `return` field in toml. - The ABI will have a `return` field, so when proving, we use the `skip_output` flag to ignore the `return` field in the ABI - As noted in the comments, this will look cleaner when we add a `public_outputs` field * nargo/tests - remove "return" field from toml files * cargo fmt imports * remove SetPub Expression * ssa - remove SetPub operation * evaluator - add a method which tells you whether a witness index corresponds to private input from the ABI (main parameters) * - use Operation::Return to set the values returned from main as public. - Additionally, private ABI inputs are not allowed * fix clippy * ssa-integer: do not skip the return operation during integer overflow - Only create the Return Operation if the return type from main is not Unit * ssa-acir_gen : replace todo with Err Result variant * remove TODOs * remove returns * delete unused `to_bytes` * remove `return` from `to_le_bytes` example * reduce diff * move dedup methods to mod.rs file * remove TODO on return type keyword * move deduplication code into `compile_circuit_and_witness` * update `num_witnesses_abi_len` parameter * fix example: private values in the ABI cannot be returned as public * use HashMap and check for previous insertion value * fix clippy --- crates/nargo/src/cli/mod.rs | 41 ++- crates/nargo/src/cli/prove_cmd.rs | 18 +- crates/nargo/src/cli/verify_cmd.rs | 14 +- .../tests/test_data/hash_to_field/Prover.toml | 1 - .../higher-order-functions/Prover.toml | 2 +- .../tests/test_data/main_return/Prover.toml | 1 - .../tests/test_data/main_return/src/main.nr | 2 +- .../nargo/tests/test_data/modulus/Prover.toml | 293 +++++++++++++++++- .../tests/test_data/simple_shield/Prover.toml | 6 +- .../tests/test_data/struct_inputs/Prover.toml | 3 +- .../tests/test_data/to_le_bytes/Prover.toml | 1 - crates/noirc_abi/src/lib.rs | 39 +-- crates/noirc_evaluator/src/lib.rs | 34 ++ crates/noirc_evaluator/src/ssa/acir_gen.rs | 41 ++- crates/noirc_evaluator/src/ssa/code_gen.rs | 7 +- crates/noirc_evaluator/src/ssa/integer.rs | 7 +- .../src/monomorphisation/mod.rs | 37 --- 17 files changed, 456 insertions(+), 91 deletions(-) diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 269acbfbf6a..336dffecc09 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -1,3 +1,4 @@ +use acvm::{acir::circuit::PublicInputs, FieldElement}; pub use check_cmd::check_from_path; use clap::{App, AppSettings, Arg}; use const_format::formatcp; @@ -9,7 +10,7 @@ use noirc_abi::{ use noirc_driver::Driver; use noirc_frontend::graph::{CrateName, CrateType}; use std::{ - collections::BTreeMap, + collections::{BTreeMap, HashMap, HashSet}, fs::File, io::Write, path::{Path, PathBuf}, @@ -206,6 +207,44 @@ fn path_to_stdlib() -> PathBuf { dirs::config_dir().unwrap().join("noir-lang").join("std/src") } +// Removes duplicates from the list of public input witnesses +fn dedup_public_input_indices(indices: PublicInputs) -> PublicInputs { + let duplicates_removed: HashSet<_> = indices.0.into_iter().collect(); + PublicInputs(duplicates_removed.into_iter().collect()) +} + +// Removes duplicates from the list of public input witnesses and the +// associated list of duplicate values. +pub(crate) fn dedup_public_input_indices_values( + indices: PublicInputs, + values: Vec, +) -> (PublicInputs, Vec) { + // Assume that the public input index lists and the values contain duplicates + assert_eq!(indices.0.len(), values.len()); + + let mut public_inputs_without_duplicates = Vec::new(); + let mut already_seen_public_indices = HashMap::new(); + + for (index, value) in indices.0.iter().zip(values) { + match already_seen_public_indices.get(index) { + Some(expected_value) => { + // The index has already been added + // so lets check that the values already inserted is equal to the value, we wish to insert + assert_eq!(*expected_value, value, "witness index {index:?} does not have a canonical map. The expected value is {expected_value}, the received value is {value}.") + } + None => { + already_seen_public_indices.insert(*index, value); + public_inputs_without_duplicates.push(value) + } + } + } + + ( + PublicInputs(already_seen_public_indices.keys().copied().collect()), + public_inputs_without_duplicates, + ) +} + // FIXME: I not sure that this is the right place for this tests. #[cfg(test)] mod tests { diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index f9624ecc751..a1fda13b3db 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -1,5 +1,3 @@ -use std::{collections::BTreeMap, path::PathBuf}; - use acvm::acir::native_types::Witness; use acvm::FieldElement; use acvm::PartialWitnessGenerator; @@ -7,9 +5,13 @@ use acvm::ProofSystemCompiler; use clap::ArgMatches; use noirc_abi::errors::AbiError; use noirc_abi::input_parser::{Format, InputValue}; -use std::path::Path; +use std::{ + collections::BTreeMap, + path::{Path, PathBuf}, +}; use super::{create_named_dir, read_inputs_from_file, write_inputs_to_file, write_to_file}; +use crate::cli::dedup_public_input_indices; use crate::{ constants::{PROOFS_DIR, PROOF_EXT, PROVER_INPUT_FILE, VERIFIER_INPUT_FILE}, errors::CliError, @@ -69,12 +71,20 @@ pub fn compile_circuit_and_witness>( show_ssa: bool, allow_unused_variables: bool, ) -> Result<(noirc_driver::CompiledProgram, BTreeMap), CliError> { - let compiled_program = super::compile_cmd::compile_circuit( + let mut compiled_program = super::compile_cmd::compile_circuit( program_dir.as_ref(), show_ssa, allow_unused_variables, )?; let solved_witness = parse_and_solve_witness(program_dir, &compiled_program)?; + + // Since the public outputs are added into the public inputs list + // There can be duplicates. We keep the duplicates for when one is + // encoding the return values into the Verifier.toml + // However, for creating a proof, we remove these duplicates. + compiled_program.circuit.public_inputs = + dedup_public_input_indices(compiled_program.circuit.public_inputs); + Ok((compiled_program, solved_witness)) } diff --git a/crates/nargo/src/cli/verify_cmd.rs b/crates/nargo/src/cli/verify_cmd.rs index 30d184d92e4..8ec38531102 100644 --- a/crates/nargo/src/cli/verify_cmd.rs +++ b/crates/nargo/src/cli/verify_cmd.rs @@ -1,4 +1,6 @@ -use super::{compile_cmd::compile_circuit, read_inputs_from_file}; +use super::{ + compile_cmd::compile_circuit, dedup_public_input_indices_values, read_inputs_from_file, +}; use crate::{ constants::{PROOFS_DIR, PROOF_EXT, VERIFIER_INPUT_FILE}, errors::CliError, @@ -59,7 +61,7 @@ pub fn verify_with_path>( } fn verify_proof( - compiled_program: CompiledProgram, + mut compiled_program: CompiledProgram, public_inputs: BTreeMap, proof: Vec, ) -> Result { @@ -71,8 +73,14 @@ fn verify_proof( _ => CliError::from(error), })?; + // Similarly to when proving -- we must remove the duplicate public witnesses which + // can be present because a public input can also be added as a public output. + let (dedup_public_indices, dedup_public_values) = + dedup_public_input_indices_values(compiled_program.circuit.public_inputs, public_inputs); + compiled_program.circuit.public_inputs = dedup_public_indices; + let backend = crate::backends::ConcreteBackend; - let valid_proof = backend.verify_from_cs(&proof, public_inputs, compiled_program.circuit); + let valid_proof = backend.verify_from_cs(&proof, dedup_public_values, compiled_program.circuit); Ok(valid_proof) } diff --git a/crates/nargo/tests/test_data/hash_to_field/Prover.toml b/crates/nargo/tests/test_data/hash_to_field/Prover.toml index 079763a108e..f6597d3f78a 100644 --- a/crates/nargo/tests/test_data/hash_to_field/Prover.toml +++ b/crates/nargo/tests/test_data/hash_to_field/Prover.toml @@ -1,2 +1 @@ input = "1" -return = "0x25cebc29ded2fa515a937e2b5f674e3026c012e5b57f8a48d7dce6b7d274f9d9" diff --git a/crates/nargo/tests/test_data/higher-order-functions/Prover.toml b/crates/nargo/tests/test_data/higher-order-functions/Prover.toml index d3504ebe83f..8b137891791 100644 --- a/crates/nargo/tests/test_data/higher-order-functions/Prover.toml +++ b/crates/nargo/tests/test_data/higher-order-functions/Prover.toml @@ -1 +1 @@ -return = "5" \ No newline at end of file + diff --git a/crates/nargo/tests/test_data/main_return/Prover.toml b/crates/nargo/tests/test_data/main_return/Prover.toml index ef1ca7bb7e8..63e9878811a 100644 --- a/crates/nargo/tests/test_data/main_return/Prover.toml +++ b/crates/nargo/tests/test_data/main_return/Prover.toml @@ -1,2 +1 @@ -return = "" x = "8" diff --git a/crates/nargo/tests/test_data/main_return/src/main.nr b/crates/nargo/tests/test_data/main_return/src/main.nr index aefa2532731..06347eb0919 100644 --- a/crates/nargo/tests/test_data/main_return/src/main.nr +++ b/crates/nargo/tests/test_data/main_return/src/main.nr @@ -1,3 +1,3 @@ -fn main(x: Field) -> pub Field { +fn main(x: pub Field) -> pub Field { x } diff --git a/crates/nargo/tests/test_data/modulus/Prover.toml b/crates/nargo/tests/test_data/modulus/Prover.toml index d46300b8a5b..d435609bb1a 100644 --- a/crates/nargo/tests/test_data/modulus/Prover.toml +++ b/crates/nargo/tests/test_data/modulus/Prover.toml @@ -1,3 +1,290 @@ -bn254_modulus_be_bytes = [48, 100, 78, 114, 225, 49, 160, 41, 184, 80, 69, 182, 129, 129, 88, 93, 40, 51, 232, 72, 121, 185, 112, 145, 67, 225, 245, 147, 240, 0, 0, 1] -bn254_modulus_be_bits = [1, 1, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1, 0, 0, 1, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1, 1, 0, 1, 1, 0, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 1, 1, 0, 0, 0, 0, 1, 0, 1, 1, 1, 0, 1, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 0, 1, 1, 1, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 1, 0, 1, 1, 0, 0, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] -return = "" \ No newline at end of file +bn254_modulus_be_bytes = [ + 48, + 100, + 78, + 114, + 225, + 49, + 160, + 41, + 184, + 80, + 69, + 182, + 129, + 129, + 88, + 93, + 40, + 51, + 232, + 72, + 121, + 185, + 112, + 145, + 67, + 225, + 245, + 147, + 240, + 0, + 0, + 1, +] +bn254_modulus_be_bits = [ + 1, + 1, + 0, + 0, + 0, + 0, + 0, + 1, + 1, + 0, + 0, + 1, + 0, + 0, + 0, + 1, + 0, + 0, + 1, + 1, + 1, + 0, + 0, + 1, + 1, + 1, + 0, + 0, + 1, + 0, + 1, + 1, + 1, + 0, + 0, + 0, + 0, + 1, + 0, + 0, + 1, + 1, + 0, + 0, + 0, + 1, + 1, + 0, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 1, + 0, + 1, + 0, + 0, + 1, + 1, + 0, + 1, + 1, + 1, + 0, + 0, + 0, + 0, + 1, + 0, + 1, + 0, + 0, + 0, + 0, + 0, + 1, + 0, + 0, + 0, + 1, + 0, + 1, + 1, + 0, + 1, + 1, + 0, + 1, + 1, + 0, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 1, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 1, + 0, + 1, + 0, + 1, + 1, + 0, + 0, + 0, + 0, + 1, + 0, + 1, + 1, + 1, + 0, + 1, + 0, + 0, + 1, + 0, + 1, + 0, + 0, + 0, + 0, + 0, + 1, + 1, + 0, + 0, + 1, + 1, + 1, + 1, + 1, + 0, + 1, + 0, + 0, + 0, + 0, + 1, + 0, + 0, + 1, + 0, + 0, + 0, + 0, + 1, + 1, + 1, + 1, + 0, + 0, + 1, + 1, + 0, + 1, + 1, + 1, + 0, + 0, + 1, + 0, + 1, + 1, + 1, + 0, + 0, + 0, + 0, + 1, + 0, + 0, + 1, + 0, + 0, + 0, + 1, + 0, + 1, + 0, + 0, + 0, + 0, + 1, + 1, + 1, + 1, + 1, + 0, + 0, + 0, + 0, + 1, + 1, + 1, + 1, + 1, + 0, + 1, + 0, + 1, + 1, + 0, + 0, + 1, + 0, + 0, + 1, + 1, + 1, + 1, + 1, + 1, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 1, +] diff --git a/crates/nargo/tests/test_data/simple_shield/Prover.toml b/crates/nargo/tests/test_data/simple_shield/Prover.toml index 4c03ef25a18..67e825f6333 100644 --- a/crates/nargo/tests/test_data/simple_shield/Prover.toml +++ b/crates/nargo/tests/test_data/simple_shield/Prover.toml @@ -5,11 +5,7 @@ index = "0" note_hash_path = [ "0x0000000000000000000000000000000000000000000000000000000000000000", "0x0e4223f3925f98934393c74975142bd73079ab0621f4ee133cee050a3c194f1a", - "0x2fd7bb412155bf8693a3bd2a3e7581a679c95c68a052f835dddca85fa1569a40" + "0x2fd7bb412155bf8693a3bd2a3e7581a679c95c68a052f835dddca85fa1569a40", ] to_pubkey_x = "0x0000000000000000000000000000000000000000000000000000000000000001" to_pubkey_y = "0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c" -return = [ - "0x1fa077bf4c787f858e0f3a7fa031ff11f200a0ccc6e549c6af98ec96800af340", - "0x0c25956c07e2277e75f1ca10bae32c29f24b27f25625fe9258cec29dc90651dd" -] \ No newline at end of file diff --git a/crates/nargo/tests/test_data/struct_inputs/Prover.toml b/crates/nargo/tests/test_data/struct_inputs/Prover.toml index 46aae99b425..ca992f8502c 100644 --- a/crates/nargo/tests/test_data/struct_inputs/Prover.toml +++ b/crates/nargo/tests/test_data/struct_inputs/Prover.toml @@ -1,5 +1,4 @@ x = "5" -return = "" [y] foo = "5" @@ -15,4 +14,4 @@ message = "helld" [a.bar_struct] val = "1" array = [0, 1] -message = "hello" \ No newline at end of file +message = "hello" diff --git a/crates/nargo/tests/test_data/to_le_bytes/Prover.toml b/crates/nargo/tests/test_data/to_le_bytes/Prover.toml index f5f4f367d1c..07fe857ac7c 100644 --- a/crates/nargo/tests/test_data/to_le_bytes/Prover.toml +++ b/crates/nargo/tests/test_data/to_le_bytes/Prover.toml @@ -1,2 +1 @@ x = "2040124" -return = [0x3c, 0x21, 0x1f, 0x00] \ No newline at end of file diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index dda568472e7..821cb663912 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -155,12 +155,24 @@ impl Abi { pub fn encode( self, inputs: &BTreeMap, - allow_undefined_return: bool, + skip_output: bool, ) -> Result, AbiError> { - let param_names = self.parameter_names(); + // Condition that specifies whether we should filter the "return" + // parameter. We do this in the case that it is not in the `inputs` + // map specified. + // + // See Issue #645 : Adding a `public outputs` field into acir and + // the ABI will clean up this logic + // For prosperity; the prover does not know about a `return` value + // so we skip this when encoding the ABI + let return_condition = + |param_name: &&String| !skip_output || (param_name != &MAIN_RETURN_NAME); + + let parameters = self.parameters.iter().filter(|param| return_condition(&¶m.name)); + let param_names: Vec<&String> = parameters.clone().map(|param| ¶m.name).collect(); let mut encoded_inputs = Vec::new(); - for param in self.parameters.iter() { + for param in parameters { let value = inputs .get(¶m.name) .ok_or_else(|| AbiError::MissingParam(param.name.to_owned()))? @@ -170,26 +182,6 @@ impl Abi { return Err(AbiError::TypeMismatch { param: param.to_owned(), value }); } - // As the circuit calculates the return value in the process of calculating rest of the witnesses - // it's not absolutely necessary to provide them as inputs. We then tolerate an undefined value for - // the return value input and just skip it. - if allow_undefined_return - && param.name == MAIN_RETURN_NAME - && matches!(value, InputValue::Undefined) - { - let return_witness_len = param.typ.field_count(); - - // We do not support undefined arrays for now - TODO - if return_witness_len != 1 { - return Err(AbiError::Generic( - "Values of array returned from main must be specified".to_string(), - )); - } else { - // This assumes that the return value is at the end of the ABI, otherwise values will be misaligned. - continue; - } - } - encoded_inputs.extend(Self::encode_value(value, ¶m.name)?); } @@ -240,7 +232,6 @@ impl Abi { let mut field_iterator = encoded_inputs.iter().cloned(); let mut decoded_inputs = BTreeMap::new(); - for param in &self.parameters { let decoded_value = Self::decode_value(&mut field_iterator, ¶m.typ)?; diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index ba98393f7cd..3e07c6ae7c1 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -20,6 +20,9 @@ pub struct Evaluator { // so it is safer to use a u32, at least until clang is changed // to compile wasm64. current_witness_index: u32, + // This is the number of witnesses indices used when + // creating the private/public inputs of the ABI. + num_witnesses_abi_len: usize, public_inputs: Vec, opcodes: Vec, } @@ -60,6 +63,7 @@ impl Evaluator { fn new() -> Self { Evaluator { public_inputs: Vec::new(), + num_witnesses_abi_len: 0, // XXX: Barretenberg, reserves the first index to have value 0. // When we increment, we do not use this index at all. // This means that every constraint system at the moment, will either need @@ -73,6 +77,22 @@ impl Evaluator { } } + // Returns true if the `witness_index` + // was created in the ABI as a private input. + // + // Note: This method is used so that we don't convert private + // ABI inputs into public outputs. + fn is_private_abi_input(&self, witness_index: Witness) -> bool { + // 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_inputs.contains(&witness_index); + + !is_intermediate_variable && !is_public_input + } + // Creates a new Witness index fn add_witness_to_cs(&mut self) -> Witness { self.current_witness_index += 1; @@ -261,6 +281,16 @@ impl Evaluator { let main = igen.program.main(); let main_params = std::mem::take(&mut main.parameters); let abi_params = std::mem::take(&mut igen.program.abi.parameters); + + // Remove the return type from the parameters + // Since this is not in the main functions parameters. + // + // TODO(See Issue633) regarding adding a `return_type` field to the ABI struct + let abi_params: Vec<_> = abi_params + .into_iter() + .filter(|param| param.name != noirc_abi::MAIN_RETURN_NAME) + .collect(); + assert_eq!(main_params.len(), abi_params.len()); for ((param_id, _, param_name, _), abi_param) in main_params.iter().zip(abi_params) { @@ -269,5 +299,9 @@ impl Evaluator { self.param_to_var(param_name, def, &abi_param.typ, &abi_param.visibility, igen) .unwrap(); } + + // Store the number of witnesses used to represent the types + // in the ABI + self.num_witnesses_abi_len = self.current_witness_index as usize; } } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen.rs b/crates/noirc_evaluator/src/ssa/acir_gen.rs index 679bf4de86c..13efd26c211 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen.rs @@ -188,7 +188,46 @@ impl Acir { InternalVar::from(v) } Operation::Call { .. } => unreachable!("call instruction should have been inlined"), - Operation::Return(_) => todo!(), //return from main + Operation::Return(node_ids) => { + // XXX: When we return a node_id that was created from + // the UnitType, there is a witness associated with it + // Ideally no witnesses are created for such types. + + // This can only ever be called in the main context. + // In all other context's, the return operation is transformed. + + for node_id in node_ids { + // An array produces a single node_id + // We therefore need to check if the node_id is referring to an array + // and deference to get the elements + let objects = match Memory::deref(ctx, *node_id) { + Some(a) => { + let array = &ctx.mem[a]; + self.load_array(array, false, evaluator) + } + None => vec![self.substitute(*node_id, evaluator, ctx)], + }; + + for mut object in objects { + let witness = if object.expression.is_const() { + evaluator.create_intermediate_variable(object.expression) + } else { + object.generate_witness(evaluator) + }; + + // Before pushing to the public inputs, we need to check that + // it was not a private ABI input + if evaluator.is_private_abi_input(witness) { + return Err(RuntimeErrorKind::Spanless(String::from( + "we do not allow private ABI inputs to be returned as public outputs", + ))); + } + evaluator.public_inputs.push(witness); + } + } + + InternalVar::default() + } Operation::Cond { condition, val_true: lhs, val_false: rhs } => { let cond = self.substitute(*condition, evaluator, ctx); let l_c = self.substitute(*lhs, evaluator, ctx); diff --git a/crates/noirc_evaluator/src/ssa/code_gen.rs b/crates/noirc_evaluator/src/ssa/code_gen.rs index 0e3df891aec..75a44c5e8f1 100644 --- a/crates/noirc_evaluator/src/ssa/code_gen.rs +++ b/crates/noirc_evaluator/src/ssa/code_gen.rs @@ -126,7 +126,12 @@ impl IRGenerator { pub fn codegen_main(&mut self) -> Result<(), RuntimeError> { let main_body = self.program.take_main_body(); - self.codegen_expression(&main_body)?; + let value = self.codegen_expression(&main_body)?; + let node_ids = value.to_node_ids(); + + if self.program.main().return_type != Type::Unit { + self.context.new_instruction(Operation::Return(node_ids), ObjectType::NotAnObject)?; + } Ok(()) } diff --git a/crates/noirc_evaluator/src/ssa/integer.rs b/crates/noirc_evaluator/src/ssa/integer.rs index ed4945dcbfc..b989f7f427a 100644 --- a/crates/noirc_evaluator/src/ssa/integer.rs +++ b/crates/noirc_evaluator/src/ssa/integer.rs @@ -246,10 +246,7 @@ fn block_overflow( for mut ins in instructions { if matches!( ins.operation, - Operation::Nop - | Operation::Call { .. } - | Operation::Result { .. } - | Operation::Return(_) + Operation::Nop | Operation::Call { .. } | Operation::Result { .. } ) { //For now we skip completely functions from overflow; that means arguments are NOT truncated. //The reasoning is that this is handled by doing the overflow strategy after the function has been inlined @@ -481,7 +478,7 @@ fn get_max_value(ins: &Instruction, max_map: &mut HashMap) -> B Operation::Load { .. } => unreachable!(), Operation::Store { .. } => BigUint::zero(), Operation::Call { .. } => ins.res_type.max_size(), //n.b. functions should have been inlined - Operation::Return(_) => todo!(), + Operation::Return(_) => ins.res_type.max_size(), Operation::Result { .. } => { unreachable!("Functions must have been inlined before checking for overflows") } diff --git a/crates/noirc_frontend/src/monomorphisation/mod.rs b/crates/noirc_frontend/src/monomorphisation/mod.rs index d20a520c4f6..746e1203fd1 100644 --- a/crates/noirc_frontend/src/monomorphisation/mod.rs +++ b/crates/noirc_frontend/src/monomorphisation/mod.rs @@ -129,49 +129,12 @@ impl Monomorphiser { self.globals.entry(id).or_default().insert(typ, new_id); } - /// The main function is special, we need to check for a return type and if present, - /// insert an extra constrain on the return value. fn compile_main(&mut self, main_id: node_interner::FuncId) -> Abi { let new_main_id = self.next_function_id(); assert_eq!(new_main_id, Program::main_id()); self.function(main_id, new_main_id); let main_meta = self.interner.function_meta(&main_id); - let main = self.finished_functions.get_mut(&new_main_id).unwrap(); - - // If the main function has a return type we manually desugar it here - // to add a `constrain ret == expected_ret` where `ret` is the body of main - // and `expected_ret` is a new parameter added to the public inputs. - if main.return_type != ast::Type::Unit { - let id = self.next_local_id(); - - let main = self.finished_functions.get_mut(&new_main_id).unwrap(); - main.parameters.push((id, false, "return".into(), main.return_type.clone())); - main.return_type = ast::Type::Unit; - - let name = "_".into(); - let typ = Self::convert_type(main_meta.return_type()); - let lhs = Box::new(ast::Expression::Ident(ast::Ident { - definition: Definition::Local(id), - mutable: false, - location: None, - name, - typ, - })); - - // Need to temporarily swap out main.body here because we cannot - // move out of it directly. - let tmp_body = ast::Expression::Literal(ast::Literal::Unit); - let main_body = std::mem::replace(&mut main.body, tmp_body); - - let rhs = Box::new(main_body); - let operator = ast::BinaryOp::Equal; - let eq = ast::Expression::Binary(ast::Binary { operator, lhs, rhs }); - - let location = self.interner.function_meta(&main_id).location; - main.body = ast::Expression::Constrain(Box::new(eq), location); - } - main_meta.into_abi(&self.interner) } From b0a96956ec626bf8ee059abf2604b9f6e8df3d1e Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Feb 2023 16:33:19 +0000 Subject: [PATCH 14/14] Rename methods that use `conditionalize` to be more descriptive (#739) --- crates/noirc_evaluator/src/ssa/conditional.rs | 28 ++++++++++++++----- crates/noirc_evaluator/src/ssa/inline.rs | 7 ++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/conditional.rs b/crates/noirc_evaluator/src/ssa/conditional.rs index df30bf5e8cf..d45ada720b8 100644 --- a/crates/noirc_evaluator/src/ssa/conditional.rs +++ b/crates/noirc_evaluator/src/ssa/conditional.rs @@ -283,7 +283,7 @@ impl DecisionTree { ctx[current].assumption = block_assumption; self.compute_assumption(ctx, current); - self.conditionalize_block(ctx, current, &mut data.stack)?; + self.apply_condition_to_block(ctx, current, &mut data.stack)?; Ok(result) } @@ -397,7 +397,9 @@ impl DecisionTree { Ok(()) } - pub fn conditionalize_block( + /// Apply the condition of the block to each instruction + /// in the block. + pub fn apply_condition_to_block( &self, ctx: &mut SsaContext, block: BlockId, @@ -405,14 +407,16 @@ impl DecisionTree { ) -> Result<(), RuntimeError> { let assumption_id = ctx[block].assumption; let instructions = ctx[block].instructions.clone(); - self.conditionalise_inline(ctx, &instructions, stack, assumption_id)?; + self.apply_condition_to_instructions(ctx, &instructions, stack, assumption_id)?; ctx[block].instructions.clear(); ctx[block].instructions.append(&mut stack.stack); assert!(stack.stack.is_empty()); Ok(()) } - pub fn conditionalise_inline( + /// Applies a condition to each instruction + /// and places into the stack frame. + pub fn apply_condition_to_instructions( &self, ctx: &mut SsaContext, instructions: &[NodeId], @@ -422,7 +426,13 @@ impl DecisionTree { if predicate == AssumptionId::dummy() || self[predicate].value != Some(ctx.zero()) { let mut short_circuit = false; for i in instructions { - if !self.conditionalise_into(ctx, result, *i, predicate, short_circuit)? { + if !self.apply_condition_to_instruction( + ctx, + result, + *i, + predicate, + short_circuit, + )? { short_circuit = true; } } @@ -477,7 +487,11 @@ impl DecisionTree { } } - pub fn conditionalise_into( + /// Applies a condition to the instruction + /// For most instructions, this does nothing + /// but for instructions with side-effects + /// this will alter the behavior. + pub fn apply_condition_to_instruction( &self, ctx: &mut SsaContext, stack: &mut StackFrame, @@ -667,7 +681,7 @@ impl DecisionTree { ObjectType::Pointer(array_dup), &mut memcpy_stack, ); - self.conditionalise_inline( + self.apply_condition_to_instructions( ctx, &memcpy_stack.stack, stack, diff --git a/crates/noirc_evaluator/src/ssa/inline.rs b/crates/noirc_evaluator/src/ssa/inline.rs index 5d6626b39da..42b16a4faa2 100644 --- a/crates/noirc_evaluator/src/ssa/inline.rs +++ b/crates/noirc_evaluator/src/ssa/inline.rs @@ -384,7 +384,12 @@ pub fn inline_in_block( if short_circuit { super::block::short_circuit_inline(ctx, stack_frame.block); } else { - decision.conditionalise_inline(ctx, &stack_frame.stack, &mut stack2, predicate)?; + decision.apply_condition_to_instructions( + ctx, + &stack_frame.stack, + &mut stack2, + predicate, + )?; // we add the conditionalised instructions to the target_block, at proper location (really need a linked list!) stack2.apply(ctx, stack_frame.block, call_id, false); }