chore: replace array operation on 0-len arrays with side effect assertion

[guipublic]

Description

Problem*

Related to #8874

Summary*

Array operations on 0-length arrays are replaced by an assertion on the 'side-effects-enabled'

Additional Context

I tried to do it with a ssa pass, but it seemed overkill for an uncommon case (it cannot use the 'simple optimisation' pass) . I end up doing this transformation directly in acir-gen.

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 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 12e7b91	Previous: 3172cf8	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	202 s	168 s	1.20

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

CC: @TomAFrench

[TomAFrench]

From looking at the intepreter logic, it returns Value::uninitialized on any read when the predicate is inactive whereas this PR only handles zero length arrays.

[jfecher]

I tried to do it with a ssa pass, but it seemed overkill for an uncommon case (it cannot use the 'simple optimisation' pass) . I end up doing this transformation directly in acir-gen.

Maybe in Instruction::simplify we can check if the array argument of ArrayGet/Set's length is 0, and if so replace it with an assert false there? Then again I think that wouldn't work either since the result would be a different type.

Per Tom's comment, the SSA interpreter is just meant to match the SSA so when we fix this we could apply it to the interpreter as well. E.g. return array[0] there unless the length is zero.

[guipublic]

Indeed, that's why I said that this PR relates to #8874 instead of resolving it, because it did not resolve it.

I just updated the interpreter in order to have a similar behaviour than what acir-gen is doing.
I think it is not worth to try to get the exact same behaviour, but using a similar strategy is good nevertheless.
What I did is to use a valid index when the index is out-of-range and side-effects are disabled, except when the array length is zero, since there is no valid index in this case.
I believe that with this we can say that #8874 is resolved?

[jfecher]

related #9195

[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: 2ebafd8	Previous: 92f56e7	Ratio
semaphore-depth-10	0.025 s	0.019 s	1.32

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

CC: @TomAFrench

[jfecher]

Thank you!

[jfecher]

@TomAFrench need a re-review here