Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit be60568

Browse files
committedNov 3, 2023
Fix issue when inlining complex constants in pattern matching.
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.
1 parent 7e776d0 commit be60568

5 files changed

+107
-4
lines changed
 

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#### :bug: Bug Fix
1616
- Fix issue with GenType and `result` introduced in rc.5. https://github.com/rescript-lang/rescript-compiler/pull/6464
17+
- Fix compiler crash when inlining complex constants in pattern matching. https://github.com/rescript-lang/rescript-compiler/pull/6471
1718

1819
# 11.0.0-rc.5
1920

‎jscomp/core/lam.ml

+7-3
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,14 @@ and eq_approx_list ls ls1 = Ext_list.for_all2_no_exn ls ls1 eq_approx
405405
let switch lam (lam_switch : lambda_switch) : t =
406406
match lam with
407407
| Lconst (Const_int { i }) ->
408-
Ext_list.assoc_by_int lam_switch.sw_consts (Int32.to_int i)
409-
lam_switch.sw_failaction
408+
(* Because of inlining and dead code, we might be looking at a value of unexpected type
409+
e.g. an integer, so the const case might not be found *)
410+
(try
411+
Ext_list.assoc_by_int lam_switch.sw_consts (Int32.to_int i) lam_switch.sw_failaction
412+
with _ -> Lswitch(lam, lam_switch))
410413
| Lconst (Const_block (i, _, _)) ->
411-
Ext_list.assoc_by_int lam_switch.sw_blocks i lam_switch.sw_failaction
414+
(try Ext_list.assoc_by_int lam_switch.sw_blocks i lam_switch.sw_failaction
415+
with _ -> Lswitch(lam, lam_switch))
412416
| _ -> Lswitch (lam, lam_switch)
413417

414418
let stringswitch (lam : t) cases default : t =

‎jscomp/test/build.ninja

+2-1
Large diffs are not rendered by default.

‎jscomp/test/inline_condition_with_pattern_matching.js

+61
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
module Test1 = {
2+
type status = Vacations(int) | Sabbatical(int) | Sick
3+
type person =
4+
| Teacher({age: int})
5+
| Student({status: status})
6+
7+
let person1 = Teacher({age: 12345})
8+
9+
let message = switch person1 {
10+
| Student({status: Vacations(_) | Sick}) => "a"
11+
| _ => "b"
12+
}
13+
}
14+
15+
module Test2 = {
16+
type status = Vacations(int) | Sabbatical(int) | Sick | Present
17+
type reportCard = {passing: bool, gpa: float}
18+
type person =
19+
| Teacher({name: string, age: int})
20+
| Student({name: string, status: status, reportCard: reportCard})
21+
22+
let person2 = Teacher({name: "Jane", age: 12345})
23+
24+
let message = switch person2 {
25+
| Teacher({name: "Mary" | "Joe"}) => `Hey, still going to the party on Saturday?`
26+
| Teacher({name}) =>
27+
// this is matched only if `name` isn't "Mary" or "Joe"
28+
`Hello ${name}.`
29+
| Student({name, reportCard: {passing: true, gpa}}) =>
30+
`Congrats ${name}, nice GPA of ${Js.Float.toString(gpa)} you got there!`
31+
| Student({reportCard: {gpa: 0.0}, status: Vacations(daysLeft) | Sabbatical(daysLeft)}) =>
32+
`Come back in ${Js.Int.toString(daysLeft)} days!`
33+
| Student({status: Sick}) => `How are you feeling?`
34+
| Student({name}) => `Good luck next semester ${name}!`
35+
}
36+
}

0 commit comments

Comments
 (0)
Please sign in to comment.