Skip to content

Commit

Permalink
Fix issue when inlining complex constants in pattern matching.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cristianoc committed Nov 3, 2023
1 parent 7e776d0 commit 124f1ad
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

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

# 11.0.0-rc.5

Expand Down
10 changes: 7 additions & 3 deletions jscomp/core/lam.ml
Original file line number Diff line number Diff line change
Expand Up @@ -405,10 +405,14 @@ and eq_approx_list ls ls1 = Ext_list.for_all2_no_exn ls ls1 eq_approx
let switch lam (lam_switch : lambda_switch) : t =
match lam with
| Lconst (Const_int { i }) ->
Ext_list.assoc_by_int lam_switch.sw_consts (Int32.to_int i)
lam_switch.sw_failaction
(* Because of inlining and dead code, we might be looking at a value of unexpected type
e.g. an integer, so the const case might not be found *)
(try
Ext_list.assoc_by_int lam_switch.sw_consts (Int32.to_int i) lam_switch.sw_failaction
with _ -> Lswitch(lam, lam_switch))
| Lconst (Const_block (i, _, _)) ->
Ext_list.assoc_by_int lam_switch.sw_blocks i lam_switch.sw_failaction
(try Ext_list.assoc_by_int lam_switch.sw_blocks i lam_switch.sw_failaction
with _ -> Lswitch(lam, lam_switch))
| _ -> Lswitch (lam, lam_switch)

let stringswitch (lam : t) cases default : t =
Expand Down
3 changes: 2 additions & 1 deletion jscomp/test/build.ninja

Large diffs are not rendered by default.

61 changes: 61 additions & 0 deletions jscomp/test/inline_condition_with_pattern_matching.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions jscomp/test/inline_condition_with_pattern_matching.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module Test1 = {
type status = Vacations(int) | Sabbatical(int) | Sick
type person =
| Teacher({age: int})
| Student({status: status})

let person1 = Teacher({age: 12345})

let message = switch person1 {
| Student({status: Vacations(_) | Sick}) => "a"
| _ => "b"
}
}

module Test2 = {
type status = Vacations(int) | Sabbatical(int) | Sick | Present
type reportCard = {passing: bool, gpa: float}
type person =
| Teacher({name: string, age: int})
| Student({name: string, status: status, reportCard: reportCard})

let person2 = Teacher({name: "Jane", age: 12345})

let message = switch person2 {
| Teacher({name: "Mary" | "Joe"}) => `Hey, still going to the party on Saturday?`
| Teacher({name}) =>
// this is matched only if `name` isn't "Mary" or "Joe"
`Hello ${name}.`
| Student({name, reportCard: {passing: true, gpa}}) =>
`Congrats ${name}, nice GPA of ${Js.Float.toString(gpa)} you got there!`
| Student({reportCard: {gpa: 0.0}, status: Vacations(daysLeft) | Sabbatical(daysLeft)}) =>
`Come back in ${Js.Int.toString(daysLeft)} days!`
| Student({status: Sick}) => `How are you feeling?`
| Student({name}) => `Good luck next semester ${name}!`
}
}

0 comments on commit 124f1ad

Please sign in to comment.