For loops requiring semicolons is unexpected

[TomAFrench]

Description

Expected behavior

For loops shouldn't require a semicolon on their closing brace in order to be valid Noir. This is consistent with Rust's syntax and stated test cases.

Bug

Based on this testcase we shouldn't need semicolons after for loops (This is consistent with Rust syntax that Noir is emulating) https://github.com/noir-lang/noir/blob/1e7c5fa465884ecf021f4f5031e14d15f666c11d/crates/noirc_frontend/src/parser/parser.rs#L1286-L1311

Compiling a Noir program with a for loop with a missing semicolon however gives the following error

$ nargo build
error: Expected type (), found type [()]
   ┌─ /home/tom/Programming/noir/crates/nargo/tests/test_data/6_array/src/main.nr:8:4
   │  
 8 │ ╭    for i in 0..5 {
 9 │ │       c = z*z*y[i];
10 │ │       z = z - c;
11 │ │    }
   │ ╰────'

To reproduce

https://github.com/noir-lang/noir/blob/1e7c5fa465884ecf021f4f5031e14d15f666c11d/crates/nargo/tests/test_data/6_array/src/main.nr#L5-L12

1. Go into nargo/tests/test_data/6_array
2. Delete a semicolon from the closing brace of a for loop
3. Run nargo build

Environment

(Specify your setup and versions of dependencies.)

• OS:

For nargo users

• noir-lang/noir commit cloned: 1e7c5fa465884ecf021f4f5031e14d15f666c11d
• Proving backend
  • [x] default
  • Clang: (run clang --version)
  • [ ] wasm-base

Additional context

(If applicable.)

[jfecher]

This is expected behavior currently. We do not match rust exactly here because for loops are expressions rather than statements in noir. They return an array where each element is the corresponding loop iteration result. This is also why the ; is needed - to discard the expression result when it is unused. The parser tests do not require it because ; is not required syntactically. It is only required to discard the result during type checking. Resultingly, it is more similar to range.into_iter().map(loop_body).collect() in rust.

This feature is currently unimplemented in our IR but may be someday. It is currently needed by design in some cases to create a new array from another, with each element mapped to something else. There are workarounds while it is unimplemented - namely taking advantage of the fact arrays are cloned when passed as parameters and mutating this cloned value and returning it. In the future we may get rid of this for loop feature when there are better ways to dynamically build arrays - e.g. via push/pop functions.

[kevaundray]

@jfecher what are your suggestions to move this issue forward?

[jfecher]

This is completed, for loops no longer return values and thus no longer require semi-colons for discarding the value. Fixed in #514.