fix(semantic): enforce the fixed-size-array size cap on the type form - #10218
Conversation
`verify_fixed_size_array_size` (which rejects sizes > i16::MAX with E2174) was only applied on the value path, so a type like `[felt252; 32768]` in a signature or annotation slipped through while the value `[0; 32768]` errored. An un-capped `[T; N]` reaches sierra generation as `[t].repeat(N)`, so this is also a potential huge allocation from a bare signature. Run the check in `resolve_type_ex`'s fixed-size-array arm too, matching the value path.
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR SummaryLow Risk Overview During Diagnostic tests for illegal sizes were extended to cover these cases. Reviewed by Cursor Bugbot for commit 2d4f08c. Bugbot is set up for automated code reviews on this repo. Configure here. |
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
crates/cairo-lang-semantic/src/types.rs line 691 at r1 (raw file):
}; if let Some(size_int) = size.to_int(db) { verify_fixed_size_array_size(db, diagnostics, size_int, array_syntax)?;
Wouldn't it be better to check after we resolve generics, some where in lowering?
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 eytan-starkware and TomerStarkware).
crates/cairo-lang-semantic/src/types.rs line 691 at r1 (raw file):
Previously, eytan-starkware wrote…
Wouldn't it be better to check after we resolve generics, some where in lowering?
this is a semantic diagnostic - it is just about too long arrays - we don't know anything about sizes before - but we always do know that size must be in i16 range.
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
Summary
Adds validation of fixed-size array size limits (
< 2^15) when the size is inferred from the array literal expression, rather than only when the size is explicitly specified in the type annotation. Previously, a declaration likelet _x = [1_felt252; 32768];(without an explicit type annotation) would not trigger theE2174size limit error. Now,verify_fixed_size_array_sizeis also called during type resolution when the size can be determined from the integer value, ensuring the limit is enforced consistently in both cases.Type of change
Please check one:
Why is this change needed?
The
E2174diagnostic for fixed-size arrays exceeding2^15was only emitted when the size appeared in an explicit type annotation (e.g.,[u32; 32768]). When the size was inferred directly from the array expression (e.g.,[1_felt252; 32768]), the size limit was not checked, allowing invalid array sizes to pass through semantic analysis without an error.What was the behavior or documentation before?
let _x = [1_felt252; 32768];compiled without any diagnostic, even though the size exceeds the allowed maximum of2^15 - 1.What is the behavior or documentation after?
let _x = [1_felt252; 32768];now correctly emits:The size limit is enforced regardless of whether the size is written in the type annotation or inferred from the expression.
Related issue or discussion (if any)
N/A
Additional context
The fix also adds an additional
E2174diagnostic for the type annotation side of[u32; 32768]that was previously missing from the test expectations, confirming both the type and expression positions are now validated.