fix(semantic): validate inferred numeric literal values against their type - #10040
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR SummaryMedium Risk Overview Lowering no longer runs Test fixtures now expect E2008 under semantic diagnostics and empty lowering literal errors where applicable. Reviewed by Cursor Bugbot for commit 1850335. Bugbot is set up for automated code reviews on this repo. Configure here. |
… type
A suffix-less numeric literal (`let _x: u8 = 256;`, `match x { 256 => .. }`)
got a deferred `NumericLiteral` type var and was never value-validated — only
suffixed (`256_u8`) and const literals were. Out-of-range inferred literals
were silently accepted at semantic analysis and only caught later at lowering
(E3009).
Validate in `apply_inference_rewriter`: once a literal's type var is resolved,
report `E2008` for an out-of-range value (`LiteralError::OutOfRange` only — an
invalid literal type is already reported at conform time). The same
`handle_literal_rewrite` path covers both expression and pattern literals, so
these now error at semantic analysis instead of lowering.
This is now the single validation point for source literals in all contexts,
so the redundant per-literal check in const evaluation is dropped (const eval
bails on a prior error; computed const values are still validated where they
are evaluated). Add a regression test and refresh the lowering `literal`
golden (E3009 → E2008).
4bde605 to
1850335
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bde605df5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 9 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on eytan-starkware and orizi).
orizi
left a comment
There was a problem hiding this comment.
@orizi resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).
Summary
Moves out-of-range literal validation from the lowering phase (
E3009) to the semantic phase (E2008). This is done by performing literal range checks insideapply_inference_rewriterafter inference has resolved the literal's type, and removing the redundant validation that was previously done inConstantEvaluateContext. The lowering test data is updated to reflect that these errors now appear as semantic diagnostics, and a new semantic test case is added for an inferred-type numeric literal whose value is out of range.Type of change
Please check one:
Why is this change needed?
Out-of-range literal errors were being reported during lowering (
E3009) rather than during semantic analysis (E2008). Since the type of a numeric literal is resolved during inference (which happens in the semantic phase), the range check should also occur there. Reporting it at the wrong phase meant the error code was incorrect and the diagnostic was emitted later than necessary.What was the behavior or documentation before?
Out-of-range numeric literal errors were reported as
E3009(a lowering-phase error) after inference had already resolved the literal type. TheLiteralErrorvariant existed inLoweringDiagnosticKindand validation was scattered acrosslower_expr_literal_to_var_usage,create_node_for_value, andhandle_u256_literal.What is the behavior or documentation after?
Out-of-range numeric literal errors are reported as
E2008(a semantic-phase error) duringapply_inference_rewriter, immediately after inference resolves the literal's type. For suffix-less literals whose type was still an unresolvedNumericLiteralvar, onlyOutOfRangeerrors are reported to avoid cascading from already-reported type errors. TheLiteralErrorvariant is removed fromLoweringDiagnosticKindentirely, along with thereporthelper onFlowControlGraphBuilderthat was only used for that purpose.Related issue or discussion (if any)
Additional context
The
ConstantEvaluateContext::validatemethod previously had a dedicatedExpr::Literalarm that re-ran literal validation. This is now removed sinceapply_inference_rewriteralready handles it. Thehandle_u256_literalfunction no longer needs to validate or return aMayberesult, since out-of-range u256 patterns are now caught semantically and the lowering-level arm-dropping logic is removed.