chore(frontend): Loop control flow unit tests

[vezenovm]

Description

Problem*

Working towards the audit of the elaborator group 7a

Summary*

I was looking at the loop and break/continue elaboration semantics. I have added various tests for features that were missing test coverage.

The main change I actually made was in elaborate_jump. We were returning a fresh type var from a break statement. This did not feel like the right place to generate a type variable. break/continue both have known types (Unit). In fact, the only reason we do not inadvertently unify break with an inferred type is because in elaborate_block_expression we patch the block type when the last statement is a break or continue to be a Type::Unit. We should just have a break/continue return a Type::Unit right away. Perhaps in the future we were expecting to break with values like in Rust, but either way we should not have to patch the block expression to get the correct block expression's type.

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.

[github-actions]

⚠️ 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: 07bddac	Previous: 83245db	Ratio
private-kernel-inner	3.348 s	2.1 s	1.59
private-kernel-tail	3.43 s	1.534 s	2.24
rollup-block-root-first-empty-tx	4.25 s	1.398 s	3.04
rollup-block-root-single-tx	4.26 s	1.38 s	3.09
rollup-block-root	4.41 s	1.49 s	2.96
rollup-checkpoint-merge	4.426 s	1.436 s	3.08
rollup-checkpoint-root	573 s	400 s	1.43
rollup-root	4.538 s	1.532 s	2.96
rollup-tx-merge	3.322 s	1.384 s	2.40
semaphore-depth-10	1.846 s	0.803 s	2.30
sha512-100-bytes	2.518 s	1.605 s	1.57

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

CC: @TomAFrench

[github-actions]

⚠️ 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: 07bddac	Previous: 83245db	Ratio
sha512-100-bytes	0.077 s	0.059 s	1.31

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

CC: @TomAFrench

[vezenovm]

These report alerts #10387 (review) look off. Looking at the actual report in CI https://github.com/noir-lang/noir/actions/runs/19113633283/job/54618022377?pr=10387 everything looks normal.

[asterite]

In Rust this is valid code:

fn main() {
    let i = 0;
    loop {
        let x = if i == 3 {
            break;
        } else {
            10
        };
        println!("{}", x);
    }
}

So it might make sense to have break return a type variable.

That said, a similar code in Noir right now doesn't compile. This:

unconstrained fn main() {
    let mut i = 0;
    loop {
        let x = if i == 3 { break; } else { 10 };
        println(x);
    }
}

gives this error:

error: Expected type (), found type Field
  ┌─ src/main.nr:4:45
  │
4 │         let x = if i == 3 { break; } else { 10 };
  │                                             --
  │
  = Are you missing a semicolon at the end of your 'else' branch?

So this PR doesn't break anything. If we'd want to implement the above we'd probably need to revert a few lines from this PR... but it's probably fine.

[vezenovm]

So this PR doesn't break anything. If we'd want to implement the above we'd probably need to revert a few lines from this PR... but it's probably fine.

Yeah I did not want to change the existing functionality. I just wanted to make the type declaration more clear (e.g., removing the type patch when elaborating block expressions).

If we ever wanted to let break statements unify with types other than unit I think that would be best as a separate feature/PR. That would probably require the introduction of the ! type like in Rust. In fact you mentioned that type here #8712 and why unit was chosen. This PR just switches where we declare that a break/continue has a unit type.