Skip to content

fix: stop inserting instructions after break and continue#8712

Merged
asterite merged 8 commits intomasterfrom
ab/unconditional-break
Jun 3, 2025
Merged

fix: stop inserting instructions after break and continue#8712
asterite merged 8 commits intomasterfrom
ab/unconditional-break

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented May 29, 2025

Description

Problem

Resolves #8710
Resolves #8629
Resolves #8770

Summary

Now FunctionBuilder tracks whether the current block is closed. If so, no more instructions can be added to it. Then, on the codegen side, when we codegen break or continue we close the current block.

A few cases also need to check if the current block is closed to prevent trying to insert instructions via functions like insert_allocate and so on which check that a result is returned (but none is returned if the instruction is removed because the block is closed).

Finally, I realized that a break doesn't have a type (or, well, the type is a type variable) and that type was given to a block if it had an unconditional break. That's not correct. The correct thing to do would be to give the block the type "no return" or ! in Rust, but we don't have that. Instead, I chose unit as a type. At least it prevents code like 1 + { break; } to compile, and also makes #8629 give a better error.

To give a bit more detail on the last paragraph, the type of break should be ! by itself but, because we don't have that I chose (). However, in Rust this code:

let a = { break; 1 };

has a be i32, not !. That is, the type of a block is always the type of the last statement, regardless of it having a break/continue or not. I chose to do the same thing in Noir. It maybe leads to simpler error messages and more intuitive types.

Additional Context

Documentation

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 16ed24e Previous: 89783b6 Ratio
rollup-merge 0.004 s 0.003 s 1.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: a8b5250 Previous: 89783b6 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 53 s 43 s 1.23

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: b195fef Previous: 58ce59e Ratio
private-kernel-reset 9.074 s 7.35 s 1.23

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@asterite asterite requested a review from a team May 29, 2025 17:40
@asterite asterite enabled auto-merge June 3, 2025 13:49
@asterite asterite added this pull request to the merge queue Jun 3, 2025
Merged via the queue into master with commit 86b3638 Jun 3, 2025
120 of 121 checks passed
@asterite asterite deleted the ab/unconditional-break branch June 3, 2025 14:13
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 6, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: type unification tests, and try moving constants to the other side
(noir-lang/noir#8807)
fix!: disallow casting numeric to bool
(noir-lang/noir#8703)
fix: delay associated constants resolution
(noir-lang/noir#8744)
fix: right shift overflow to 0
(noir-lang/noir#8772)
fix: use value predicated by range checks
(noir-lang/noir#8778)
fix: unify infix expressions by isolating unbound type variables
(noir-lang/noir#8796)
feat: use `asm` feature flag in arkworks
(noir-lang/noir#8792)
chore: add a failing test for #8780
(noir-lang/noir#8785)
chore: Adjust the frequency of 'for' statements in ACIR fuzz generation
(noir-lang/noir#8788)
fix: Merge `replacement_type` and `is_function_type` in
`defunctionalization` (noir-lang/noir#8784)
chore(fuzz): Remove unreachable functions in the AST fuzzer
(noir-lang/noir#8782)
chore(docs): Copy attribute docs into versioned docs
(noir-lang/noir#8777)
fix(licm): Preserve semantic ordering of side-effectual instructions
when hoisting (noir-lang/noir#8724)
fix: Create SSA interpreter arguments from scratch for each invocation
(noir-lang/noir#8762)
chore: mark sha512 as non-critical
(noir-lang/noir#8776)
fix!: disallow specifying associated items via generics
(noir-lang/noir#8756)
fix: stop inserting instructions after break and continue
(noir-lang/noir#8712)
fix: Fix comptime casts of negative integer to field
(noir-lang/noir#8696)
chore(SSA): restrict `shr` and `shl` right-hand side to u8
(noir-lang/noir#8753)
chore: bump some JS packages
(noir-lang/noir#8771)
chore: document `allow(dead_code)` and reorganize attributes
(noir-lang/noir#8766)
fix: Add missing cases for finding function values in
`find_functions_as_values` (noir-lang/noir#8738)
fix: correct bitsize in signed division
(noir-lang/noir#8733)
chore: remove noir-lang/noir_rsa from external libraries
(noir-lang/noir#8752)
chore: bump external pinned commits
(noir-lang/noir#8747)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: type unification tests, and try moving constants to the other side
(noir-lang/noir#8807)
fix!: disallow casting numeric to bool
(noir-lang/noir#8703)
fix: delay associated constants resolution
(noir-lang/noir#8744)
fix: right shift overflow to 0
(noir-lang/noir#8772)
fix: use value predicated by range checks
(noir-lang/noir#8778)
fix: unify infix expressions by isolating unbound type variables
(noir-lang/noir#8796)
feat: use `asm` feature flag in arkworks
(noir-lang/noir#8792)
chore: add a failing test for AztecProtocol#8780
(noir-lang/noir#8785)
chore: Adjust the frequency of 'for' statements in ACIR fuzz generation
(noir-lang/noir#8788)
fix: Merge `replacement_type` and `is_function_type` in
`defunctionalization` (noir-lang/noir#8784)
chore(fuzz): Remove unreachable functions in the AST fuzzer
(noir-lang/noir#8782)
chore(docs): Copy attribute docs into versioned docs
(noir-lang/noir#8777)
fix(licm): Preserve semantic ordering of side-effectual instructions
when hoisting (noir-lang/noir#8724)
fix: Create SSA interpreter arguments from scratch for each invocation
(noir-lang/noir#8762)
chore: mark sha512 as non-critical
(noir-lang/noir#8776)
fix!: disallow specifying associated items via generics
(noir-lang/noir#8756)
fix: stop inserting instructions after break and continue
(noir-lang/noir#8712)
fix: Fix comptime casts of negative integer to field
(noir-lang/noir#8696)
chore(SSA): restrict `shr` and `shl` right-hand side to u8
(noir-lang/noir#8753)
chore: bump some JS packages
(noir-lang/noir#8771)
chore: document `allow(dead_code)` and reorganize attributes
(noir-lang/noir#8766)
fix: Add missing cases for finding function values in
`find_functions_as_values` (noir-lang/noir#8738)
fix: correct bitsize in signed division
(noir-lang/noir#8733)
chore: remove noir-lang/noir_rsa from external libraries
(noir-lang/noir#8752)
chore: bump external pinned commits
(noir-lang/noir#8747)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants