fix(interpreter): Promote ConstantDoesNotFitInType to non-internal error

.github/benchmark_projects.yml

@@ -25,7 +25,7 @@ projects:
     path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset
     num_runs: 5
     timeout: 250
-    compilation-timeout: 8
+    compilation-timeout: 10
     execution-timeout: 0.35
     compilation-memory-limit: 750
     execution-memory-limit: 300

compiler/noirc_evaluator/src/ssa/interpreter/errors.rs

@@ -58,6 +58,8 @@ pub enum InterpreterError {
     ToRadixFailed { field_id: ValueId, field: FieldElement, radix: u32 },
     #[error("Failed to solve blackbox function {name}: {reason}")]
     BlackBoxError { name: String, reason: String },
+    #[error("Constant `{constant}` does not fit in type `{typ}`")]
+    ConstantDoesNotFitInType { constant: FieldElement, typ: NumericType },
 }
 
 /// These errors can only result from interpreting malformed SSA
@@ -140,6 +142,4 @@ pub enum InternalError {
     UnexpectedInstruction { reason: &'static str },
     #[error("Expected array of {expected_size} elements, got {size}")]
     InvalidInputSize { expected_size: usize, size: usize },
-    #[error("Constant `{constant}` does not fit in type `{typ}`")]
-    ConstantDoesNotFitInType { constant: FieldElement, typ: NumericType },
 }

compiler/noirc_evaluator/src/ssa/interpreter/value.rs

@@ -177,7 +177,7 @@
 
     /// Return an uninitialized value of the given type. This is usually a zeroed
     /// value but we make no guarantee that it is. This is often used as the default
     /// value to return for side-effectful functions like `call` or `array_get` when
     /// side-effects are disabled.
     pub(crate) fn uninitialized(typ: &Type, id: ValueId) -> Value {
         match typ {
@@ -286,10 +286,10 @@
     }
 
     pub fn from_constant(constant: FieldElement, typ: NumericType) -> IResult<NumericValue> {
-        use super::InternalError::{ConstantDoesNotFitInType, UnsupportedNumericType};
-        use super::InterpreterError::Internal;
+        use super::InternalError::UnsupportedNumericType;
+        use super::InterpreterError::{ConstantDoesNotFitInType, Internal};
 
-        let does_not_fit = Internal(ConstantDoesNotFitInType { constant, typ });
+        let does_not_fit = ConstantDoesNotFitInType { constant, typ };
 
         match typ {
             NumericType::NativeField => Ok(Self::Field(constant)),

compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/cast.rs

@@ -4,6 +4,7 @@ use num_bigint::BigUint;
 use crate::ssa::ir::{
     dfg::{DataFlowGraph, simplify::SimplifyResult},
     instruction::Instruction,
+    integer::IntegerConstant,
     types::{NumericType, Type},
     value::{Value, ValueId},
 };
@@ -56,11 +57,15 @@ pub(super) fn simplify_cast(
                 | NumericType::Signed { .. },
                 NumericType::Signed { bit_size },
             ) => {
+                // `IntegerConstant::from_numeric_constant` cannot handle i128
+                if bit_size == 128 {
+                    return None;
+                }
                 // Field/Unsigned -> signed
-                // We only simplify to signed when we are below the maximum signed integer of the destination type.
-                let integer_modulus = BigUint::from(2u128).pow(bit_size - 1);
-                let constant_uint: BigUint = BigUint::from_bytes_be(&constant.to_be_bytes());
-                if constant_uint < integer_modulus {
+                // We could only simplify to signed when we are below the maximum integer of the destination type.
+                // However, we expect that overflow constraints have been generated appropriately that enforce correctness.
+                let integer_constant = IntegerConstant::from_numeric_constant(constant, dst_typ);
+                if integer_constant.is_some() {
                     SimplifiedTo(dfg.make_constant(constant, dst_typ))
                 } else {
                     None
@@ -73,3 +78,56 @@ pub(super) fn simplify_cast(
         None
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::{assert_ssa_snapshot, ssa::ssa_gen::Ssa};
+
+    #[test]
+    fn unsigned_u8_to_i8_safe() {
+        let src = "
+        brillig(inline) predicate_pure fn main f0 {
+          b0():
+            v2 = cast u8 135 as i8
+            v3 = truncate v2 to 8 bits, max_bit_size: 9
+            v4 = cast v3 as u8
+            v6 = lt v4, u8 128
+            constrain v6 == u1 0
+            return v3
+        }
+        ";
+        let ssa = Ssa::from_str_simplifying(src).unwrap();
+
+        assert_ssa_snapshot!(ssa, @r"
+        brillig(inline) predicate_pure fn main f0 {
+          b0():
+            return i8 135
+        }
+        ");
+    }
+
+    #[test]
+    fn unsigned_u8_to_i8_out_of_bounds() {
+        let src = "
+        brillig(inline) predicate_pure fn main f0 {
+          b0():
+            v2 = cast u8 300 as i8
+            v3 = truncate v2 to 8 bits, max_bit_size: 9
+            v4 = cast v3 as u8
+            v6 = lt v4, u8 128
+            constrain v6 == u1 0
+            return v3
+        }
+        ";
+        let ssa = Ssa::from_str_simplifying(src).unwrap();
+
+        // The overflow check would fail here.
+        assert_ssa_snapshot!(ssa, @r"
+        brillig(inline) predicate_pure fn main f0 {
+          b0():
+            constrain u1 1 == u1 0
+            return i8 44
+        }
+        ");
+    }
+}

test_programs/execution_success/regression_8305/Nargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "regression_8305"
+type = "bin"
+authors = [""]
+
+[dependencies]

test_programs/execution_success/regression_8305/Prover.toml

@@ -0,0 +1 @@
+a = -2

test_programs/execution_success/regression_8305/src/main.nr

@@ -0,0 +1,9 @@
+fn main(a: i32) -> pub i32 {
+    let mut r: i32 = 0;
+    let s: i32 = -2;
+    let e: i32 = -1;
+    for _ in (s - 0)..(e + 0) {
+        r = r + a;
+    }
+    r
+}