IncRc not issued on array reassigns

[sirasistant]

Aim

A program like this one:

unconstrained fn main(items_to_update: u64, index: u64) {
    let mut arr = [0; 5000];
    let mut mid_change = [0; 5000];

    for i in 0..items_to_update {
        if i == 2500 {
            mid_change = arr;
        }
        arr[i] = 27;
    }

    assert_eq(arr[index], 27);
    assert(mid_change[index] != 27);
}

With this prover.toml

items_to_update = 4000
index = 3000

Fails the assertions

Expected Behavior

It should pass with the given inputs

Bug

The SSA generated is missing an inc_rc in mid_change = arr

...
  b1(v7: u64):
    v86 = lt v7, v0
    jmpif v86 then: b2, else: b3
  b2():
    v93 = eq v7, u64 2500
    jmpif v93 then: b4, else: b5
  b4():
    v97 = load v84
    store v97 at v85
    jmp b5()
  b5():
    v94 = load v84
    v95 = array_set v94, index v7, value Field 27
    v96 = add v7, Field 1
    store v95 at v84
    jmp b1(v96)
  b3():
    v87 = load v84
    v88 = array_get v87, index v1
    constrain v88 == Field 27
    v89 = load v85
    v90 = array_get v89, index v1
    v91 = eq v90, Field 27
    v92 = not v91
    constrain v92 == u1 1
    return 
}

To Reproduce

Installation Method

None

Nargo Version

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

[jfecher]

Labelling this P-HIGH since it causes the code to silently change behavior

[kevaundray]

What is the status of this issue?

[jfecher]

@kevaundray it is still open

[jfecher]

I've added the missing IncRc instruction on branch jf/fix-incrc-assign, yet the bug is still present. Any ideas why this may be @sirasistant?

[sirasistant]

This got reverted #4332 because of a regression that made a critical path in aztec-packages to blow up in complexity, never ending. I've added an e2e test in the revert PR of that path.
If the issue is related with rc being a separate value and not together with the items in the array reference, I could try to tackle that @jfecher

[jfecher]

@sirasistant I'll test an older commit of the reverted PR which had a different approach first.