(feat): add array support to equality analysis#9802
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
bd180d8 to
1f639eb
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware resolved 1 discussion.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on orizi and TomerStarkware).
1f639eb to
960b5dd
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on eytan-starkware and orizi).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 382 at r3 (raw file):
if id == self.array_pop_front || id == self.array_pop_front_consume { // Some arm: var_ids = [remaining_arr, boxed_elem] if arm.var_ids.len() == 2 {
Use arm_selector to distinguish the arms
960b5dd to
a34743e
Compare
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 382 at r3 (raw file):
Previously, TomerStarkware wrote…
Use arm_selector to distinguish the arms
Done.
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 788 at r4 (raw file):
let ty = self.lowered.variables[call_stmt.outputs[0]].ty; info.set_struct_construct(ty, vec![], call_stmt.outputs[0]); } else if id == self.array_append {
how about cases of:
arr.append(3);
let mut span = arr.span();
let x = *span.pop_back();
x is 3 - but since we knew nothing about arr - we didn't propagate through it.
this possibly means that we still want a somewhat different rep for arrays - WDYT?
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 788 at r4 (raw file):
Previously, orizi wrote…
how about cases of:
arr.append(3); let mut span = arr.span(); let x = *span.pop_back();
xis 3 - but since we knew nothing about arr - we didn't propagate through it.this possibly means that we still want a somewhat different rep for arrays - WDYT?
(and in any case add a test with a TODO for this sort of case)
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment.
Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on orizi and TomerStarkware).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 788 at r4 (raw file):
Previously, orizi wrote…
(and in any case add a test with a TODO for this sort of case)
There is a todo to deal with this (maybe on the next PRs) and . I have a concept of Unknown vars I introduce in https://reviewable.io/reviews/starkware-libs/cairo/9804. Array specifically will be somewhat limited as we don't know the len, so I might add support a bit later for this specific case. Would be better to discuss on that PR
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 788 at r4 (raw file):
Previously, eytan-starkware wrote…
There is a todo to deal with this (maybe on the next PRs) and . I have a concept of Unknown vars I introduce in https://reviewable.io/reviews/starkware-libs/cairo/9804. Array specifically will be somewhat limited as we don't know the len, so I might add support a bit later for this specific case. Would be better to discuss on that PR
my claim is that array is very different from structs - since the "unknown var" is a subspan of the array of unknown size. and in struct it is very well known.
a34743e to
718ee31
Compare
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment.
Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on orizi and TomerStarkware).
crates/cairo-lang-lowering/src/analysis/equality_analysis.rs line 788 at r4 (raw file):
Previously, orizi wrote…
my claim is that array is very different from structs - since the "unknown var" is a subspan of the array of unknown size. and in struct it is very well known.
I understand that. Added a test with a todo.
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on TomerStarkware).
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).


Summary
Extends the equality analysis to track array operations (
array_new,array_append,array_pop_front,array_snapshot_pop_front,array_snapshot_pop_back) by reusing the existing struct hashcons infrastructure. Arrays are now represented as(TypeId, Vec<VariableId>)mappings, and pop operations act as destructures that can recover original elements and remaining arrays.Type of change
Please check one:
Why is this change needed?
The equality analysis previously only tracked snapshot/desnap, box/unbox, and struct construct relationships. Array operations were not analyzed, meaning the system couldn't detect when array elements could be recovered from pop operations or when two arrays with equivalent elements should be considered equivalent. This limited optimization opportunities for array-heavy code.
What was the behavior or documentation before?
The equality analysis ignored array operations entirely. Calls to
array_new,array_append, and array pop functions were treated as opaque operations with no tracked relationships between inputs and outputs.What is the behavior or documentation after?
The analysis now:
array_new()as creating an empty array constructarray_append(arr, elem)as extending the array's element listarray_pop_frontby recovering the first element (as boxed) and remaining arrayarray_snapshot_pop_frontandarray_snapshot_pop_backby properly managing snapshot/box relationshipsRelated issue or discussion (if any)
None specified.
Additional context
The implementation cleverly reuses the existing struct hashcons infrastructure since both structs and arrays map
(TypeId, Vec<VariableId>). The analysis correctly handles the complex snapshot and boxing relationships that occur during array pop operations, ensuring that unboxing popped elements yields the correct snapshot types rather than falsely equating snapshot and non-snapshot values.Note
Medium Risk
Updates core dataflow/equality logic to model array operations via hashcons and match-arm transfers, which could affect downstream optimizations that rely on these equivalences. Scope is contained to the equality analysis and its tests but changes touch control-flow transfer behavior and extern call interpretation.
Overview
Adds array awareness to
EqualityAnalysisby reusing the existing struct hashcons to recordarray_new/array_appendconstruct chains and by interpretingarray_pop_front/array_snapshot_pop_front/array_snapshot_pop_backextern matches as destructures that relate the popped (boxed) element and remaining array (including snapshot/box wrappers).Updates the analysis API to require a
Databasehandle for resolving corelib extern IDs, and revises the file-based tests to inline trait calls into extern array calls and adds multiple new test cases covering construct tracking, pop element recovery, snapshot pop chains, and None-arm empty-array handling.Reviewed by Cursor Bugbot for commit 718ee31. Bugbot is set up for automated code reviews on this repo. Configure here.