Skip to content

Commit 5c00643

Browse files
cristianoccknitt
authored andcommitted
disambiguate optional labels
When disambiguating record types, there's a check that all the labels are supplied when constructing a record. While not supplying all the labels is supported in case of optional labels, the order of disambiguation is affected by the presence of optional labels. Example: ```res type t1 = {x:int, y:int} type t2 = {x:int, y:int, z?:int} let v = {x:3, y:4} ``` Currently `v` has type `t1`, while it's perfectly fine for it to have type `t2`. In particular, the normal shadowing behaviour that applies without optional labels, does not happen. (If you remove `z` from the second type definition, then the normal shadowing happens, and `v` gets type `t2`. This wip changes the disambiguation so that supplying at least all the mandatory labels is enough in disambiguation. The change also addresses the issue #6752 of spurious warning of unused open. # Conflicts: # CHANGELOG.md # jscomp/test/build.ninja
1 parent 3ba36c7 commit 5c00643

File tree

5 files changed

+50
-4
lines changed

5 files changed

+50
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
- Fix location of let bindings with attributes. https://github.com/rescript-lang/rescript-compiler/pull/6791
1818
- PPX v4: mark props type in externals as `@live` to avoid dead code warnings for prop fields in the editor tooling. https://github.com/rescript-lang/rescript-compiler/pull/6796
19+
- Fix issue where optional labels were not taken into account when disambiguating record value construction. https://github.com/rescript-lang/rescript-compiler/pull/6798
1920

2021
# 11.1.1
2122

jscomp/ml/typecore.ml

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,12 @@ let extract_concrete_variant env ty =
311311
| (p0, p, {type_kind=Type_open}) -> (p0, p, [])
312312
| _ -> raise Not_found
313313

314+
let has_optional_labels ld =
315+
match ld.lbl_repres with
316+
| Record_optional_labels _ -> true
317+
| Record_inlined {optional_labels} -> optional_labels <> []
318+
| _ -> false
319+
314320
let label_is_optional ld =
315321
match ld.lbl_repres with
316322
| Record_optional_labels lbls -> Ext_list.mem_string lbls ld.lbl_name
@@ -843,9 +849,15 @@ let disambiguate_label_by_ids keep closed ids labels =
843849
let check_ids (lbl, _) =
844850
let lbls = Hashtbl.create 8 in
845851
Array.iter (fun lbl -> Hashtbl.add lbls lbl.lbl_name ()) lbl.lbl_all;
846-
List.for_all (Hashtbl.mem lbls) ids
847-
and check_closed (lbl, _) =
848-
(not closed || List.length ids = Array.length lbl.lbl_all)
852+
List.for_all (Hashtbl.mem lbls) ids in
853+
let mandatory_labels_are_present num_ids lbl = (* check that all mandatory labels are present *)
854+
if has_optional_labels lbl then (
855+
let mandatory_lbls = ref 0 in
856+
Ext_array.iter lbl.lbl_all (fun l -> if not (label_is_optional l) then incr mandatory_lbls);
857+
num_ids >= !mandatory_lbls)
858+
else num_ids = Array.length lbl.lbl_all in
859+
let check_closed (lbl, _) =
860+
(not closed || mandatory_labels_are_present (List.length ids) lbl)
849861
in
850862
let labels' = Ext_list.filter labels check_ids in
851863
if keep && labels' = [] then (false, labels) else

jscomp/test/DisambiguateOptionalFields.js

Lines changed: 24 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
type t1 = {x:int, y:int}
2+
type t2 = {x:int, y:int, z?:int}
3+
4+
let f1 = (v:t1) => v.x
5+
let f2 = (v:t2) => v.x
6+
7+
let v = {x:3, y:4}
8+
let res = f2(v) // Check that t2 shadows t1

jscomp/test/build.ninja

Lines changed: 2 additions & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)