feature(cairo): Support Span<T> destructuring via fixed-size array pa…#9823
feature(cairo): Support Span<T> destructuring via fixed-size array pa…#9823ilyalesokhin-starkware wants to merge 1 commit intomainfrom
Conversation
dbd4689 to
210b40d
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
210b40d to
4f1006f
Compare
8741730 to
dad17c0
Compare
4f1006f to
2621846
Compare
2621846 to
27b5dd6
Compare
27b5dd6 to
e748528
Compare
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware resolved 1 discussion.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on orizi).
orizi
left a comment
There was a problem hiding this comment.
@orizi made 2 comments.
Reviewable status: 0 of 11 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware).
corelib/src/test/language_features/match_test.cairo line 26 at r3 (raw file):
assert_eq!(b, @20); assert_eq!(c, @30); },
Suggestion:
[a, b, c] => {
assert_eq!((*a, *b, *c), (10, 20, 30));
},crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 2603 at r3 (raw file):
match s { [a, b] => *a + *b, [a, b] => *a + *b,
add also:
Suggestion:
match s {
[a, b] | [a, b, _] => *a + *b,
9df5995 to
0aa3fa0
Compare
|
Not sure if I should do that like in Code quote: opt_wildcard_idx = Some(idx);
for group in size_groups.values_mut() {
group.filter.add(idx);
group.patterns.push(None);
} |
|
Done. |
0aa3fa0 to
f765fac
Compare
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware made 1 comment and resolved 1 discussion.
Reviewable status: 0 of 11 files reviewed, 3 unresolved discussions (waiting on orizi).
crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 2603 at r3 (raw file):
Previously, orizi wrote…
add also:
Done.
|
Previously, ilyalesokhin-starkware wrote…
see r5 <-> r6. |
f765fac to
344d7d3
Compare
|
Previously, orizi wrote…
We don't generated code for the unreacheable arms, just diagnostics |
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware made 2 comments.
Reviewable status: 8 of 11 files reviewed, 4 unresolved discussions (waiting on orizi).
crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 354 at r10 (raw file):
elem_ty, ); }
Done.
crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 443 at r10 (raw file):
let snapshot_member_id = members.iter().next().unwrap().1.id; let snapshot_array_ty = members.iter().next().unwrap().1.ty; let snapshot_array_var = graph.new_var(snapshot_array_ty, location);
Done.
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 3 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).
crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):
Previously, ilyalesokhin-starkware wrote…
"can open the type"?
i think i didn't understand the original question.
|
Previously, orizi wrote…
I'm wondering if we are going to have a pattern that is not a catch all pattern. In that case, I can't just break after reaching that pattern. |
|
Previously, ilyalesokhin-starkware wrote…
fox example: if but I think the code to support it would be much more complicated. |
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).
crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):
Previously, ilyalesokhin-starkware wrote…
fox example:
if
Span { snapshot: inner } if inner.len() == 5was supported, then thebreakis incorrect.but I think the code to support it would be much more complicated.
we must cover all patterns - either by explicit matching - or a final _ - same as rust
|
Previously, orizi wrote…
The question is not whether we cover everything. It is whether the code can assume that a struct pattern on a slice is a catch-all pattern. |
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).
crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):
Previously, ilyalesokhin-starkware wrote…
The question is not whether we cover everything.
It is whether the code can assume that a struct pattern on a slice is a catch-all pattern.
yes - match guards would change that - but since we don't have them currently - it is fine.
|
Previously, orizi wrote…
Done. |
6749927 to
cec3f2b
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).
cec3f2b to
9be728f
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed all commit messages and made 1 comment.
Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).
crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 3077 at r13 (raw file):
//! > lowering_diagnostics error[E3004]: Match is non-exhaustive: `(Span { snapshot: _ }, _)` not covered.
this seems like a bug - right?
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.
Reviewed by Cursor Bugbot for commit 9be728f. Configure here.
| } | ||
| // A catch-all struct pattern makes every subsequent arm unreachable, so we stop | ||
| // adding to `size_groups`. The match-completeness pass will flag the dead arms. | ||
| break; |
There was a problem hiding this comment.
Struct pattern break drops subsequent reachable arms in tuples
High Severity
The break after processing a Span { snapshot: ... } struct pattern in create_slice_destructure_chain incorrectly drops all subsequent patterns. While Span { snapshot: inner } is a catch-all for the span dimension, it is NOT a catch-all when nested inside a tuple match — subsequent arms with different patterns on other tuple elements remain reachable. This causes valid exhaustive matches like match (s, val) { ..., (Span { snapshot: inner }, 5) => ..., _ => 0 } to produce a false Match is non-exhaustive compilation error, because the _ => 0 arm is never added to any filter. The test confirms this via the Missing node (node 5) and the spurious error diagnostic.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9be728f. Configure here.
There was a problem hiding this comment.
fixed
|
Previously, orizi wrote…
yes, it seems that the approch of breaking after a wildcard does not work. |
ad2e0ae to
3d1cf88
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi made 2 comments and resolved 1 discussion.
Reviewable status: 9 of 11 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware).
crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 467 at r15 (raw file):
inner_patterns: vec![None; fallback_group.inner_patterns.len()], inner_bindings: fallback_group.inner_bindings.clone(), });
Suggestion:
let group = size_groups.entry(n).or_insert_with(|| fallback_group.clone());crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 549 at r15 (raw file):
// callback. The path string is `"_"` because the match has no `[..]` syntax to name the // fallback case. let mut failure_node = {
Suggestion:
let mut current_node = {| } | ||
| // A catch-all struct pattern makes every subsequent arm unreachable, so we stop | ||
| // adding to `size_groups`. The match-completeness pass will flag the dead arms. | ||
| break; |
There was a problem hiding this comment.
fixed
3d1cf88 to
a7a0903
Compare
|
Done. |
|
Done. |
a7a0903 to
4436dd2
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 3 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).
ilyalesokhin-starkware
left a comment
There was a problem hiding this comment.
@ilyalesokhin-starkware reviewed 11 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).
…tterns
Add support for `let [a, b] = span else { ... }` syntax by introducing
a SliceDestructure flow control node that calls tuple_from_span to
convert Span<T> to [T; N] at runtime.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4436dd2 to
3da6c69
Compare



…tterns
Add support for
let [a, b] = span else { ... }syntax by introducing a SliceDestructure flow control node that calls tuple_from_span to convert Span to [T; N] at runtime.Note
Medium Risk
Touches semantic type-checking and match/let lowering by adding a new flow-control node and runtime conversion path; bugs here could affect pattern matching correctness and generated code for matches involving
Span<T>and fixed-size array patterns.Overview
Adds support for matching/destructuring
Span<T>with fixed-size array patterns (e.g.let [a, b] = span else { ... }andmatch span { [a,b] => ... }).Lowering now builds a size-dispatch chain for
Span<T>patterns via a newFlowControlNode::SliceDestructure, which calls corelibtuple_from_span(withFixedSizedArrayInfoImpl) and handles success/failure branching. Semantic pattern computation is updated to accept fixed-size array patterns onSpantypes, and diagnostics/tests are extended to cover the new behavior.Reviewed by Cursor Bugbot for commit dbd4689. Bugbot is set up for automated code reviews on this repo. Configure here.