Skip to content

Commit 919b378

Browse files
bobzhangbobzhang
authored
Merge pull request rescript-lang#2786 from cristianoc/fix_caml_compare
Fix polymorphic compare on nullables.
2 parents 3404ded + 13e8fa5 commit 919b378

File tree

4 files changed

+105
-4
lines changed

4 files changed

+105
-4
lines changed

‎jscomp/runtime/caml_obj.ml

+4-2
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ let unsafe_js_compare x y =
167167
let rec caml_compare (a : Obj.t) (b : Obj.t) : int =
168168
if a == b then 0 else
169169
(*front and formoest, we do not compare function values*)
170+
if a == (Obj.repr Js.null) then -1 else
171+
if b == (Obj.repr Js.null) then 1 else
172+
if a == (Obj.repr Js.undefined) then -1 else
173+
if b == (Obj.repr Js.undefined) then 1 else
170174
let a_type = Js.typeof a in
171175
let b_type = Js.typeof b in
172176
if a_type = "string" then
@@ -181,8 +185,6 @@ let rec caml_compare (a : Obj.t) (b : Obj.t) : int =
181185
| false, true -> 1
182186
| false, false ->
183187
if a_type = "boolean"
184-
|| a_type = "undefined"
185-
|| a == (Obj.repr Js_null.empty)
186188
then (* TODO: refine semantics when comparing with [null] *)
187189
unsafe_js_compare a b
188190
else if a_type = "function" || b_type = "function"

‎jscomp/test/caml_compare_test.js

+73-1
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,79 @@ var suites_001 = /* :: */[
883883
]);
884884
})
885885
],
886-
/* [] */0
886+
/* :: */[
887+
/* tuple */[
888+
"File \"caml_compare_test.ml\", line 81, characters 4-11",
889+
(function () {
890+
return /* Eq */Block.__(0, [
891+
Caml_obj.caml_compare(null, /* :: */[
892+
3,
893+
/* [] */0
894+
]),
895+
-1
896+
]);
897+
})
898+
],
899+
/* :: */[
900+
/* tuple */[
901+
"File \"caml_compare_test.ml\", line 84, characters 4-11",
902+
(function () {
903+
return /* Eq */Block.__(0, [
904+
Caml_obj.caml_compare(/* :: */[
905+
3,
906+
/* [] */0
907+
], null),
908+
1
909+
]);
910+
})
911+
],
912+
/* :: */[
913+
/* tuple */[
914+
"File \"caml_compare_test.ml\", line 87, characters 4-11",
915+
(function () {
916+
return /* Eq */Block.__(0, [
917+
Caml_obj.caml_compare(null, 0),
918+
-1
919+
]);
920+
})
921+
],
922+
/* :: */[
923+
/* tuple */[
924+
"File \"caml_compare_test.ml\", line 90, characters 4-11",
925+
(function () {
926+
return /* Eq */Block.__(0, [
927+
Caml_obj.caml_compare(0, null),
928+
1
929+
]);
930+
})
931+
],
932+
/* :: */[
933+
/* tuple */[
934+
"File \"caml_compare_test.ml\", line 93, characters 4-11",
935+
(function () {
936+
return /* Eq */Block.__(0, [
937+
Caml_obj.caml_compare(undefined, 0),
938+
-1
939+
]);
940+
})
941+
],
942+
/* :: */[
943+
/* tuple */[
944+
"File \"caml_compare_test.ml\", line 96, characters 4-11",
945+
(function () {
946+
return /* Eq */Block.__(0, [
947+
Caml_obj.caml_compare(0, undefined),
948+
1
949+
]);
950+
})
951+
],
952+
/* [] */0
953+
]
954+
]
955+
]
956+
]
957+
]
958+
]
887959
]
888960
]
889961
]

‎jscomp/test/caml_compare_test.ml

+19
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,25 @@ let suites = Mt.[
7777
"eq_in_list2", (fun _ -> Eq ([[%bs.obj {x=2}]] = [[%bs.obj {x=2}]], true));
7878
"eq_with_list", (fun _ -> Eq ([%bs.obj {x=[0]}] = [%bs.obj {x=[0]}], true));
7979
"eq_with_list2", (fun _ -> Eq ([%bs.obj {x=[0]}] = [%bs.obj {x=[1]}], false));
80+
81+
__LOC__ , begin fun _ ->
82+
Eq(compare Js.null (Js.Null.return [3]), -1)
83+
end;
84+
__LOC__ , begin fun _ ->
85+
Eq(compare (Js.Null.return [3]) Js.null, 1)
86+
end;
87+
__LOC__ , begin fun _ ->
88+
Eq(compare Js.null (Js.Null.return 0), -1)
89+
end;
90+
__LOC__ , begin fun _ ->
91+
Eq(compare (Js.Null.return 0) Js.null, 1)
92+
end;
93+
__LOC__ , begin fun _ ->
94+
Eq(compare Js.Nullable.undefined (Js.Nullable.return 0), -1)
95+
end;
96+
__LOC__ , begin fun _ ->
97+
Eq(compare (Js.Nullable.return 0) Js.Nullable.undefined, 1)
98+
end;
8099
]
81100
;;
82101

‎lib/js/caml_obj.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ function caml_compare(_a, _b) {
6666
var a = _a;
6767
if (a === b) {
6868
return 0;
69+
} else if (a === null) {
70+
return -1;
71+
} else if (b === null) {
72+
return 1;
73+
} else if (a === undefined) {
74+
return -1;
75+
} else if (b === undefined) {
76+
return 1;
6977
} else {
7078
var a_type = typeof a;
7179
var b_type = typeof b;
@@ -82,7 +90,7 @@ function caml_compare(_a, _b) {
8290
}
8391
} else if (is_b_number) {
8492
return 1;
85-
} else if (a_type === "boolean" || a_type === "undefined" || a === null) {
93+
} else if (a_type === "boolean") {
8694
var x = a;
8795
var y = b;
8896
if (x === y) {

0 commit comments

Comments
 (0)