diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 74e5dea1797..4cfb4776a05 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/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; } } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 00b3c08cf21..23b59df6910 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/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 + } } } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c4668b4f77a..eef4ad54dc5 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/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 } diff --git a/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src/main.nr b/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src/main.nr index c2cef244801..d1a10c18573 100644 --- a/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src/main.nr +++ b/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; } \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src_hash.txt b/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src_hash.txt index 6c38fcafd58..9aeb4f4e6bd 100644 --- a/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src_hash.txt +++ b/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/src_hash.txt @@ -1 +1 @@ -15565082428725147447 \ No newline at end of file +3791504174465283486 \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/stderr.txt b/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/stderr.txt index bc9eedb2b46..7beffa5c877 100644 --- a/test_programs/compile_failure/noirc_frontend_tests_break_and_continue_outside_loop/stderr.txt +++ b/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; │ ------ │ diff --git a/test_programs/execution_success/brillig_continue_break/Nargo.toml b/test_programs/execution_success/brillig_continue_break/Nargo.toml new file mode 100644 index 00000000000..d41d22c3b20 --- /dev/null +++ b/test_programs/execution_success/brillig_continue_break/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "brillig_continue_break" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/brillig_continue_break/src/main.nr b/test_programs/execution_success/brillig_continue_break/src/main.nr new file mode 100644 index 00000000000..efd5224bb8d --- /dev/null +++ b/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 +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap new file mode 100644 index 00000000000..0c61ab625eb --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap @@ -0,0 +1,20 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": "H4sIAAAAAAAA/5XDsQkAAADCMAv+f7PuTgaCFm19Arunwj5IAAAA", + "debug_symbols": "ndHBCoMwDAbgd+m5h2lV1FcZQ2qNUihtqa0wxHdfFOvcYDA8pcnfL5fMpIM2DI3UvRlJfZ9J66RScmiUEdxLo3E6L5TEtvEOAEfklKOy3IH2pNZBKUomrsL2abRcb9Vzh+mNEtAdVlzYSwXra6FvfftNkyzdcZKxg+f/++rwVXnBp0X0aXHJlyz6srrgWZ7vnhXJh39gx4V03xebuJO8VbC3fdDilPqnjUm8uHVGQBccrJu2DHe/AA==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_0.snap new file mode 100644 index 00000000000..0c61ab625eb --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_0.snap @@ -0,0 +1,20 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": "H4sIAAAAAAAA/5XDsQkAAADCMAv+f7PuTgaCFm19Arunwj5IAAAA", + "debug_symbols": "ndHBCoMwDAbgd+m5h2lV1FcZQ2qNUihtqa0wxHdfFOvcYDA8pcnfL5fMpIM2DI3UvRlJfZ9J66RScmiUEdxLo3E6L5TEtvEOAEfklKOy3IH2pNZBKUomrsL2abRcb9Vzh+mNEtAdVlzYSwXra6FvfftNkyzdcZKxg+f/++rwVXnBp0X0aXHJlyz6srrgWZ7vnhXJh39gx4V03xebuJO8VbC3fdDilPqnjUm8uHVGQBccrJu2DHe/AA==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_9223372036854775807.snap b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_9223372036854775807.snap new file mode 100644 index 00000000000..0c61ab625eb --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_false_inliner_9223372036854775807.snap @@ -0,0 +1,20 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": "H4sIAAAAAAAA/5XDsQkAAADCMAv+f7PuTgaCFm19Arunwj5IAAAA", + "debug_symbols": "ndHBCoMwDAbgd+m5h2lV1FcZQ2qNUihtqa0wxHdfFOvcYDA8pcnfL5fMpIM2DI3UvRlJfZ9J66RScmiUEdxLo3E6L5TEtvEOAEfklKOy3IH2pNZBKUomrsL2abRcb9Vzh+mNEtAdVlzYSwXra6FvfftNkyzdcZKxg+f/++rwVXnBp0X0aXHJlyz6srrgWZ7vnhXJh39gx4V03xebuJO8VbC3fdDilPqnjUm8uHVGQBccrJu2DHe/AA==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap new file mode 100644 index 00000000000..f10e988888d --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap @@ -0,0 +1,32 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": "H4sIAAAAAAAA/7VVUW6DMAw1TWjHEFq1/Ww/2wH2sTCYqDRNmrRdBLXiHBy9INmNZbn9AMcSSsDmvfjlQTKIkeHoYUUQyA+OBY4blnfT9Yv3YV3UheC1xD+Eti2U/gzX3xSImaXBD4SfSP+wQ5z/MeLzXoj3brpKNq8g+iNV/7R/Kft/utEzeedvjDkw5C5RyxehZaJem4physiUZ06p4X54w3ml1AHLSX09m3NM+ZyC9M9F/TOOpJtj71t65JHhguAqIfrEj/b83SEE4nVMB20PPcvz+le8v1f68CvWOXR9PTT90H/1p1N77KVOc2yYTtzr0hdyXRLH3cCW3w2A3d7v4fr/oVJ4l/awV7DI99Lfhv1dzi9vj/1J2Lk9dr2dMN4R6wF5tkxf6Ss653Kl1okc1X7jONd+GHH5K1wd42oWcIFSS7prHt4xviU6WvJp39b8vzgD59JII90KAAA=", + "debug_symbols": "nZPNioQwDIDfpece7L/6KsMgVetQKFU6urCI777RqOscFhYv/YzJlzbQzqR19fSqfOz6NykfM6mTD8G/qtA3dvR9hL8zydaFSVIySphCaIRB5IhiA4d6DmAIjhAbBEQCIBASoRAaYRD5BomVEiolwCAgpyhRsIMGQE8DEAiFOY05g4CTmWWh5JirGpNz61iXQWH8wSYXR1LGKQRKvmyYtqL3YOPG0SbIZpS42AKhYeeDW78W+mtnf6ta7K7OT1n9284POzc3bCb5rjMp7vjF6Rd3Ts/14fNb0/NzfJ4XN3yh1O4LzT78J0S28enj2i9rp+RtHdwedlNsLtnxezgyx7MZUt+4dkpu7XR5O7A+uKIiey7rbj8=", + "file_map": { + "50": { + "source": "fn main() {\n // Safety: test program\n let bug = unsafe { foo() };\n assert(!bug);\n}\n\nunconstrained fn foo() -> bool {\n let mut i = 0;\n let mut bug = false;\n loop {\n if i == 3 {\n break;\n bug = true;\n } else if i == 2 {\n i += 1;\n continue;\n bug = true;\n }\n i += 1;\n }\n bug\n}\n", + "path": "" + } + }, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_0.snap new file mode 100644 index 00000000000..3c72fc2e0f5 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_0.snap @@ -0,0 +1,32 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": "H4sIAAAAAAAA/7VWsQ6CQAw94FAQjUYnBwcHBxc9FKMjgz9y0fAdfLqQtF7THAxQmpDe0foe70GFQLkIIGs1IhDkATmFHJJ61Bwl7M24yFPGK4n/MkWRevQJXv89BcxgGnyD+BP5b+aA864dPtWCvElzZGS9VO75mEo/3r8p9e96NGeg9Qr7FetV7HeK1SLPuZD5FpJaKaRp06Np5eEdq4H3lMI6+L2n3Frev/9Mx/LYN8SeyWPnLeYesNbAMyfe8ec2gXXs6dWshr1nyC3XQYgr7uA6Ea7jAC7l6UXffTOSEL4hPkry+WY3YHvNzuMaZyRm/RfIfHak/1O3BFcxrky5bwpdy/M/X8Ygb1Q7H9ATGprUaX8B+0WHjnLgdVZPm1d3W9mH/X6Lj+U+tUHfPT8MAX2E3QkAAA==", + "debug_symbols": "nZNNjoQgEEbvwtqF/Cl4lU6ng4odEoKG1kkmxrtPAerowl644Ykfr6CMzKjV9fR+Gdf1H1Q9ZlR7Y615v2zfqNH0Dt7OKA8DpqgiGcIsgScUCWWCSJARJE9IKwmspAAZQSFjAKjJAVCzAPCEMmUiZTKC4ZgxgqoSQBNYAnjlsmRoO/Rr9FqHMx+6gN4G5bUbUeUmazP0o+wUF30G5SJH5SHNM6RdC4SCnbE6PC3Zv51fqwVd3ULsMj/b+NrGjKw6ZvSOL3df3tmfFJtPbp2fiK19IuQNn3K++rTAV/6Xr79vL8qT/YSZaow//dNLqOONqq1ep93kmkM6/g5bst2JwfeNbievQ6XDxYDxwcqMk+cSdvsD", + "file_map": { + "50": { + "source": "fn main() {\n // Safety: test program\n let bug = unsafe { foo() };\n assert(!bug);\n}\n\nunconstrained fn foo() -> bool {\n let mut i = 0;\n let mut bug = false;\n loop {\n if i == 3 {\n break;\n bug = true;\n } else if i == 2 {\n i += 1;\n continue;\n bug = true;\n }\n i += 1;\n }\n bug\n}\n", + "path": "" + } + }, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_9223372036854775807.snap b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_9223372036854775807.snap new file mode 100644 index 00000000000..3c72fc2e0f5 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/brillig_continue_break/execute__tests__force_brillig_true_inliner_9223372036854775807.snap @@ -0,0 +1,32 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": "H4sIAAAAAAAA/7VWsQ6CQAw94FAQjUYnBwcHBxc9FKMjgz9y0fAdfLqQtF7THAxQmpDe0foe70GFQLkIIGs1IhDkATmFHJJ61Bwl7M24yFPGK4n/MkWRevQJXv89BcxgGnyD+BP5b+aA864dPtWCvElzZGS9VO75mEo/3r8p9e96NGeg9Qr7FetV7HeK1SLPuZD5FpJaKaRp06Np5eEdq4H3lMI6+L2n3Frev/9Mx/LYN8SeyWPnLeYesNbAMyfe8ec2gXXs6dWshr1nyC3XQYgr7uA6Ea7jAC7l6UXffTOSEL4hPkry+WY3YHvNzuMaZyRm/RfIfHak/1O3BFcxrky5bwpdy/M/X8Ygb1Q7H9ATGprUaX8B+0WHjnLgdVZPm1d3W9mH/X6Lj+U+tUHfPT8MAX2E3QkAAA==", + "debug_symbols": "nZNNjoQgEEbvwtqF/Cl4lU6ng4odEoKG1kkmxrtPAerowl644Ykfr6CMzKjV9fR+Gdf1H1Q9ZlR7Y615v2zfqNH0Dt7OKA8DpqgiGcIsgScUCWWCSJARJE9IKwmspAAZQSFjAKjJAVCzAPCEMmUiZTKC4ZgxgqoSQBNYAnjlsmRoO/Rr9FqHMx+6gN4G5bUbUeUmazP0o+wUF30G5SJH5SHNM6RdC4SCnbE6PC3Zv51fqwVd3ULsMj/b+NrGjKw6ZvSOL3df3tmfFJtPbp2fiK19IuQNn3K++rTAV/6Xr79vL8qT/YSZaow//dNLqOONqq1ep93kmkM6/g5bst2JwfeNbievQ6XDxYDxwcqMk+cSdvsD", + "file_map": { + "50": { + "source": "fn main() {\n // Safety: test program\n let bug = unsafe { foo() };\n assert(!bug);\n}\n\nunconstrained fn foo() -> bool {\n let mut i = 0;\n let mut bug = false;\n loop {\n if i == 3 {\n break;\n bug = true;\n } else if i == 2 {\n i += 1;\n continue;\n bug = true;\n }\n i += 1;\n }\n bug\n}\n", + "path": "" + } + }, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/expand/execution_success/brillig_continue_break/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/expand/execution_success/brillig_continue_break/execute__tests__expanded.snap new file mode 100644 index 00000000000..f537bd8d8e9 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/expand/execution_success/brillig_continue_break/execute__tests__expanded.snap @@ -0,0 +1,24 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() { + // Safety: comment added by `nargo expand` + let bug: bool = unsafe { foo() }; + assert(!bug); +} + +unconstrained fn foo() -> bool { + let mut i: Field = 0; + let mut bug: bool = false; + loop { + if i == 3 { + break; + } else if i == 2 { + i = i + 1; + continue; + }; + i = i + 1; + } + bug +}