Skip to content
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

Fix issue where exhaustiveness check for pattern matching char was no… #5744

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

cristianoc
Copy link
Collaborator

…t working

Fixes #5557 Fixes #5743

Exhaustiveness check for pattern matching relies on checking that the pattern does not lead to a type error. Since char constants are represented internally as int, this causes an internal type error when processing patterns of the form 'x', where the constant is represented as Pconst_integer but the expected type is char.

This PR intercepts this situation and changes the expected type to int. A cleaner solution would be to represent chars differently (but we can't change the AST), or make it official that char and int is the same type (and lose some abstraction).

…t working

Fixes #5557
Fixes #5743

Exhaustiveness check for pattern matching relies on checking that the pattern does not lead to a type error.
Since char constants are represented internally as int, this causes an internal type error when processing patterns of the form 'x', where the constant is represented as `Pconst_integer` but the expected type is `char`.

This PR intercepts this situation and changes the expected type to `int`.
A cleaner solution would be to represent chars differently (but we can't change the AST), or make it official that char and int is the same type (and lose some abstraction).
jscomp/ml/typecore.ml Outdated Show resolved Hide resolved
@cristianoc cristianoc merged commit a8b0cfb into 10.1_release Oct 19, 2022
@cristianoc cristianoc deleted the char_exhaustiveness_check branch October 19, 2022 15:44
bobzhang added a commit that referenced this pull request Oct 24, 2022
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.

None yet

2 participants