From 518b26ee8cd2384ad10e5df1856c8b8572e4f778 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 28 Oct 2026 15:31:37 +0000 Subject: [PATCH] Compare literals by JavaScript value when simplifying x === c1 && x === c2 Why: The simplifier read "not eq_expression" as "different constants", but eq_expression never equates float literals, nor an int with a float. So `x === 2.5 && x === 2.5` became `false`, and an untagged case whose payload literal equals a literal tag (`@as(1) L1 | N(float)`, `N(1.0)`) went to the wrong arm. Js_analyzer.eq_literal now decides literal equality as `===` does, and the rules apply only when it knows. JS modified (and-simplification of literal comparisons): Example: let a = (x: float) => x == 2.5 && x == 2.5 let b = (x: float) => x != 2.5 && x == 2.5 @unboxed type t = @as(1) L1 | N(float) | S(string) let f = (v: t) => switch v { | N(1.0) => 4 | _ => 6 } Before (js): function a(x) { return false; } function b(x) { return x === 2.5; } function f(v) { if (v !== 1.0) { return 6; } else { return 4; } } After (js): function a(x) { return x === 2.5; } function b(x) { return false; } function f(v) { return 6; } Fixes: #9099 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel --- .../float-literal-equality-simplify.fix.md | 1 + compiler/core/js_analyzer.ml | 29 +++++++ compiler/core/js_analyzer.mli | 4 + compiler/core/js_exp_make.ml | 43 +++++++---- .../src/number_literal_simplify_test.mjs | 77 +++++++++++++++++++ .../src/number_literal_simplify_test.res | 55 +++++++++++++ 6 files changed, 192 insertions(+), 17 deletions(-) create mode 100644 changes/float-literal-equality-simplify.fix.md create mode 100644 tests/tests/src/number_literal_simplify_test.mjs create mode 100644 tests/tests/src/number_literal_simplify_test.res diff --git a/changes/float-literal-equality-simplify.fix.md b/changes/float-literal-equality-simplify.fix.md new file mode 100644 index 00000000000..fd30b8297f6 --- /dev/null +++ b/changes/float-literal-equality-simplify.fix.md @@ -0,0 +1 @@ +- Fix the JavaScript simplifier treating equal float literals, or an int and a float literal of the same value, as different constants, which turned `x == 2.5 && x == 2.5` into `false`. diff --git a/compiler/core/js_analyzer.ml b/compiler/core/js_analyzer.ml index da4cd276d14..85f46a68a8a 100644 --- a/compiler/core/js_analyzer.ml +++ b/compiler/core/js_analyzer.ml @@ -184,6 +184,35 @@ let no_side_effect_statement st = true with _ -> false +(* The value of a number literal as JavaScript sees it, so that [1] and + [1.0] are the same number. [None] for a bigint, which is never [===] to + a number. *) +let number_value (n : J.number) : float option = + match n with + | Int {i} -> Some (Int32.to_float i) + | Float {f} -> float_of_string_opt f + | BigInt _ -> None + +let eq_literal (x : J.expression_desc) (y : J.expression_desc) : bool option = + match (x, y) with + | ( Number (BigInt {positive = p0; value = v0}), + Number (BigInt {positive = p1; value = v1}) ) -> + Some (p0 = p1 && v0 = v1) + | Number (BigInt _), Number _ | Number _, Number (BigInt _) -> Some false + | Number n0, Number n1 -> ( + match (number_value n0, number_value n1) with + (* Float [=] is IEEE equality like [===]: NaN differs from itself, and + [0.] equals [-0.] *) + | Some f0, Some f1 -> Some (f0 = f1) + | _ -> None) + | Bool b0, Bool b1 -> Some (b0 = b1) + | Str s0, Str s1 -> Some (s0 = s1) + | Null, Null | Undefined _, Undefined _ -> Some true + | ( (Bool _ | Number _ | Str _ | Null | Undefined _), + (Bool _ | Number _ | Str _ | Null | Undefined _) ) -> + Some false + | _ -> None + (* TODO: generate [fold2] This make sense, for example: {[ diff --git a/compiler/core/js_analyzer.mli b/compiler/core/js_analyzer.mli index 80f8c216d10..281ec6c4127 100644 --- a/compiler/core/js_analyzer.mli +++ b/compiler/core/js_analyzer.mli @@ -57,6 +57,10 @@ val no_side_effect_statement : J.statement -> bool val eq_expression : J.expression -> J.expression -> bool +val eq_literal : J.expression_desc -> J.expression_desc -> bool option +(** Whether two literals are the same value under JavaScript [===], for + example [1] and [1.0]; [None] when either is not a literal. *) + val eq_block : J.block -> J.block -> bool val rev_flatten_seq : J.expression -> J.block diff --git a/compiler/core/js_exp_make.ml b/compiler/core/js_exp_make.ml index 076cbb4bff1..64baaf1eb09 100644 --- a/compiler/core/js_exp_make.ml +++ b/compiler/core/js_exp_make.ml @@ -1161,32 +1161,41 @@ let rec simplify_and_ ~n (e1 : t) (e2 : t) : t option = | ( Bin ( EqEqEq, {expression_desc = Var ia}, - {expression_desc = Bool _ | Null | Undefined _ | Number _ | Str _} - ), + { + expression_desc = + (Bool _ | Null | Undefined _ | Number _ | Str _) as v1; + } ), Bin ( EqEqEq, {expression_desc = Var ib}, - {expression_desc = Bool _ | Null | Undefined _ | Number _ | Str _} - ) ) - when Js_op_util.same_vident ia ib -> - (* Note: case x = y is handled above *) - Some false_ + { + expression_desc = + (Bool _ | Null | Undefined _ | Number _ | Str _) as v2; + } ) ) + when Js_op_util.same_vident ia ib -> ( + match Js_analyzer.eq_literal v1 v2 with + | Some true -> Some e1 + | Some false -> Some false_ + | None -> None) | ( Bin ( ((EqEqEq | NotEqEq) as op1), {expression_desc = Var ia}, - ({ - expression_desc = Bool _ | Null | Undefined _ | Number _ | Str _; - } as v1) ), + { + expression_desc = + (Bool _ | Null | Undefined _ | Number _ | Str _) as v1; + } ), Bin ( ((EqEqEq | NotEqEq) as op2), {expression_desc = Var ib}, - ({ - expression_desc = Bool _ | Null | Undefined _ | Number _ | Str _; - } as v2) ) ) - when Js_op_util.same_vident ia ib && op1 != op2 -> - if Js_analyzer.eq_expression v1 v2 then Some false_ - else if op1 = EqEqEq then Some e1 - else Some e2 + { + expression_desc = + (Bool _ | Null | Undefined _ | Number _ | Str _) as v2; + } ) ) + when Js_op_util.same_vident ia ib && op1 != op2 -> ( + match Js_analyzer.eq_literal v1 v2 with + | Some true -> Some false_ + | Some false -> if op1 = EqEqEq then Some e1 else Some e2 + | None -> None) | _ -> None in res diff --git a/tests/tests/src/number_literal_simplify_test.mjs b/tests/tests/src/number_literal_simplify_test.mjs new file mode 100644 index 00000000000..1bc7b465ecf --- /dev/null +++ b/tests/tests/src/number_literal_simplify_test.mjs @@ -0,0 +1,77 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Test_utils from "./test_utils.mjs"; + +function float_eq_eq(x) { + return x === 2.5; +} + +function float_neq_eq(x) { + return false; +} + +function float_eq_neq(x) { + return false; +} + +function float_eq_other(x) { + return false; +} + +function float_neq_other(x) { + return x === 3.5; +} + +function float_or(x) { + if (x !== 2.5) { + return true; + } else { + return x !== 2.5; + } +} + +function int_tag_shadowed(v) { + return 6; +} + +function float_tag_shadowed(v) { + return 6; +} + +let opaque = (x => x); + +Mocha.describe("Number_literal_simplify_test", () => { + Mocha.test("equal float literals", () => { + Test_utils.eq("File \"number_literal_simplify_test.res\", line 37, characters 7-14", true, float_eq_eq(opaque(2.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 38, characters 7-14", false, float_eq_eq(opaque(3.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 39, characters 7-14", false, float_neq_eq(opaque(2.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 40, characters 7-14", false, float_eq_neq(opaque(2.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 41, characters 7-14", true, float_or(opaque(3.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 42, characters 7-14", false, float_or(opaque(2.5))); + }); + Mocha.test("different float literals", () => { + Test_utils.eq("File \"number_literal_simplify_test.res\", line 45, characters 7-14", false, float_eq_other(opaque(2.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 46, characters 7-14", true, float_neq_other(opaque(3.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 47, characters 7-14", false, float_neq_other(opaque(2.5))); + }); + Mocha.test("literal tag equal to a payload literal", () => { + Test_utils.eq("File \"number_literal_simplify_test.res\", line 50, characters 7-14", 6, int_tag_shadowed(opaque(1))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 51, characters 7-14", 6, int_tag_shadowed(opaque(2.0))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 52, characters 7-14", 6, float_tag_shadowed(opaque(2.5))); + Test_utils.eq("File \"number_literal_simplify_test.res\", line 53, characters 7-14", 6, float_tag_shadowed(opaque(1.0))); + }); +}); + +export { + float_eq_eq, + float_neq_eq, + float_eq_neq, + float_eq_other, + float_neq_other, + float_or, + int_tag_shadowed, + float_tag_shadowed, + opaque, +} +/* Not a pure module */ diff --git a/tests/tests/src/number_literal_simplify_test.res b/tests/tests/src/number_literal_simplify_test.res new file mode 100644 index 00000000000..940b0aa6eb7 --- /dev/null +++ b/tests/tests/src/number_literal_simplify_test.res @@ -0,0 +1,55 @@ +// `x === c1 && x === c2` and `x !== c1 && x === c2` are simplified by +// comparing c1 and c2 as JavaScript values: equal float literals are equal, +// and so are 1 and 1.0. + +let float_eq_eq = (x: float) => x == 2.5 && x == 2.5 +let float_neq_eq = (x: float) => x != 2.5 && x == 2.5 +let float_eq_neq = (x: float) => x == 2.5 && x != 2.5 +let float_eq_other = (x: float) => x == 2.5 && x == 3.5 +let float_neq_other = (x: float) => x != 2.5 && x == 3.5 +let float_or = (x: float) => x != 2.5 || x != 2.5 + +@unboxed type int_tag = | @as(1) L1 | N(float) | S(string) +// The payload case is unused (warning 11): 1 always matches as L1. +@warning("-11") +let int_tag_shadowed = (v: int_tag) => + switch v { + | N(1.0) => 4 + | _ => 6 + } + +@unboxed type float_tag = | @as(2.5) L | M(float) | P(string) +@warning("-11") +let float_tag_shadowed = (v: float_tag) => + switch v { + | M(2.5) => 4 + | _ => 6 + } + +// Keeps the arguments unknown, so the calls are not folded to constants. +let opaque: 'a => 'a = %raw(`x => x`) + +open Mocha +open Test_utils + +describe(__MODULE__, () => { + test("equal float literals", () => { + eq(__LOC__, true, float_eq_eq(opaque(2.5))) + eq(__LOC__, false, float_eq_eq(opaque(3.5))) + eq(__LOC__, false, float_neq_eq(opaque(2.5))) + eq(__LOC__, false, float_eq_neq(opaque(2.5))) + eq(__LOC__, true, float_or(opaque(3.5))) + eq(__LOC__, false, float_or(opaque(2.5))) + }) + test("different float literals", () => { + eq(__LOC__, false, float_eq_other(opaque(2.5))) + eq(__LOC__, true, float_neq_other(opaque(3.5))) + eq(__LOC__, false, float_neq_other(opaque(2.5))) + }) + test("literal tag equal to a payload literal", () => { + eq(__LOC__, 6, int_tag_shadowed(opaque(L1))) + eq(__LOC__, 6, int_tag_shadowed(opaque(N(2.0)))) + eq(__LOC__, 6, float_tag_shadowed(opaque(L))) + eq(__LOC__, 6, float_tag_shadowed(opaque(M(1.0)))) + }) +})