fix: allow statement-level consts in named argument shorthand instead of panicking#9814
Conversation
… of panicking Fixes #9789
|
This change is |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 807d98d7c7
ℹ️ 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".
| if let Expr::Var(expr_var) = &res { | ||
| let item = ResolvedGenericItem::Variable(expr_var.var); | ||
| ctx.resolver.data.resolved_items.generic.insert(identifier.stable_ptr(ctx.db), item); | ||
| } |
There was a problem hiding this comment.
When resolve_variable_by_name resolves a statement-level const (Expr::Constant), this new if let Expr::Var path skips inserting any entry into resolved_items. As a result, identifier lookups that rely on lookup_resolved_generic_item_by_ptr / lookup_resolved_concrete_item_by_ptr cannot resolve :x/{ x } shorthand uses of local consts for tooling (e.g., definition/debug mapping), even though the expression now compiles. resolve_expr_path already handles this by inserting a concrete constant entry, so this shorthand path should mirror that behavior.
Useful? React with 👍 / 👎.
2 participants
Summary
Fixed a panic in variable resolution by replacing
extract_matches!macro with safe pattern matching when resolving variables by name. Added test case for statement-level const used in named argument shorthand.Type of change
Please check one:
Why is this change needed?
The
extract_matches!macro was causing a panic when the resolved expression was not a variable expression (Expr::Var). This could occur in certain edge cases where variable resolution returns a different expression type, leading to runtime crashes during semantic analysis.What was the behavior or documentation before?
The code used
extract_matches!(&res, Expr::Var).varwhich would panic if the resolved expressionreswas not of typeExpr::Var, causing the compiler to crash unexpectedly.What is the behavior or documentation after?
The code now uses safe pattern matching with
if let Expr::Var(expr_var) = &resto only insert the resolved item when the expression is actually a variable, preventing panics and allowing the function to continue gracefully in all cases.Related issue or discussion (if any)
Fixes #9789
Additional context
The test case demonstrates that statement-level constants used in named argument shorthand should work without diagnostics, which helps ensure this fix doesn't break legitimate use cases.
Note
Medium Risk
Touches core expression/variable resolution in the semantic layer; while the change is small and defensive, it can affect name resolution behavior across the compiler.
Overview
Fixes a panic in
resolve_variable_by_nameby replacing the unconditionalextract_matches!(..., Expr::Var)with safe pattern matching, so non-variable bindings (e.g., statement-levelconst) no longer crash semantic analysis when used via named-argument shorthand.Adds a regression test ensuring a statement-level
constcan be passed withtakes_val(:x)without producing diagnostics.Written by Cursor Bugbot for commit 807d98d. This will update automatically on new commits. Configure here.