Skip to content

Commit 244cdad

Browse files
committed
Regression in pattern matching optional fields.
Fixes #7400 Fix regression in pattern matching for optional fields. There's an optimisation where `{field: VariantCase}` is compiled to `field:VariantCase` instead of `field:Some(VariantCase)` to avoid generating code that handles undefined, since `VariantCase` is going to be a string. However, that optimisation happens early in the pattern matching compiler, and can end up interfering with other cases. For example, when `VariantCase` is the only case in a variant, and the following case is `_`, the compiler is induced in thinking that the variant case covers all cases, and the case for `_` is never generated. Similar behaviour happens when handling the last case of a variant followed by `_`.
1 parent 39de4dc commit 244cdad

File tree

6 files changed

+110
-9
lines changed

6 files changed

+110
-9
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
- Fix field flattening optimization to avoid creating unnecessary copies of allocating constants. https://github.com/rescript-lang/rescript-compiler/pull/7421
3232
- Fix leading comments removed when braces inside JSX contains `let` assignment. https://github.com/rescript-lang/rescript/pull/7424
3333
- Fix JSON escaping in code editor analysis: JSON was not always escaped properly, which prevented code actions from being available in certain situations https://github.com/rescript-lang/rescript/pull/7435
34+
- Fix regression in pattern matching for optional fields containing variants. https://github.com/rescript-lang/rescript/pull/7440
3435

3536
#### :house: Internal
3637

compiler/ml/matching.ml

+3-3
Original file line numberDiff line numberDiff line change
@@ -2413,7 +2413,7 @@ let combine_array names loc arg partial ctx def (len_lambda_list, total1, _pats)
24132413

24142414
(* Insertion of debugging events *)
24152415

2416-
let[@inline] event_branch _repr lam = lam
2416+
(* let[@inline] event_branch _repr lam = lam *)
24172417

24182418
(*
24192419
This exception is raised when the compiler cannot produce code
@@ -2602,8 +2602,8 @@ let rec compile_match repr partial ctx m =
26022602
| {cases = ([], action) :: rem} ->
26032603
if is_guarded action then
26042604
let lambda, total = compile_match None partial ctx {m with cases = rem} in
2605-
(event_branch repr (patch_guarded lambda action), total)
2606-
else (event_branch repr action, jumps_empty)
2605+
(patch_guarded lambda action, total)
2606+
else (action, jumps_empty)
26072607
| {args = (arg, str) :: argl} ->
26082608
let v, newarg = arg_to_var arg m.cases in
26092609
let first_match, rem =

compiler/ml/parmatch.ml

-5
Original file line numberDiff line numberDiff line change
@@ -553,11 +553,6 @@ let all_record_args lbls =
553553
in
554554
match cdecl with
555555
| None -> x
556-
| Some cstr
557-
when Ast_untagged_variants.is_nullary_variant cstr.cd_args ->
558-
let _, tag = Ast_untagged_variants.get_cstr_loc_tag cstr in
559-
if Ast_untagged_variants.tag_can_be_undefined tag then x
560-
else (id, lbl, pat_construct, o)
561556
| Some cstr -> (
562557
match
563558
Ast_untagged_variants.get_block_type ~env:pat.pat_env cstr

tests/tests/src/pattern_match_json.mjs

+5-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ function decodeGroup(group) {
2525
}
2626

2727
function decodeNull(x) {
28-
let tmp = x.field;
28+
let match = x.field;
29+
if (match === undefined) {
30+
return "no";
31+
}
32+
let tmp = Primitive_option.valFromOption(match);
2933
if (tmp === null) {
3034
return "yes it's null";
3135
} else {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
4+
function bad1(schema) {
5+
if (schema.format !== undefined) {
6+
return "int32";
7+
} else {
8+
return "default";
9+
}
10+
}
11+
12+
function good1(schema) {
13+
if (schema.format !== undefined) {
14+
return "int32";
15+
} else {
16+
return "default";
17+
}
18+
}
19+
20+
let SingleFormatCase = {
21+
bad1: bad1,
22+
good1: good1
23+
};
24+
25+
function bad2(schema) {
26+
let match = schema.format;
27+
if (match !== undefined) {
28+
if (match === "Int32") {
29+
return "int32";
30+
} else {
31+
return "dd";
32+
}
33+
} else {
34+
return "default";
35+
}
36+
}
37+
38+
function good2(schema) {
39+
let match = schema.format;
40+
if (match !== undefined) {
41+
if (match === "Int32") {
42+
return "int32";
43+
} else {
44+
return "dd";
45+
}
46+
} else {
47+
return "default";
48+
}
49+
}
50+
51+
let MultipleFormatCase = {
52+
bad2: bad2,
53+
good2: good2
54+
};
55+
56+
export {
57+
SingleFormatCase,
58+
MultipleFormatCase,
59+
}
60+
/* No side effect */
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
module SingleFormatCase = {
2+
type format1 = Int32
3+
4+
type schema = {format?: format1}
5+
6+
let bad1 = schema => {
7+
switch schema {
8+
| {format: Int32} => "int32"
9+
| _ => "default"
10+
}
11+
}
12+
13+
let good1 = schema => {
14+
switch schema {
15+
| {format: _} => "int32"
16+
| _ => "default"
17+
}
18+
}
19+
}
20+
21+
module MultipleFormatCase = {
22+
type format2 = Int32 | DD
23+
24+
type schema = {format?: format2}
25+
26+
let bad2 = schema => {
27+
switch schema {
28+
| {format: Int32} => "int32"
29+
| {format: DD} => "dd"
30+
| _ => "default"
31+
}
32+
}
33+
34+
let good2 = schema => {
35+
switch schema {
36+
| {format: Int32} => "int32"
37+
| {format: _} => "dd"
38+
| _ => "default"
39+
}
40+
}
41+
}

0 commit comments

Comments
 (0)