From 62b7496c450fbf105e405aa463c3e796de92a428 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 2 May 2023 16:29:46 +0100 Subject: [PATCH] chore: Replace explicit if-elses with `FieldElement::from()` for boolean fields (#1266) * chore: replace if-elses with `FieldElement::from()` * chore: replace explicit equality with usage of `is_zero()` * chore: replace explicit usage of `from` with `.into()` --- crates/noirc_abi/src/input_parser/toml.rs | 14 +------ .../src/ssa/acir_gen/operations/bitwise.rs | 4 +- crates/noirc_evaluator/src/ssa/node.rs | 42 +++++++++++-------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/crates/noirc_abi/src/input_parser/toml.rs b/crates/noirc_abi/src/input_parser/toml.rs index 180cde4bf78..a737f784031 100644 --- a/crates/noirc_abi/src/input_parser/toml.rs +++ b/crates/noirc_abi/src/input_parser/toml.rs @@ -115,11 +115,7 @@ impl InputValue { InputValue::Field(new_value) } - TomlTypes::Bool(boolean) => { - let new_value = if boolean { FieldElement::one() } else { FieldElement::zero() }; - - InputValue::Field(new_value) - } + TomlTypes::Bool(boolean) => InputValue::Field(boolean.into()), TomlTypes::ArrayNum(arr_num) => { let array_elements = vecmap(arr_num, |elem_num| FieldElement::from(i128::from(elem_num))); @@ -132,13 +128,7 @@ impl InputValue { InputValue::Vec(array_elements) } TomlTypes::ArrayBool(arr_bool) => { - let array_elements = vecmap(arr_bool, |elem_bool| { - if elem_bool { - FieldElement::one() - } else { - FieldElement::zero() - } - }); + let array_elements = vecmap(arr_bool, FieldElement::from); InputValue::Vec(array_elements) } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/bitwise.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/bitwise.rs index f8ca271835e..00396f4d4b6 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/bitwise.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/bitwise.rs @@ -40,8 +40,8 @@ pub(super) fn simplify_bitwise( let max = FieldElement::from((1_u128 << bit_size) - 1); let (field, var) = match (lhs.to_const(), rhs.to_const()) { - (Some(l_c), None) => (l_c == FieldElement::zero() || l_c == max).then_some((l_c, rhs))?, - (None, Some(r_c)) => (r_c == FieldElement::zero() || r_c == max).then_some((r_c, lhs))?, + (Some(l_c), None) => (l_c.is_zero() || l_c == max).then_some((l_c, rhs))?, + (None, Some(r_c)) => (r_c.is_zero() || r_c == max).then_some((r_c, lhs))?, _ => return None, }; diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs index 8819a96e1c3..bec3c923a6d 100644 --- a/crates/noirc_evaluator/src/ssa/node.rs +++ b/crates/noirc_evaluator/src/ssa/node.rs @@ -918,8 +918,10 @@ impl Binary { !res_type.is_native_field(), "ICE: comparisons are not implemented for field elements" ); - let res = if lhs < rhs { FieldElement::one() } else { FieldElement::zero() }; - return Ok(NodeEval::Const(res, ObjectType::boolean())); + return Ok(NodeEval::Const( + FieldElement::from(lhs < rhs), + ObjectType::boolean(), + )); } } BinaryOp::Ule => { @@ -931,8 +933,10 @@ impl Binary { !res_type.is_native_field(), "ICE: comparisons are not implemented for field elements" ); - let res = if lhs <= rhs { FieldElement::one() } else { FieldElement::zero() }; - return Ok(NodeEval::Const(res, ObjectType::boolean())); + return Ok(NodeEval::Const( + FieldElement::from(lhs <= rhs), + ObjectType::boolean(), + )); } } BinaryOp::Slt => (), @@ -942,8 +946,10 @@ impl Binary { return Ok(NodeEval::Const(FieldElement::zero(), ObjectType::boolean())); //n.b we assume the type of lhs and rhs is unsigned because of the opcode, we could also verify this } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { - let res = if lhs < rhs { FieldElement::one() } else { FieldElement::zero() }; - return Ok(NodeEval::Const(res, ObjectType::boolean())); + return Ok(NodeEval::Const( + FieldElement::from(lhs < rhs), + ObjectType::boolean(), + )); } } BinaryOp::Lte => { @@ -951,30 +957,30 @@ impl Binary { return Ok(NodeEval::Const(FieldElement::one(), ObjectType::boolean())); //n.b we assume the type of lhs and rhs is unsigned because of the opcode, we could also verify this } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { - let res = if lhs <= rhs { FieldElement::one() } else { FieldElement::zero() }; - return Ok(NodeEval::Const(res, ObjectType::boolean())); + return Ok(NodeEval::Const( + FieldElement::from(lhs <= rhs), + ObjectType::boolean(), + )); } } BinaryOp::Eq => { if self.lhs == self.rhs { return Ok(NodeEval::Const(FieldElement::one(), ObjectType::boolean())); } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { - if lhs == rhs { - return Ok(NodeEval::Const(FieldElement::one(), ObjectType::boolean())); - } else { - return Ok(NodeEval::Const(FieldElement::zero(), ObjectType::boolean())); - } + return Ok(NodeEval::Const( + FieldElement::from(lhs == rhs), + ObjectType::boolean(), + )); } } BinaryOp::Ne => { if self.lhs == self.rhs { return Ok(NodeEval::Const(FieldElement::zero(), ObjectType::boolean())); } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { - if lhs != rhs { - return Ok(NodeEval::Const(FieldElement::one(), ObjectType::boolean())); - } else { - return Ok(NodeEval::Const(FieldElement::zero(), ObjectType::boolean())); - } + return Ok(NodeEval::Const( + FieldElement::from(lhs != rhs), + ObjectType::boolean(), + )); } } BinaryOp::And => {