fix: warn on and discard unreachable statements after break and continue

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -151,13 +151,25 @@ impl Elaborator<'_> {
         let statements_len = block.statements.len();
         let mut statements = Vec::with_capacity(statements_len);
 
+        // If we found a break or continue statement, this holds its location (only for the first one)
+        let mut break_or_continue_location = None;
+        // When encountering a statement after a break or continue we'll error saying it's unreachable,
+        // but we only want to error for the first statement.
+        let mut errored_unreachable = false;
+
         for (i, statement) in block.statements.into_iter().enumerate() {
+            let location = statement.location;
             let statement_target_type = if i == statements_len - 1 { target_type } else { None };
             let (id, stmt_type) =
                 self.elaborate_statement_with_target_type(statement, statement_target_type);
-            statements.push(id);
 
-            if let HirStatement::Semi(expr) = self.interner.statement(&id) {
+            if break_or_continue_location.is_none() {
+                statements.push(id);
+            }
+
+            let stmt = self.interner.statement(&id);
+
+            if let HirStatement::Semi(expr) = stmt {
                 let inner_expr_type = self.interner.id_type(expr);
                 let location = self.interner.expr_location(&expr);
 
@@ -167,7 +179,18 @@ impl Elaborator<'_> {
                 });
             }
 
-            if i + 1 == statements.len() {
+            if let Some(break_or_continue_location) = break_or_continue_location {
+                if !errored_unreachable {
+                    self.push_err(ResolverError::UnreachableStatement {
+                        location,
+                        break_or_continue_location,
+                    });
+                    errored_unreachable = true;
+                }
+            } else if matches!(stmt, HirStatement::Break | HirStatement::Continue) {
+                break_or_continue_location = Some(location);
+                block_type = stmt_type;
+            } else if i + 1 == statements.len() {
                 block_type = stmt_type;
             }
         }

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -200,6 +200,8 @@ pub enum ResolverError {
         "The type parameter `{ident}` is not constrained by the impl trait, self type, or predicates"
     )]
     UnconstrainedTypeParameter { ident: Ident },
+    #[error("Unreachable statement")]
+    UnreachableStatement { location: Location, break_or_continue_location: Location },
 }
 
 impl ResolverError {
@@ -268,7 +270,8 @@ impl ResolverError {
             | ResolverError::NoPredicatesAttributeOnUnconstrained { location, .. }
             | ResolverError::FoldAttributeOnUnconstrained { location, .. }
             | ResolverError::OracleMarkedAsConstrained { location, .. }
-            | ResolverError::LowLevelFunctionOutsideOfStdlib { location } => *location,
+            | ResolverError::LowLevelFunctionOutsideOfStdlib { location }
+            | ResolverError::UnreachableStatement { location, .. } => *location,
             ResolverError::UnusedVariable { ident }
             | ResolverError::UnusedItem { ident, .. }
             | ResolverError::DuplicateField { field: ident }
@@ -827,6 +830,15 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                     ident.location(),
                 )
             }
+            ResolverError::UnreachableStatement { location, break_or_continue_location} => {
+                let mut diagnostic = Diagnostic::simple_warning(
+                    "Unreachable statement".to_string(),
+                    "Unreachable statement".to_string(),
+                    *location,
+                );
+                diagnostic.add_secondary("Any code following this expression is unreachable".to_string(), *break_or_continue_location);
+                diagnostic
+            }
         }
     }
 }

compiler/noirc_frontend/src/tests.rs

@@ -1344,9 +1344,11 @@ fn break_and_continue_in_constrained_fn() {
 #[test]
 fn break_and_continue_outside_loop() {
     let src = r#"
-        unconstrained fn main() {
+        pub unconstrained fn foo() {
             continue;
             ^^^^^^^^^ continue is only allowed within loops
+        }
+        pub unconstrained fn bar() {
             break;
             ^^^^^^ break is only allowed within loops
         }

test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src/main.nr

@@ -1,6 +1,8 @@
 
-        unconstrained fn main() {
+        pub unconstrained fn foo() {
             continue;
+        }
+        pub unconstrained fn bar() {
             break;
         }
 

test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src_hash.txt

@@ -1 +1 @@
-15565082428725147447
+3791504174465283486

test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/stderr.txt

@@ -6,9 +6,9 @@ error: continue is only allowed within loops
   │
 
 error: break is only allowed within loops
-  ┌─ src/main.nr:4:13
+  ┌─ src/main.nr:6:13
   │
-4 │             break;
+6 │             break;
   │             ------
   │
 

test_programs/execution_success/brillig_continue_break/Nargo.toml

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

test_programs/execution_success/brillig_continue_break/src/main.nr

@@ -0,0 +1,22 @@
+fn main() {
+    // Safety: test program
+    let bug = unsafe { foo() };
+    assert(!bug);
+}
+
+unconstrained fn foo() -> bool {
+    let mut i = 0;
+    let mut bug = false;
+    loop {
+        if i == 3 {
+            break;
+            bug = true;
+        } else if i == 2 {
+            i += 1;
+            continue;
+            bug = true;
+        }
+        i += 1;
+    }
+    bug
+}