Skip to content

Commit d0f9714

Browse files
author
Cristiano Calcagno
committed
Fix polymorphic compare on nullables.
Polymorphic compare could crash when the second argument is null.
1 parent cb3798b commit d0f9714

File tree

4 files changed

+44
-3
lines changed

4 files changed

+44
-3
lines changed

jscomp/runtime/caml_obj.ml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ 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.empty) then -1 else
171+
if b == (Obj.repr Js_null.empty) then 1 else
170172
let a_type = Js.typeof a in
171173
let b_type = Js.typeof b in
172174
if a_type = "string" then
@@ -182,7 +184,6 @@ let rec caml_compare (a : Obj.t) (b : Obj.t) : int =
182184
| false, false ->
183185
if a_type = "boolean"
184186
|| a_type = "undefined"
185-
|| a == (Obj.repr Js_null.empty)
186187
then (* TODO: refine semantics when comparing with [null] *)
187188
unsafe_js_compare a b
188189
else if a_type = "function" || b_type = "function"

jscomp/test/caml_compare_test.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,35 @@ 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+
/* [] */0
913+
]
914+
]
887915
]
888916
]
889917
]

jscomp/test/caml_compare_test.ml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ 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+
8088
]
8189
;;
8290

lib/js/caml_obj.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ 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;
6973
} else {
7074
var a_type = typeof a;
7175
var b_type = typeof b;
@@ -82,7 +86,7 @@ function caml_compare(_a, _b) {
8286
}
8387
} else if (is_b_number) {
8488
return 1;
85-
} else if (a_type === "boolean" || a_type === "undefined" || a === null) {
89+
} else if (a_type === "boolean" || a_type === "undefined") {
8690
var x = a;
8791
var y = b;
8892
if (x === y) {

0 commit comments

Comments
 (0)