fix(acir): Unify empty vector handling in pop

[aakoshh]

Description

Problem

Resolves https://cantina.xyz/code/50033e8c-8b46-41bc-b019-62098708057b/findings?finding=18

Summary

Unifies the handling of empty-vector checks in vector_pop_front and vector_pop_back.

Additional Context

• fix(acir-gen): Use the side effect variable in slice_pop_back #10455 added some prevention but it only worked for the disabled case.
• fix(ACIR): check vector length is not zero before pop_front #11107 added handling in vector_pop_front and an integration test for vector_pop_back, but the latter failed with the Failed to solve program: 'Index out of bounds, array has size 2, but index was -1' instead of Assertion failed: Attempt to pop_back from an empty vector.

I was a bit preplexed why we need both of the above fixes, but we do, otherwise integration tests fail. If we have a non-constant length, we add an assertion that the length is not zero, which only kicks in if the side effects are enabled, but if they are disabled, we return 0 for the new length, so we end up reading item 0 in both pop versions, rather than item -1.

But how can we read item 0 from an empty vector? The reason this is needed is because we are reading item 0 from a vector which is merged across multiple branches, and isn't actually physically empty, just its semantic length is 0. So we can read item 0 without a problem on the disabled branch. This is illustrated in the first PR linked above.

If we didn't have a case of merging vectors of different lengths, at least one of which is non-zero, then the compiler would know that the length is constant 0, and it would return early from the convert_vector_pop_* methods.

User Documentation

Check one:

• No user documentation needed.
• Changes in docs/ included in this PR.
• [For Experimental Features] Changes in docs/ 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: 18412cd	Previous: 34e3758	Ratio
test_report_zkpassport_noir-ecdsa_	2 s	1 s	2
test_report_zkpassport_noir_rsa_	2 s	1 s	2

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

CC: @TomAFrench

[github-actions]

Changes to circuit sizes

Generated at commit: 00749a2e8e7cec3ab251c3584104325c5539f212, compared to commit: 71c03848e5f26c478bea51e2ed36f0e759aa4e7f

🧾 Summary (10% most significant diffs)

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
regression_10446	+4 ❌	+23.53%	+3 ❌	+3.09%
regression_10170	+10 ❌	+5.95%	+5 ❌	+0.15%

---

Full diff report 👇

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
regression_10446	21 (+4)	+23.53%	100 (+3)	+3.09%
regression_10170	178 (+10)	+5.95%	3,432 (+5)	+0.15%
vector_dynamic_index	806 (+5)	+0.62%	5,162 (+4)	+0.08%
regression_9312	28 (+2)	+7.69%	2,861 (+1)	+0.03%
vectors	325 (+2)	+0.62%	3,936 (+1)	+0.03%

[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: 189e473	Previous: 71c0384	Ratio
rollup-checkpoint-merge	0.003 s	0.002 s	1.50

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

CC: @TomAFrench

[asterite]

👍