From dfad378cdb5b5e29ded43ef6b628dc6cb6599769 Mon Sep 17 00:00:00 2001
From: Tom French <tom@tomfren.ch>
Date: Fri, 10 Jan 2025 13:46:42 +0000
Subject: [PATCH] chore: address some todos

---
 .../src/ssa/opt/remove_bit_shifts.rs           | 18 +++++++-----------
 .../noirc_evaluator/src/ssa/ssa_gen/context.rs |  8 ++++----
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs
index 980fab05d9..d1615c6d1a 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs
@@ -134,7 +134,7 @@ impl Context<'_> {
             let pow = self.pow(base, rhs);
             let pow = self.insert_cast(pow, typ);
 
-            // TODO: should this be unchecked?
+            // Unchecked mul because `predicate` will be 1 or 0
             (
                 FieldElement::max_num_bits(),
                 self.insert_binary(predicate, BinaryOp::Mul { unchecked: false }, pow),
@@ -218,22 +218,18 @@ impl Context<'_> {
             let rhs_bits = rhs_bits[0];
             let one = self.field_constant(FieldElement::one());
             let mut r = one;
+            // All operations are unchecked as we're acting on Field types (which are always unchecked)
             for i in 1..bit_size + 1 {
-                // TODO: should this be unchecked?
-                let r_squared = self.insert_binary(r, BinaryOp::Mul { unchecked: false }, r);
-                // TODO: should this be unchecked?
-                let a = self.insert_binary(r_squared, BinaryOp::Mul { unchecked: false }, lhs);
+                let r_squared = self.insert_binary(r, BinaryOp::Mul { unchecked: true }, r);
+                let a = self.insert_binary(r_squared, BinaryOp::Mul { unchecked: true }, lhs);
                 let idx = self.field_constant(FieldElement::from((bit_size - i) as i128));
                 let b = self.insert_array_get(rhs_bits, idx, Type::bool());
                 let not_b = self.insert_not(b);
                 let b = self.insert_cast(b, NumericType::NativeField);
                 let not_b = self.insert_cast(not_b, NumericType::NativeField);
-                // TODO: should this be unchecked?
-                let r1 = self.insert_binary(a, BinaryOp::Mul { unchecked: false }, b);
-                // TODO: should this be unchecked?
-                let r2 = self.insert_binary(r_squared, BinaryOp::Mul { unchecked: false }, not_b);
-                // TODO: should this be unchecked?
-                r = self.insert_binary(r1, BinaryOp::Add { unchecked: false }, r2);
+                let r1 = self.insert_binary(a, BinaryOp::Mul { unchecked: true }, b);
+                let r2 = self.insert_binary(r_squared, BinaryOp::Mul { unchecked: true }, not_b);
+                r = self.insert_binary(r1, BinaryOp::Add { unchecked: true }, r2);
             }
             r
         } else {
diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
index 12ca9aeb5e..c3ca161895 100644
--- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
+++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
@@ -330,7 +330,7 @@ impl<'a> FunctionContext<'a> {
             BinaryOp::Mul { unchecked: false },
             two_complement,
         );
-        // TODO: should this be unchecked?
+        // Unchecked addition because either `positive_predicate` or `negative_predicate` will be 0
         self.builder.insert_binary(
             positive_predicate,
             BinaryOp::Add { unchecked: false },
@@ -490,10 +490,10 @@ impl<'a> FunctionContext<'a> {
                 //Check the result has the same sign as its inputs
                 let result_sign = self.builder.insert_binary(result, BinaryOp::Lt, half_width);
                 let sign_diff = self.builder.insert_binary(result_sign, BinaryOp::Eq, lhs_sign);
-                // TODO: should this be unchecked?
+                // Unchecked multiplication because boolean inputs
                 let sign_diff_with_predicate = self.builder.insert_binary(
                     sign_diff,
-                    BinaryOp::Mul { unchecked: false },
+                    BinaryOp::Mul { unchecked: true },
                     same_sign,
                 );
                 let overflow_check = Instruction::Constrain(
@@ -527,7 +527,7 @@ impl<'a> FunctionContext<'a> {
                 let not_same = self.builder.insert_not(same_sign);
                 let not_same_sign_field =
                     self.insert_safe_cast(not_same, NumericType::unsigned(bit_size), location);
-                // TODO: should this be unchecked
+                // TODO: should this be unchecked?
                 let positive_maximum_with_offset = self.builder.insert_binary(
                     half_width,
                     BinaryOp::Add { unchecked: false },