Skip to content

feature(cairo): Support Span<T> destructuring via fixed-size array pa…#9823

Merged
ilyalesokhin-starkware merged 1 commit intomainfrom
ilya/span_unpack
Apr 23, 2026
Merged

feature(cairo): Support Span<T> destructuring via fixed-size array pa…#9823
ilyalesokhin-starkware merged 1 commit intomainfrom
ilya/span_unpack

Conversation

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware commented Apr 6, 2026

…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 { ... } and match span { [a,b] => ... }).

Lowering now builds a size-dispatch chain for Span<T> patterns via a new FlowControlNode::SliceDestructure, which calls corelib tuple_from_span (with FixedSizedArrayInfoImpl) and handles success/failure branching. Semantic pattern computation is updated to accept fixed-size array patterns on Span types, 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.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Comment thread crates/cairo-lang-semantic/src/expr/compute.rs Outdated
@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as draft April 6, 2026 13:55
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from main to graphite-base/9823 April 9, 2026 08:38
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/9823 to ilya/span_pattern_semantic April 9, 2026 08:38
Copy link
Copy Markdown
Contributor Author

ilyalesokhin-starkware commented Apr 9, 2026

@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from ilya/span_pattern_semantic to graphite-base/9823 April 9, 2026 13:19
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/9823 to main April 12, 2026 06:22
Comment thread crates/cairo-lang-semantic/src/expr/compute.rs Outdated
Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware resolved 1 discussion.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on orizi).

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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,

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/span_unpack branch 3 times, most recently from 9df5995 to 0aa3fa0 Compare April 14, 2026 08:41
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 465 at r5 (raw file):

                        group.filter.add(idx);
                        group.patterns.push(None);
                    }

Not sure if I should do that like in create_node_for_enum, or just use break here.

Code quote:

                    opt_wildcard_idx = Some(idx);
                    for group in size_groups.values_mut() {
                        group.filter.add(idx);
                        group.patterns.push(None);
                    }

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

corelib/src/test/language_features/match_test.cairo line 26 at r3 (raw file):

            assert_eq!(b, @20);
            assert_eq!(c, @30);
        },

Done.

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 465 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

Not sure if I should do that like in create_node_for_enum, or just use break here.

see r5 <-> r6.

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as ready for review April 14, 2026 10:56
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, orizi wrote…

we must cover all patterns - either by explicit matching - or a final _ - same as rust

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.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, orizi wrote…

yes - match guards would change that - but since we don't have them currently - it is fine.

Done.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as draft April 19, 2026 11:42
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs Outdated
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 3077 at r13 (raw file):

Previously, orizi wrote…

this seems like a bug - right?

yes, it seems that the approch of breaking after a wildcard does not work.

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/span_unpack branch 2 times, most recently from ad2e0ae to 3d1cf88 Compare April 20, 2026 09:58
Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 = {

Comment thread crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs Outdated
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

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(),
                });

Done.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

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 = {

Done.

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as ready for review April 20, 2026 11:32
Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@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).

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware reviewed 11 files and all commit messages.
Reviewable status: :shipit: 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>
Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

@ilyalesokhin-starkware ilyalesokhin-starkware added this pull request to the merge queue Apr 23, 2026
Merged via the queue into main with commit 9811014 Apr 23, 2026
53 checks passed
@ilyalesokhin-starkware ilyalesokhin-starkware deleted the ilya/span_unpack branch April 23, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants