chore: Change enable_side_effects in acir_gen from an `Option<AcirV…

…ar>` to an `AcirVar` (#1906) Enable_side_effects should not be Optional
noir-lang · Jul 10, 2023 · 0628c3c · 0628c3c
1 parent e5fe839
commit 0628c3c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 13 deletions.
diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs
@@ -590,11 +590,11 @@ impl AcirContext {
         lhs: AcirVar,
         rhs: AcirVar,
         bit_size: u32,
-        predicate: Option<AcirVar>,
+        predicate: AcirVar,
     ) -> Result<AcirVar, AcirGenError> {
         // Flip the result of calling more than equal method to
         // compute less than.
-        let comparison = self.more_than_eq_var(lhs, rhs, bit_size, predicate)?;
+        let comparison = self.more_than_eq_var(lhs, rhs, bit_size, Some(predicate))?;
 
         let one = self.add_constant(FieldElement::one());
         self.sub_var(one, comparison) // comparison_negated

diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
@@ -37,7 +37,6 @@ mod acir_ir;
 
 /// Context struct for the acir generation pass.
 /// May be similar to the Evaluator struct in the current SSA IR.
-#[derive(Default)]
 struct Context {
     /// Maps SSA values to `AcirVar`.
     ///
@@ -49,7 +48,7 @@ struct Context {
 
     /// The `AcirVar` that describes the condition belonging to the most recently invoked
     /// `SideEffectsEnabled` instruction.
-    current_side_effects_enabled_var: Option<AcirVar>,
+    current_side_effects_enabled_var: AcirVar,
 
     /// Manages and builds the `AcirVar`s to which the converted SSA values refer.
     acir_context: AcirContext,
@@ -84,7 +83,7 @@ impl Ssa {
         abi_distinctness: AbiDistinctness,
         allow_log_ops: bool,
     ) -> GeneratedAcir {
-        let context = Context::default();
+        let context = Context::new();
         let mut generated_acir = context.convert_ssa(self, brillig, allow_log_ops);
 
         match abi_distinctness {
@@ -110,6 +109,13 @@ impl Ssa {
 }
 
 impl Context {
+    fn new() -> Context {
+        let mut acir_context = AcirContext::default();
+        let current_side_effects_enabled_var = acir_context.add_constant(FieldElement::one());
+
+        Context { ssa_values: HashMap::new(), current_side_effects_enabled_var, acir_context }
+    }
+
     /// Converts SSA into ACIR
     fn convert_ssa(self, ssa: Ssa, brillig: Brillig, allow_log_ops: bool) -> GeneratedAcir {
         let main_func = ssa.main();
@@ -266,7 +272,8 @@ impl Context {
 
                                 let outputs: Vec<AcirType> = vecmap(result_ids, |result_id| dfg.type_of_value(*result_id).into());
 
-                                let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var,code, inputs, outputs);
+                                let output_values = self.acir_context.brillig(Some(self.current_side_effects_enabled_var), code, inputs, outputs);
+
                                 // Compiler sanity check
                                 assert_eq!(result_ids.len(), output_values.len(), "ICE: The number of Brillig output values should match the result ids in SSA");
 
@@ -318,7 +325,7 @@ impl Context {
             }
             Instruction::EnableSideEffects { condition } => {
                 let acir_var = self.convert_numeric_value(*condition, dfg);
-                self.current_side_effects_enabled_var = Some(acir_var);
+                self.current_side_effects_enabled_var = acir_var;
             }
             Instruction::ArrayGet { array, index } => {
                 self.handle_array_operation(instruction_id, *array, *index, None, dfg);
@@ -376,11 +383,9 @@ impl Context {
 
         if index >= array.len() {
             // Ignore the error if side effects are disabled.
-            if let Some(var) = self.current_side_effects_enabled_var {
-                if self.acir_context.is_constant_one(&var) {
-                    // TODO: Can we save a source Location for this error?
-                    panic!("Index {} is out of bounds for array of length {}", index, array.len());
-                }
+            if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) {
+                // TODO: Can we save a source Location for this error?
+                panic!("Index {} is out of bounds for array of length {}", index, array.len());
             }
             let result_type = dfg.type_of_value(dfg.instruction_results(instruction)[0]);
             let value = self.create_default_value(&result_type);
@@ -865,7 +870,7 @@ mod tests {
 
         let ssa = builder.finish();
 
-        let context = Context::default();
+        let context = Context::new();
         let acir = context.convert_ssa(ssa, Brillig::default(), false);
 
         let expected_opcodes =