Skip to content

fix: handle missing struct field in const evaluation instead of panicking#9812

Open
orizi wants to merge 1 commit intoorizi/03-31-fix_unsupported_extern_fn_in_constfrom
orizi/03-31-fix_missing_struct_field_in_const
Open

fix: handle missing struct field in const evaluation instead of panicking#9812
orizi wants to merge 1 commit intoorizi/03-31-fix_unsupported_extern_fn_in_constfrom
orizi/03-31-fix_missing_struct_field_in_const

Conversation

@orizi
Copy link
Copy Markdown
Collaborator

@orizi orizi commented Mar 31, 2026

Summary

Fixed a panic in constant evaluation when struct literals have missing fields by returning a ConstValue::Missing instead of calling expect(). Added a test case to verify proper error handling for statement-level const struct literals with missing fields.


Type of change

Please check one:

  • Bug fix (fixes incorrect behavior)
  • New feature
  • Performance improvement
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

Why is this change needed?

The constant evaluator was panicking when encountering struct literals with missing fields during constant evaluation. While semantic validation should catch these cases, the evaluator should handle them gracefully rather than crashing with an expect() call.


What was the behavior or documentation before?

The constant evaluator would panic with "Should have been caught by semantic validation" when processing struct literals with missing fields during constant evaluation.


What is the behavior or documentation after?

The constant evaluator now gracefully handles missing struct fields by returning ConstValue::Missing and continues processing, allowing semantic validation to report the appropriate error messages to the user.


Related issue or discussion (if any)

Fixes #9790


Additional context

The fix maintains the existing error reporting behavior while preventing crashes in the constant evaluator. The test case demonstrates that proper error messages are still generated for missing struct fields in const declarations.


Note

Low Risk
Low risk: small, localized change in constant evaluation that replaces a panic with a ConstValue::Missing fallback, plus a targeted diagnostic test.

Overview
Fixes a crash in const evaluation for struct literals with omitted fields by replacing an expect() with a safe fallback to ConstValue::Missing(skip_diagnostic()), allowing compilation to continue and diagnostics to surface normally.

Adds a regression test covering statement-level const declarations with a struct literal missing a field, asserting the missing-member error (and unused-const warning) is reported instead of panicking.

Written by Cursor Bugbot for commit 0284f14. This will update automatically on new commits. Configure here.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Contributor

@eytan-starkware eytan-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).

Copy link
Copy Markdown
Contributor

@eytan-starkware eytan-starkware left a comment

Choose a reason for hiding this comment

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

@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 728 at r1 (raw file):

                                .find(|(_, member_id)| m.id == *member_id)
                                .map(|(expr_id, _)| self.evaluate(*expr_id))
                                // A missing field should have been caught by semantic validation.

Comment is a misleading. It was almost certainly caught, but allowed to continue processing

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