fix: stop inserting instructions after break and continue

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

@@ -3,6 +3,7 @@ pub mod data_bus;
 use std::{borrow::Cow, collections::BTreeMap, sync::Arc};
 
 use acvm::{FieldElement, acir::circuit::ErrorSelector};
+use im::HashSet;
 use noirc_errors::{
     Location,
     call_stack::{CallStack, CallStackId},
@@ -40,6 +41,8 @@ use super::{
 pub struct FunctionBuilder {
     pub current_function: Function,
     current_block: BasicBlockId,
+    current_block_closed: bool,
+    closed_blocked: HashSet<(FunctionId, BasicBlockId)>,
     finished_functions: Vec<Function>,
     call_stack: CallStackId,
     error_types: BTreeMap<ErrorSelector, HirType>,
@@ -61,6 +64,8 @@ impl FunctionBuilder {
         let new_function = Function::new(function_name, function_id);
         Self {
             current_block: new_function.entry_block(),
+            current_block_closed: false,
+            closed_blocked: HashSet::default(),
             current_function: new_function,
             finished_functions: Vec::new(),
             call_stack: CallStackId::root(),
@@ -126,7 +131,8 @@ impl FunctionBuilder {
         let call_stack = self.current_function.dfg.get_call_stack(self.call_stack);
         let mut new_function = Function::new(name, function_id);
         new_function.set_runtime(runtime_type);
-        self.current_block = new_function.entry_block();
+        self.switch_to_block(new_function.entry_block());
+
         let old_function = std::mem::replace(&mut self.current_function, new_function);
         // Copy the call stack to the new function
         self.call_stack =
@@ -197,6 +203,17 @@ impl FunctionBuilder {
         self.current_function.dfg.make_block()
     }
 
+    /// Prevents any further instructions from being added to the current block.
+    /// That is, calls to add instructions can be called, but they will have no effect.
+    pub fn close_block(&mut self) {
+        self.closed_blocked.insert((self.current_function.id(), self.current_block));
+        self.current_block_closed = true;
+    }
+
+    pub fn current_block_is_closed(&self) -> bool {
+        self.current_block_closed
+    }
+
     /// Adds a parameter with the given type to the given block.
     /// Returns the newly-added parameter.
     pub fn add_block_parameter(&mut self, block: BasicBlockId, typ: Type) -> ValueId {
@@ -214,7 +231,12 @@ impl FunctionBuilder {
         instruction: Instruction,
         ctrl_typevars: Option<Vec<Type>>,
     ) -> InsertInstructionResult {
+        if self.current_block_closed {
+            return InsertInstructionResult::InstructionRemoved;
+        }
+
         let block = self.current_block();
+
         if self.simplify {
             self.current_function.dfg.insert_instruction_and_results(
                 instruction,
@@ -237,6 +259,8 @@ impl FunctionBuilder {
     /// instructions into a new function, call new_function instead.
     pub fn switch_to_block(&mut self, block: BasicBlockId) {
         self.current_block = block;
+        self.current_block_closed =
+            self.closed_blocked.contains(&(self.current_function.id(), block));
     }
 
     /// Returns the block currently being inserted into

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

@@ -336,6 +336,12 @@ impl FunctionContext<'_> {
         let mut result = Self::unit_value();
         for expr in block {
             result = self.codegen_expression(expr)?;
+
+            // A break or continue might happen in a block, in which case we must
+            // not codegen any further expressions.
+            if self.builder.current_block_is_closed() {
+                break;
+            }
         }
         Ok(result)
     }
@@ -576,9 +582,14 @@ impl FunctionContext<'_> {
         // Compile the loop body
         self.builder.switch_to_block(loop_body);
         self.define(for_expr.index_variable, loop_index.into());
+
         self.codegen_expression(&for_expr.block)?;
-        let new_loop_index = self.make_offset(loop_index, 1);
-        self.builder.terminate_with_jmp(loop_entry, vec![new_loop_index]);
+
+        if !self.builder.current_block_is_closed() {
+            // No need to jump if the current block is already closed
+            let new_loop_index = self.make_offset(loop_index, 1);
+            self.builder.terminate_with_jmp(loop_entry, vec![new_loop_index]);
+        }
 
         // Finish by switching back to the end of the loop
         self.builder.switch_to_block(loop_end);
@@ -1087,9 +1098,14 @@ impl FunctionContext<'_> {
         // Evaluate the rhs first - when we load the expression in the lvalue we want that
         // to reflect any mutations from evaluating the rhs.
         let rhs = self.codegen_expression(&assign.expression)?;
-        let lhs = self.extract_current_value(&assign.lvalue)?;
 
-        self.assign_new_value(lhs, rhs);
+        // Can't assign to a variable if the expression had an unconditional break in it
+        if !self.builder.current_block_is_closed() {
+            let lhs = self.extract_current_value(&assign.lvalue)?;
+
+            self.assign_new_value(lhs, rhs);
+        }
+
         Ok(Self::unit_value())
     }
 
@@ -1101,6 +1117,9 @@ impl FunctionContext<'_> {
     fn codegen_break(&mut self) -> Values {
         let loop_end = self.current_loop().loop_end;
         self.builder.terminate_with_jmp(loop_end, Vec::new());
+
+        self.builder.close_block();
+
         Self::unit_value()
     }
 
@@ -1114,6 +1133,9 @@ impl FunctionContext<'_> {
         } else {
             self.builder.terminate_with_jmp(loop_.loop_entry, vec![]);
         }
+
+        self.builder.close_block();
+
         Self::unit_value()
     }
 

test_programs/compile_success_empty/unconditional_break/Nargo.toml

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

test_programs/compile_success_empty/unconditional_break/src/main.nr

@@ -0,0 +1,16 @@
+fn main() {
+    // Safety:
+    let x = unsafe { func_1() };
+    assert(x);
+}
+
+unconstrained fn func_1() -> bool {
+    let mut x = true;
+    loop {
+        if true {
+            break;
+        }
+        x = false;
+    }
+    x
+}