reverting To(Bits|Radix)Unsafe changes, wip testing 'self.range_const…

…raint(input_expr_witness, bit_size)?;' for to_radix

compiler/noirc_evaluator/src/acir/generated_acir.rs

@@ -368,12 +368,17 @@ impl<F: AcirField> GeneratedAcir<F> {
             "ICE: Radix must be a power of 2"
         );
 
+        // TODO: is this range_constraint useful?
+        let input_expr_witness = self.create_witness_for_expression(input_expr);
+        self.range_constraint(input_expr_witness, bit_size)?;
+
         let limb_witnesses = self.brillig_to_radix(input_expr, radix, limb_count);
 
         let mut composed_limbs = Expression::default();
 
         let mut radix_pow = BigUint::from(1u128);
         for limb_witness in &limb_witnesses {
+            // TODO: do we need both range_constraint's?
             self.range_constraint(*limb_witness, bit_size)?;
 
             composed_limbs = composed_limbs.add_mul(

compiler/noirc_evaluator/src/acir/mod.rs

@@ -2148,7 +2148,7 @@ impl<'a> Context<'a> {
             Intrinsic::ApplyRangeConstraint => {
                 unreachable!("ICE: `Intrinsic::ApplyRangeConstraint` calls should be transformed into an `Instruction::RangeCheck`");
             }
-            Intrinsic::ToRadixUnsafe(endian) => {
+            Intrinsic::ToRadix(endian) => {
                 let field = self.convert_value(arguments[0], dfg).into_var()?;
                 let radix = self.convert_value(arguments[1], dfg).into_var()?;
 
@@ -2167,7 +2167,7 @@ impl<'a> Context<'a> {
                     )
                     .map(|array| vec![array])
             }
-            Intrinsic::ToBitsUnsafe(endian) => {
+            Intrinsic::ToBits(endian) => {
                 let field = self.convert_value(arguments[0], dfg).into_var()?;
 
                 let Type::Array(result_type, array_length) = dfg.type_of_value(result_ids[0])

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -496,7 +496,7 @@ impl<'block> BrilligBlock<'block> {
                                 arguments,
                             );
                         }
-                        Intrinsic::ToBitsUnsafe(endianness) => {
+                        Intrinsic::ToBits(endianness) => {
                             let results = dfg.instruction_results(instruction_id);
 
                             let source = self.convert_ssa_single_addr_value(arguments[0], dfg);
@@ -526,7 +526,7 @@ impl<'block> BrilligBlock<'block> {
                             self.brillig_context.deallocate_single_addr(two);
                         }
 
-                        Intrinsic::ToRadixUnsafe(endianness) => {
+                        Intrinsic::ToRadix(endianness) => {
                             let results = dfg.instruction_results(instruction_id);
 
                             let source = self.convert_ssa_single_addr_value(arguments[0], dfg);

compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs

@@ -307,8 +307,8 @@ impl DependencyContext {
                             | Intrinsic::SliceRemove
                             | Intrinsic::StaticAssert
                             | Intrinsic::StrAsBytes
-                            | Intrinsic::ToBitsUnsafe(..)
-                            | Intrinsic::ToRadixUnsafe(..)
+                            | Intrinsic::ToBits(..)
+                            | Intrinsic::ToRadix(..)
                             | Intrinsic::FieldLessThan => {
                                 // Record all the function arguments as parents of the results
                                 self.update_children(&arguments, &results);
@@ -587,8 +587,8 @@ impl Context {
                             | Intrinsic::SliceRemove
                             | Intrinsic::StaticAssert
                             | Intrinsic::StrAsBytes
-                            | Intrinsic::ToBitsUnsafe(..)
-                            | Intrinsic::ToRadixUnsafe(..)
+                            | Intrinsic::ToBits(..)
+                            | Intrinsic::ToRadix(..)
                             | Intrinsic::FieldLessThan => {
                                 self.value_sets.push(instruction_arguments_and_results);
                             }

compiler/noirc_evaluator/src/ssa/function_builder/mod.rs

@@ -558,7 +558,7 @@ mod tests {
         let one = builder.numeric_constant(FieldElement::one(), NumericType::bool());
         let zero = builder.numeric_constant(FieldElement::zero(), NumericType::bool());
 
-        let to_bits_id = builder.import_intrinsic_id(Intrinsic::ToBitsUnsafe(Endian::Little));
+        let to_bits_id = builder.import_intrinsic_id(Intrinsic::ToBits(Endian::Little));
         let input = builder.field_constant(FieldElement::from(7_u128));
         let length = builder.field_constant(FieldElement::from(8_u128));
         let result_types = vec![Type::Array(Arc::new(vec![Type::bool()]), 8)];

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -61,8 +61,8 @@ pub(crate) enum Intrinsic {
     SliceRemove,
     ApplyRangeConstraint,
     StrAsBytes,
-    ToBitsUnsafe(Endian),
-    ToRadixUnsafe(Endian),
+    ToBits(Endian),
+    ToRadix(Endian),
     BlackBox(BlackBoxFunc),
     Hint(Hint),
     AsWitness,
@@ -89,10 +89,10 @@ impl std::fmt::Display for Intrinsic {
             Intrinsic::SliceRemove => write!(f, "slice_remove"),
             Intrinsic::StrAsBytes => write!(f, "str_as_bytes"),
             Intrinsic::ApplyRangeConstraint => write!(f, "apply_range_constraint"),
-            Intrinsic::ToBitsUnsafe(Endian::Big) => write!(f, "to_be_bits_unsafe"),
-            Intrinsic::ToBitsUnsafe(Endian::Little) => write!(f, "to_le_bits_unsafe"),
-            Intrinsic::ToRadixUnsafe(Endian::Big) => write!(f, "to_be_radix_unsafe"),
-            Intrinsic::ToRadixUnsafe(Endian::Little) => write!(f, "to_le_radix_unsafe"),
+            Intrinsic::ToBits(Endian::Big) => write!(f, "to_be_bits"),
+            Intrinsic::ToBits(Endian::Little) => write!(f, "to_le_bits"),
+            Intrinsic::ToRadix(Endian::Big) => write!(f, "to_be_radix"),
+            Intrinsic::ToRadix(Endian::Little) => write!(f, "to_le_radix"),
             Intrinsic::BlackBox(function) => write!(f, "{function}"),
             Intrinsic::Hint(Hint::BlackBox) => write!(f, "black_box"),
             Intrinsic::AsWitness => write!(f, "as_witness"),
@@ -124,7 +124,7 @@ impl Intrinsic {
             | Intrinsic::AsWitness => true,
 
             // These apply a constraint that the input must fit into a specified number of limbs.
-            Intrinsic::ToBitsUnsafe(_) | Intrinsic::ToRadixUnsafe(_) => true,
+            Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) => true,
 
             // These imply a check that the slice is non-empty and should fail otherwise.
             Intrinsic::SlicePopBack | Intrinsic::SlicePopFront | Intrinsic::SliceRemove => true,
@@ -164,8 +164,8 @@ impl Intrinsic {
             // directly depend on the corresponding `enable_side_effect` instruction any more.
             // However, to conform with the expectations of `Instruction::can_be_deduplicated` and
             // `constant_folding` we only use this information if the caller shows interest in it.
-            Intrinsic::ToBitsUnsafe(_)
-            | Intrinsic::ToRadixUnsafe(_)
+            Intrinsic::ToBits(_)
+            | Intrinsic::ToRadix(_)
             | Intrinsic::BlackBox(
                 BlackBoxFunc::MultiScalarMul
                 | BlackBoxFunc::EmbeddedCurveAdd
@@ -203,10 +203,10 @@ impl Intrinsic {
             "slice_insert" => Some(Intrinsic::SliceInsert),
             "slice_remove" => Some(Intrinsic::SliceRemove),
             "str_as_bytes" => Some(Intrinsic::StrAsBytes),
-            "to_le_radix_unsafe" => Some(Intrinsic::ToRadixUnsafe(Endian::Little)),
-            "to_be_radix_unsafe" => Some(Intrinsic::ToRadixUnsafe(Endian::Big)),
-            "to_le_bits_unsafe" => Some(Intrinsic::ToBitsUnsafe(Endian::Little)),
-            "to_be_bits_unsafe" => Some(Intrinsic::ToBitsUnsafe(Endian::Big)),
+            "to_le_radix" => Some(Intrinsic::ToRadix(Endian::Little)),
+            "to_be_radix" => Some(Intrinsic::ToRadix(Endian::Big)),
+            "to_le_bits" => Some(Intrinsic::ToBits(Endian::Little)),
+            "to_be_bits" => Some(Intrinsic::ToBits(Endian::Big)),
             "as_witness" => Some(Intrinsic::AsWitness),
             "is_unconstrained" => Some(Intrinsic::IsUnconstrained),
             "derive_pedersen_generators" => Some(Intrinsic::DerivePedersenGenerators),
@@ -220,7 +220,7 @@ impl Intrinsic {
     }
 }
 
-/// The endian-ness of bits when encoding values as bits in e.g. ToBitsUnsafe or ToRadixUnsafe
+/// The endian-ness of bits when encoding values as bits in e.g. ToBits or ToRadix
 #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
 pub(crate) enum Endian {
     Big,

compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs

@@ -52,16 +52,16 @@ pub(super) fn simplify_call(
         arguments.iter().map(|value_id| dfg.get_numeric_constant(*value_id)).collect();
 
     let simplified_result = match intrinsic {
-        Intrinsic::ToBitsUnsafe(endian) => {
+        Intrinsic::ToBits(endian) => {
             // TODO: simplify to a range constraint if `limb_count == 1`
             if let (Some(constant_args), Some(return_type)) = (constant_args, return_type.clone()) {
                 let field = constant_args[0];
                 let limb_count = if let Type::Array(_, array_len) = return_type {
                     array_len
                 } else {
-                    unreachable!("ICE: Intrinsic::ToRadixUnsafe return type must be array")
+                    unreachable!("ICE: Intrinsic::ToRadix return type must be array")
                 };
-                constant_to_radix_unsafe(endian, field, 2, limb_count, |values| {
+                constant_to_radix(endian, field, 2, limb_count, |values| {
                     make_constant_array(
                         dfg,
                         values.into_iter(),
@@ -74,17 +74,17 @@ pub(super) fn simplify_call(
                 SimplifyResult::None
             }
         }
-        Intrinsic::ToRadixUnsafe(endian) => {
+        Intrinsic::ToRadix(endian) => {
             // TODO: simplify to a range constraint if `limb_count == 1`
             if let (Some(constant_args), Some(return_type)) = (constant_args, return_type.clone()) {
                 let field = constant_args[0];
                 let radix = constant_args[1].to_u128() as u32;
                 let limb_count = if let Type::Array(_, array_len) = return_type {
                     array_len
                 } else {
-                    unreachable!("ICE: Intrinsic::ToRadixUnsafe return type must be array")
+                    unreachable!("ICE: Intrinsic::ToRadix return type must be array")
                 };
-                constant_to_radix_unsafe(endian, field, radix, limb_count, |values| {
+                constant_to_radix(endian, field, radix, limb_count, |values| {
                     make_constant_array(
                         dfg,
                         values.into_iter(),
@@ -641,7 +641,7 @@ fn make_array(
 }
 
 /// Returns a slice (represented by a tuple (len, slice)) of constants corresponding to the limbs of the radix decomposition.
-fn constant_to_radix_unsafe(
+fn constant_to_radix(
     endian: Endian,
     field: FieldElement,
     radix: u32,

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

@@ -1558,13 +1558,13 @@ mod test {
         // After EnableSideEffectsIf removal:
         brillig(inline) fn main f0 {
           b0(v0: Field, v1: Field, v2: u1):
-            v7 = call to_be_radix_unsafe(v0, u32 256) -> [u8; 1]    // `a.to_be_radix_unsafe(256)`;
+            v7 = call to_be_radix(v0, u32 256) -> [u8; 1]    // `a.to_be_radix(256)`;
             inc_rc v7
-            v8 = call to_be_radix_unsafe(v0, u32 256) -> [u8; 1]    // duplicate load of `a`
+            v8 = call to_be_radix(v0, u32 256) -> [u8; 1]    // duplicate load of `a`
             inc_rc v8
-            v9 = cast v2 as Field                                   // `if c { a.to_be_radix_unsafe(256) }`
-            v10 = mul v0, v9                                        // attaching `c` to `a`
-            v11 = call to_be_radix_unsafe(v10, u32 256) -> [u8; 1]  // calling `to_radix(c * a)`
+            v9 = cast v2 as Field                            // `if c { a.to_be_radix(256) }`
+            v10 = mul v0, v9                                 // attaching `c` to `a`
+            v11 = call to_be_radix(v10, u32 256) -> [u8; 1]  // calling `to_radix(c * a)`
             inc_rc v11
             return
         }
@@ -1573,12 +1573,12 @@ mod test {
         let expected = "
         brillig(inline) fn main f0 {
           b0(v0: Field, v1: Field, v2: u1):
-            v5 = call to_be_radix_unsafe(v0, u32 256) -> [u8; 1]
+            v5 = call to_be_radix(v0, u32 256) -> [u8; 1]
             inc_rc v5
             inc_rc v5
             v6 = cast v2 as Field
             v7 = mul v0, v6
-            v8 = call to_be_radix_unsafe(v7, u32 256) -> [u8; 1]
+            v8 = call to_be_radix(v7, u32 256) -> [u8; 1]
             inc_rc v8
             return
         }

compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

@@ -697,7 +697,7 @@ impl<'f> Context<'f> {
                 }
                 Instruction::Call { func, mut arguments } => match self.inserter.function.dfg[func]
                 {
-                    Value::Intrinsic(Intrinsic::ToBitsUnsafe(_) | Intrinsic::ToRadixUnsafe(_)) => {
+                    Value::Intrinsic(Intrinsic::ToBits(_) | Intrinsic::ToRadix(_)) => {
                         let field = arguments[0];
                         let casted_condition =
                             self.cast_condition_to_value_type(condition, field, call_stack);

compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs

@@ -217,8 +217,7 @@ impl Context<'_> {
     fn pow(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId {
         let typ = self.function.dfg.type_of_value(rhs);
         if let Type::Numeric(NumericType::Unsigned { bit_size }) = typ {
-            let to_bits =
-                self.function.dfg.import_intrinsic(Intrinsic::ToBitsUnsafe(Endian::Little));
+            let to_bits = self.function.dfg.import_intrinsic(Intrinsic::ToBits(Endian::Little));
             let result_types = vec![Type::Array(Arc::new(vec![Type::bool()]), bit_size)];
             let rhs_bits = self.insert_call(to_bits, vec![rhs], result_types);
 

compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs

@@ -173,8 +173,8 @@ impl Context {
                     | Intrinsic::StaticAssert
                     | Intrinsic::ApplyRangeConstraint
                     | Intrinsic::StrAsBytes
-                    | Intrinsic::ToBitsUnsafe(_)
-                    | Intrinsic::ToRadixUnsafe(_)
+                    | Intrinsic::ToBits(_)
+                    | Intrinsic::ToRadix(_)
                     | Intrinsic::BlackBox(_)
                     | Intrinsic::Hint(Hint::BlackBox)
                     | Intrinsic::AsSlice

compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs

@@ -234,8 +234,8 @@ fn slice_capacity_change(
         | Intrinsic::AsWitness
         | Intrinsic::IsUnconstrained
         | Intrinsic::DerivePedersenGenerators
-        | Intrinsic::ToBitsUnsafe(_)
-        | Intrinsic::ToRadixUnsafe(_)
+        | Intrinsic::ToBits(_)
+        | Intrinsic::ToRadix(_)
         | Intrinsic::ArrayRefCount
         | Intrinsic::SliceRefCount
         | Intrinsic::FieldLessThan => SizeChange::None,

compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

@@ -191,10 +191,10 @@ impl<'local, 'context> Interpreter<'local, 'context> {
             "struct_def_module" => struct_def_module(self, arguments, location),
             "struct_def_name" => struct_def_name(interner, arguments, location),
             "struct_def_set_fields" => struct_def_set_fields(interner, arguments, location),
-            "to_be_radix_unsafe" => to_be_radix_unsafe(arguments, return_type, location),
-            "to_le_radix_unsafe" => to_le_radix_unsafe(arguments, return_type, location),
-            "to_be_bits_unsafe" => to_be_bits_unsafe(arguments, return_type, location),
-            "to_le_bits_unsafe" => to_le_bits_unsafe(arguments, return_type, location),
+            "to_be_radix" => to_be_radix(arguments, return_type, location),
+            "to_le_radix" => to_le_radix(arguments, return_type, location),
+            "to_be_bits" => to_be_bits(arguments, return_type, location),
+            "to_le_bits" => to_le_bits(arguments, return_type, location),
             "trait_constraint_eq" => trait_constraint_eq(arguments, location),
             "trait_constraint_hash" => trait_constraint_hash(arguments, location),
             "trait_def_as_trait_constraint" => {
@@ -801,42 +801,42 @@ fn quoted_tokens(arguments: Vec<(Value, Location)>, location: Location) -> IResu
     ))
 }
 
-fn to_be_bits_unsafe(
+fn to_be_bits(
     arguments: Vec<(Value, Location)>,
     return_type: Type,
     location: Location,
 ) -> IResult<Value> {
     let value = check_one_argument(arguments, location)?;
     let radix = (Value::U32(2), value.1);
-    to_be_radix_unsafe(vec![value, radix], return_type, location)
+    to_be_radix(vec![value, radix], return_type, location)
 }
 
-fn to_le_bits_unsafe(
+fn to_le_bits(
     arguments: Vec<(Value, Location)>,
     return_type: Type,
     location: Location,
 ) -> IResult<Value> {
     let value = check_one_argument(arguments, location)?;
     let radix = (Value::U32(2), value.1);
-    to_le_radix_unsafe(vec![value, radix], return_type, location)
+    to_le_radix(vec![value, radix], return_type, location)
 }
 
-fn to_be_radix_unsafe(
+fn to_be_radix(
     arguments: Vec<(Value, Location)>,
     return_type: Type,
     location: Location,
 ) -> IResult<Value> {
-    let le_radix_limbs = to_le_radix_unsafe(arguments, return_type, location)?;
+    let le_radix_limbs = to_le_radix(arguments, return_type, location)?;
 
     let Value::Array(limbs, typ) = le_radix_limbs else {
-        unreachable!("`to_le_radix_unsafe` should always return an array");
+        unreachable!("`to_le_radix` should always return an array");
     };
     let be_radix_limbs = limbs.into_iter().rev().collect();
 
     Ok(Value::Array(be_radix_limbs, typ))
 }
 
-fn to_le_radix_unsafe(
+fn to_le_radix(
     arguments: Vec<(Value, Location)>,
     return_type: Type,
     location: Location,
@@ -863,7 +863,7 @@ fn to_le_radix_unsafe(
         *element_type == Type::Integer(Signedness::Unsigned, IntegerBitSize::One);
 
     // Decompose the integer into its radix digits in little endian form.
-    let decomposed_integer = compute_to_radix_le_unsafe(value, radix);
+    let decomposed_integer = compute_to_radix_le(value, radix);
     let decomposed_integer = vecmap(0..limb_count.to_u128() as usize, |i| {
         let digit = match decomposed_integer.get(i) {
             Some(digit) => *digit,
@@ -885,7 +885,7 @@ fn to_le_radix_unsafe(
     Ok(Value::Array(decomposed_integer.into(), result_type))
 }
 
-fn compute_to_radix_le_unsafe(field: FieldElement, radix: u32) -> Vec<u8> {
+fn compute_to_radix_le(field: FieldElement, radix: u32) -> Vec<u8> {
     let bit_size = u32::BITS - (radix - 1).leading_zeros();
     let radix_big = BigUint::from(radix);
     assert_eq!(BigUint::from(2u128).pow(bit_size), radix_big, "ICE: Radix must be a power of 2");