fix: emit diagnostic for member access on unreduced impl type instead of panicking#9807
Conversation
… of panicking Fixes #9788
|
This change is |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| TypeLongId::ImplType(_) => Err(ctx.diagnostics.report( | ||
| rhs_syntax.stable_ptr(db), | ||
| TypeHasNoMembers { ty: long_ty.intern(ctx.db), member_name }, | ||
| )), |
There was a problem hiding this comment.
Duplicate match arm could be combined with existing
Low Severity
The new TypeLongId::ImplType(_) arm produces an identical TypeHasNoMembers diagnostic to the existing TypeLongId::GenericParameter(_) | TypeLongId::Coupon(_) arm just below it. These could be combined into a single pattern arm (ImplType(_) | GenericParameter(_) | Coupon(_)), reducing duplication and ensuring future changes to the diagnostic stay consistent across all three variants.
Additional Locations (1)
eytan-starkware
left a comment
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on orizi and TomerStarkware).
crates/cairo-lang-semantic/src/expr/compute.rs line 3960 at r1 (raw file):
TypeLongId::ImplType(_) => Err(ctx.diagnostics.report( rhs_syntax.stable_ptr(db), TypeHasNoMembers { ty: long_ty.intern(ctx.db), member_name },
Consider an informative error and not just has no members.
| TypeLongId::ImplType(_) => Err(ctx.diagnostics.report( | ||
| rhs_syntax.stable_ptr(db), | ||
| TypeHasNoMembers { ty: long_ty.intern(ctx.db), member_name }, | ||
| )), |
3 participants
Summary
Replace unreachable panic with proper error handling for member access on unreduced impl associated types. When attempting to access members on
ImplType, the code now reports aTypeHasNoMembersdiagnostic instead of panicking with an unreachable statement.Type of change
Please check one:
Why is this change needed?
The previous implementation would panic with an unreachable statement when encountering member access on unreduced impl associated types (like
H::Ain generic contexts). This caused the compiler to crash instead of providing a helpful error message to users.What was the behavior or documentation before?
When accessing members on unreduced impl associated types, the compiler would panic with the message "Impl type should've been reduced" and crash.
What is the behavior or documentation after?
The compiler now gracefully handles this case by reporting error E2057: "Type has no members" with the specific type and member name, allowing compilation to continue and providing better user experience.
Related issue or discussion (if any)
Fixes #9788
Additional context
The change also removes an unused import
cairo_lang_debug::DebugWithDbthat was only used in the removed unreachable statement. A test case has been added to verify the new error handling behavior works correctly for generic impl associated types.Note
Low Risk
Low risk: replaces an
unreachable!panic with a standard error path for a specific type case, plus a focused regression test.Overview
Prevents the semantic analyzer from panicking when performing member access on an unreduced
ImplType(e.g.H::A) by reportingTypeHasNoMembersinstead.Removes the now-unused debug import used only for the panic message, and adds a regression test ensuring the new
E2057diagnostic is produced for generic impl-associated type member access.Written by Cursor Bugbot for commit 64b11af. This will update automatically on new commits. Configure here.