Skip to content

Don't wrap an ErrorInst as a subpattern of another pattern #5542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 27, 2025

Conversation

danakj
Copy link
Contributor

@danakj danakj commented May 27, 2025

When a pattern is an error, avoid wrapping it as a subpattern in a RefParamPattern or VarPattern. This allows further error handling to observe the error occurred without having to unwrap it.

This avoids a crash when an invalid associated constant is written which has a VarPattern in it. The associated constant machinery expects an AssociatedConstantDecl but was getting a VarPattern with an ErrorInst inside. Now it receives an ErrorInst directly, which it is already looking for.

This choice means that var statements in an interface will always be diagnosed as being non-constant, or as being a constant with a var, so we don't need an extra diagnostic saying that var is not allowed in an interface, as this just leads to two diagnostics on the same thing.

This crash was found by a fuzzer.

@github-actions github-actions bot requested a review from josh11b May 27, 2025 16:23
Copy link
Contributor

@josh11b josh11b left a comment

Choose a reason for hiding this comment

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

This is fine, but I wonder if we can (eventually) give the advice that var isn't going to work in an interface in the diagnostic, instead of something suggesting you just have to fix the binding.

@danakj
Copy link
Contributor Author

danakj commented May 27, 2025

When a pattern is an error, avoid wrapping it as a subpattern in a RefParamPattern or VarPattern. This allows further error handling to observe the error occurred without having to unwrap it.

Maybe we can reword the "found runtime binding pattern in associated constant declaration" error if it looks ahead/behind somehow and finds a var.

@danakj danakj added this pull request to the merge queue May 27, 2025
Merged via the queue into carbon-language:trunk with commit 69ab97d May 27, 2025
10 checks passed
@danakj danakj deleted the subpattern branch May 27, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants