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: pattern match compilation in the presence of complex constant inlining #871

Merged
merged 3 commits into from
Nov 5, 2023

Conversation

anmonteiro
Copy link
Member

this is a port of rescript-lang/rescript#6471

anmonteiro and others added 3 commits November 4, 2023 23:19
The compiler's back-end has optimisations to inline complex constants.
This can interfere with pattern matching compilation.
In the tests, a person can be a Student or a Teacher.
Because of inlining, the value one pattern matches on is known to be of type Teacher.
However, the compilation of pattern matching still generates code for both student and teacher.
The (dead) code for student is generated, but the inlined value has type teacher. This means that an attempt is made to access non-existent field "status" of teacher.
Notice one could even rename "status" to "age" in Student, and the filed would exist, but just be of unexpected type.
So the constant age for Teacher is interpreted as a status value (the second field of the inline record).
The compiler optimisation expects to find a valid constant for status, while it finds the age.
This PR catches this situation and does not try to follow a specific branch of the "status" cases but reverts to the general case.
@anmonteiro anmonteiro merged commit 4de41c4 into main Nov 5, 2023
2 checks passed
@anmonteiro anmonteiro deleted the anmonteiro/complex-constant-inline-fix branch November 5, 2023 06:42
anmonteiro added a commit that referenced this pull request Nov 5, 2023
anmonteiro added a commit that referenced this pull request Nov 5, 2023
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.

2 participants