fix!: Ban Fields in for loop indices and bitwise ops

[jfecher]

Description

Problem*

Resolves #3639
Resolves #4193

Summary*

Uses the new TypeVariableKind::Integer in for loops and bitwise operations to prevent Field types from being used there.

Removes the old delayed_type_checks hack.

Additional Context

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [Exceptional Case] 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.

[kevaundray]

Note the PR body is repeated, not sure if you meant to state that it resolves two different issues:

Resolves https://github.com/noir-lang/noir/issues/3639
Resolves https://github.com/noir-lang/noir/issues/3639

[jfecher]

@kevaundray those issues are correct, the other PR didn't quite resolve them since it implemented the new type variable kind but the new kind was unused until this PR

Edit: Whoops, the first is correct but the second was meant to be a different (but also closed) issue. I'll edit the description and re-open both issues for clarity.

[sirasistant]

There is a typing issue in sort_via in std, I commented sort_via and run the println in ssa_gen/codegen_for:

      println!(
            "Type of start index is {:?} type of iterator is {:?} type of end index is {:?}",
            self.builder.type_of_value(start_index),
            index_type,
            self.builder.type_of_value(end_index)
        );

And it gave:
Type of start index is Numeric(NativeField) type of iterator is Numeric(Unsigned { bit_size: 32 }) type of end index is Numeric(NativeField)
in #4290 example (the iterator is inconsistent with the range bounds)
It's weird because it's the same error as before, maybe I did something wrong?

[github-actions]

Changes to circuit sizes

Generated at commit: 639427428a162fb7285591f445a43c27f04688bd, compared to commit: ceb8001e213066bd8a01d90657951ce5f1419f3a

🧾 Summary (10% most significant diffs)

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
array_sort	+28 ❌	+90.32%	+2,817 ❌	+2934.38%
slice_dynamic_index	+110 ❌	+5.29%	+210 ❌	+2.18%

---

Full diff report 👇

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
array_sort	59 (+28)	+90.32%	2,913 (+2,817)	+2934.38%
slice_dynamic_index	2,189 (+110)	+5.29%	9,821 (+210)	+2.18%
slices	601 (+31)	+5.44%	4,294 (+30)	+0.70%

[jfecher]

@sirasistant good catch, I found a bug when unifying TypeVariableKind::Integer where we still succeeded previously on Fields: 6bc4e23

[jfecher]

Wow, ok. So after the change it turns out we were using Fields in for loops in a lot of places. So this is actually quite the breaking change (I've since fixed each of these, hence the changes in the stdlib):

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:21:15
   │
21 │     for _r in 0..rf / 2 {
   │               ---------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:22:18
   │
22 │         for i in 0..state.len() {
   │                  --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:25:18
   │
25 │         for i in 0..state.len() {
   │                  --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:33:15
   │
33 │     for _r in 0..rp {
   │               -----
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:34:18
   │
34 │         for i in 0..state.len() {
   │                  --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:43:15
   │
43 │     for _r in 0..rf / 2 {
   │               ---------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:44:18
   │
44 │         for i in 0..state.len() {
   │                  --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:47:18
   │
47 │         for i in 0..state.len() {
   │                  --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:70:14
   │
70 │     for k in 0..msg.len() {
   │              ------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon/bn254.nr:96:14
   │
96 │     for i in 0..input.len() {
   │              --------------
   │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:105:14
    │
105 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:114:14
    │
114 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:123:14
    │
123 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:132:14
    │
132 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:141:14
    │
141 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:150:14
    │
150 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:159:14
    │
159 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:168:14
    │
168 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:177:14
    │
177 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:186:14
    │
186 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:195:14
    │
195 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:204:14
    │
204 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:213:14
    │
213 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:222:14
    │
222 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/poseidon/bn254.nr:231:14
    │
231 │     for i in 0..input.len() {
    │              --------------
    │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon.nr:37:14
   │
37 │     for r in 0..(ark.len() / state.len()) {
   │              ---------------------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon.nr:38:18
   │
38 │         for i in 0..state.len() {
   │                  --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon.nr:44:22
   │
44 │             for i in 1..state.len() {
   │                      --------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon.nr:67:14
   │
67 │     for k in 0..msg.len() {
   │              ------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon.nr:94:14
   │
94 │     for i in 0..x.len() {
   │              ----------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/poseidon.nr:96:18
   │
96 │         for j in 0..x.len() {
   │                  ----------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/hash/mimc.nr:11:14
   │
11 │     for i in 1..constants.len() {
   │              ------------------
   │

error: The type Field cannot be used in a for loop
    ┌─ std/hash/mimc.nr:120:17
    │
120 │     for elem in array {
    │                 -----
    │

error: The type Field cannot be used in a for loop
    ┌─ std/array.nr:122:14
    │
122 │     for i in 0..a.len() {
    │              ----------
    │

error: The type Field cannot be used in a for loop
  ┌─ std/merkle.nr:9:14
  │
9 │     for i in 0..n {
  │              ----
  │

error: The type Field cannot be used in a for loop
   ┌─ std/sha512.nr:95:14
   │
95 │     for k in 0..msg.len() {
   │              ------------
   │

error: The type Field cannot be used in a for loop
    ┌─ std/ec.nr:164:14
    │
164 │     for i in 0..N_BITS {
    │              ---------
    │

error: Expected type Field, found type u64
   ┌─ std/array.nr:18:48
   │
18 │             let pos = find_index(sorted_index, i);
   │                                                -
   │

error: Types in a binary operation should match, but found Field and u64
   ┌─ std/array.nr:19:20
   │
19 │             assert(sorted_index[pos] == i);
   │                    ----------------------
   │

error: expected type [Field; N], found type [u64; N]
   ┌─ std/array.nr:34:87
   │
34 │     unconstrained fn get_sorting_index<Env>(self, ordering: fn[Env](T, T) -> bool) -> [Field; N] {
   │                                                                                       ---------- expected [Field; N] because of return type
   ·
52 │         result
   │         ------ [u64; N] returned here
   │

error: The type Field cannot be used in a for loop
   ┌─ std/array.nr:59:21
   │
59 │         for elem in self {
   │                     ----
   │

error: The type Field cannot be used in a for loop
   ┌─ std/array.nr:71:18
   │
71 │         for i in 1 .. self.len() {
   │                  ---------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/array.nr:82:21
   │
82 │         for elem in self {
   │                     ----
   │

error: The type Field cannot be used in a for loop
   ┌─ std/array.nr:93:18
   │
93 │         for i in 1 .. self.len() {
   │                  ---------------
   │

error: The type Field cannot be used in a for loop
    ┌─ std/array.nr:102:21
    │
102 │         for elem in self {
    │                     ----
    │

error: The type Field cannot be used in a for loop
    ┌─ std/array.nr:111:21
    │
111 │         for elem in self {
    │                     ----
    │

error: The type Field cannot be used in a for loop
   ┌─ std/collections/bounded_vec.nr:47:18
   │
47 │         for i in 0..array.len() {
   │                  --------------
   │

error: Types in a binary operation should match, but found Field and u64
   ┌─ std/collections/bounded_vec.nr:60:29
   │
60 │             exceeded_len |= i == append_len;
   │                             ---------------
   │

error: Types in a binary operation should match, but found Field and u64
   ┌─ std/collections/bounded_vec.nr:81:29
   │
81 │             exceeded_len |= i == self.len;
   │                             -------------
   │

error: The type Field cannot be used in a for loop
   ┌─ std/slice.nr:38:21
   │
38 │         for elem in other {
   │                     -----
   │

error: The type Field cannot be used in a for loop
   ┌─ std/cmp.nr:24:18
   │
24 │         for i in 0 .. self.len() {
   │                  ---------------
   │

error: The type Field cannot be used in a for loop
    ┌─ std/cmp.nr:201:18
    │
201 │         for i in 0 .. self.len() {
    │                  ---------------
    │

[jfecher]

@sirasistant I've found the core issue in #4290 is a result from a bug in the monomorphizer. I've pulled out the fix here: #4386. In the general case though, this PR will still be required to prevent Fields in for loops.

[Savio-Sou]

cc @noir-lang/developerrelations we have warnings in Nargo for this breaking change for quite a while now; flagging for early heads up still as this might break quite some code from the community once merged and released.

[sirasistant]

Should we make len() return type be an usize (u64)? That'll probably reduce the blast radius of this breaking change, since some of the failures are iterations using 0..arr.len()

[jfecher]

@sirasistant I made that change in this PR but I'm open to debate on whether we should keep it or not.

[sirasistant]

Ah cool, I haven't look at the code yet. That sounds good to me :D

[jfecher]

error: package bumpalo v3.15.1 cannot be built because it requires rustc 1.73.0 or newer, while the currently active rustc version is 1.72.1

Not sure why the CI is selecting bumpalo v3.15.1 when our Cargo.lock specifies v3.13.0

[TomAFrench]

Spoke in person but for posterity, this is because we ignore the lockfile in the "MSRV check" workflow as this ensures that we'll be able to publish to crates.io because we want to know if this will fail before we push out a new version.

We shouldn't block non-release PRs on this however. I'll update this to run on master and raise issues in future.