fix(semantic): canonicalize felt252 inside NonZero when matching const patterns - #10058
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR SummaryLow Risk Overview Literal pattern matching in constant evaluation now goes through Adds Reviewed by Cursor Bugbot for commit e8ccd86. 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:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
crates/cairo-lang-semantic/src/items/constant.rs line 1266 at r1 (raw file):
ty: TypeId<'db>, } impl<'db> NumericArg<'db> {
Add doc
c460eb2 to
6a8270a
Compare
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/items/constant.rs line 1266 at r1 (raw file):
Previously, eytan-starkware wrote…
Add doc
Done.
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
crates/cairo-lang-semantic/src/items/constant.rs line 1272 at r2 (raw file):
match arg.long(db) { ConstValue::Int(v, ty) => Some(Self { v: v.clone(), ty: *ty }), ConstValue::Struct(v, ty) => {
We should check struct type to verify u256
…t patterns
`destructure_pattern`'s literal arm passed the matched value's OUTER type to
`eq_in_type`, so for a `NonZero<felt252>` const the felt252 canonicalization
(gated on `ty == felt252`) was skipped: `match nz_felt { PRIME-1 => .. }`
compared the raw `-1` vs `PRIME-1` and took the wildcard arm, disagreeing with
`==` (which recurses through NonZero). Route the literal arm through
`NumericArg::try_new`, which already unwraps NonZero and carries the inner
numeric type — the same machinery the const arithmetic path uses.
6a8270a to
e8ccd86
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-semantic/src/items/constant.rs line 1272 at r2 (raw file):
Previously, eytan-starkware wrote…
We should check struct type to verify u256
Done.
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:
Summary
Fixes constant evaluation of
NonZerovalues in match patterns. Previously, when aNonZero-wrapped value appeared as the scrutinee in amatchexpression during constant evaluation, the pattern matching logic did not unwrap theNonZerolayer before comparing against literal patterns, causing the match to fail. The fix propagates theNonZero-unwrapping behavior (already present inNumericArg::try_new) into the literal pattern matching branch, and consolidates thenumeric_arg_valuefree function intoNumericArg::try_newso that the type is correctly carried through after unwrapping.A new test case (
VALID_MATCH_NONZERO_FELT_CROSS_FORM) is added to verify that matching aNonZero<felt252>constant against its field-equivalent hex literal evaluates correctly.Type of change
Please check one:
Why is this change needed?
Matching a
NonZero-wrapped constant value against a literal pattern in aconstcontext would silently fail because the pattern matching code callednumeric_arg_value(which discarded the type after unwrappingNonZero) rather thanNumericArg::try_new(which preserves the unwrapped type). This meant cross-form equality checks usingmatchonNonZerovalues were broken at compile-time constant evaluation.What was the behavior or documentation before?
A
constexpression usingmatchon aNonZero<felt252>value against a literal pattern would not evaluate correctly, even when the values were semantically equal.What is the behavior or documentation after?
NonZero-wrapped values are correctly unwrapped during literal pattern matching in constant evaluation, and the unwrapped type is preserved. Thenumeric_arg_valuehelper is removed in favor of the unifiedNumericArg::try_new.Related issue or discussion (if any)
N/A
Additional context
The line number shifts in the test snapshot reflect the insertion of the new
VALID_MATCH_NONZERO_FELT_CROSS_FORMconstant (6 lines added before the error-producing constants).