Skip to content

Fix polymorphic compare on nullables. #2786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 7, 2018
+105 −4
Merged

Fix polymorphic compare on nullables. #2786

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0f9714
Fix polymorphic compare on nullables.
May 3, 2018
e18fd34
Add comparison for undefined, and tests.
May 7, 2018
13e8fa5
Remove check for type undefined, which is now redundant.
May 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions jscomp/runtime/caml_obj.ml
View file
Original file line number Diff line number Diff line change
@@ -167,6 +167,10 @@ let unsafe_js_compare x y =
let rec caml_compare (a : Obj.t) (b : Obj.t) : int =
if a == b then 0 else
(*front and formoest, we do not compare function values*)
if a == (Obj.repr Js.null) then -1 else
if b == (Obj.repr Js.null) then 1 else
if a == (Obj.repr Js.undefined) then -1 else
if b == (Obj.repr Js.undefined) then 1 else
let a_type = Js.typeof a in
let b_type = Js.typeof b in
if a_type = "string" then
@@ -181,8 +185,6 @@ let rec caml_compare (a : Obj.t) (b : Obj.t) : int =
| false, true -> 1
| false, false ->
if a_type = "boolean"
|| a_type = "undefined"
|| a == (Obj.repr Js_null.empty)
then (* TODO: refine semantics when comparing with [null] *)
unsafe_js_compare a b
else if a_type = "function" || b_type = "function"
74 changes: 73 additions & 1 deletion jscomp/test/caml_compare_test.js
View file
Original file line number Diff line number Diff line change
@@ -883,7 +883,79 @@ var suites_001 = /* :: */[
]);
})
],
/* [] */0
/* :: */[
/* tuple */[
"File \"caml_compare_test.ml\", line 81, characters 4-11",
(function () {
return /* Eq */Block.__(0, [
Caml_obj.caml_compare(null, /* :: */[
3,
/* [] */0
]),
-1
]);
})
],
/* :: */[
/* tuple */[
"File \"caml_compare_test.ml\", line 84, characters 4-11",
(function () {
return /* Eq */Block.__(0, [
Caml_obj.caml_compare(/* :: */[
3,
/* [] */0
], null),
1
]);
})
],
/* :: */[
/* tuple */[
"File \"caml_compare_test.ml\", line 87, characters 4-11",
(function () {
return /* Eq */Block.__(0, [
Caml_obj.caml_compare(null, 0),
-1
]);
})
],
/* :: */[
/* tuple */[
"File \"caml_compare_test.ml\", line 90, characters 4-11",
(function () {
return /* Eq */Block.__(0, [
Caml_obj.caml_compare(0, null),
1
]);
})
],
/* :: */[
/* tuple */[
"File \"caml_compare_test.ml\", line 93, characters 4-11",
(function () {
return /* Eq */Block.__(0, [
Caml_obj.caml_compare(undefined, 0),
-1
]);
})
],
/* :: */[
/* tuple */[
"File \"caml_compare_test.ml\", line 96, characters 4-11",
(function () {
return /* Eq */Block.__(0, [
Caml_obj.caml_compare(0, undefined),
1
]);
})
],
/* [] */0
]
]
]
]
]
]
]
]
]
19 changes: 19 additions & 0 deletions jscomp/test/caml_compare_test.ml
View file
Original file line number Diff line number Diff line change
@@ -77,6 +77,25 @@ let suites = Mt.[
"eq_in_list2", (fun _ -> Eq ([[%bs.obj {x=2}]] = [[%bs.obj {x=2}]], true));
"eq_with_list", (fun _ -> Eq ([%bs.obj {x=[0]}] = [%bs.obj {x=[0]}], true));
"eq_with_list2", (fun _ -> Eq ([%bs.obj {x=[0]}] = [%bs.obj {x=[1]}], false));

__LOC__ , begin fun _ ->
Eq(compare Js.null (Js.Null.return [3]), -1)
end;
__LOC__ , begin fun _ ->
Eq(compare (Js.Null.return [3]) Js.null, 1)
end;
__LOC__ , begin fun _ ->
Eq(compare Js.null (Js.Null.return 0), -1)
end;
__LOC__ , begin fun _ ->
Eq(compare (Js.Null.return 0) Js.null, 1)
end;
__LOC__ , begin fun _ ->
Eq(compare Js.Nullable.undefined (Js.Nullable.return 0), -1)
end;
__LOC__ , begin fun _ ->
Eq(compare (Js.Nullable.return 0) Js.Nullable.undefined, 1)
end;
]
;;

10 changes: 9 additions & 1 deletion lib/js/caml_obj.js
View file
Original file line number Diff line number Diff line change
@@ -66,6 +66,14 @@ function caml_compare(_a, _b) {
var a = _a;
if (a === b) {
return 0;
} else if (a === null) {
return -1;
} else if (b === null) {
return 1;
} else if (a === undefined) {
return -1;
} else if (b === undefined) {
return 1;
} else {
var a_type = typeof a;
var b_type = typeof b;
@@ -82,7 +90,7 @@ function caml_compare(_a, _b) {
}
} else if (is_b_number) {
return 1;
} else if (a_type === "boolean" || a_type === "undefined" || a === null) {
} else if (a_type === "boolean") {
var x = a;
var y = b;
if (x === y) {