Drop leftover conditions and empty branches from if statements

changes/drop-pure-empty-if.polish.md

@@ -0,0 +1 @@
+- Stop emitting bare conditions like `x === null;` and empty `if`/`else` branches.

changes/negated-float-comparison-nan.fix.md

@@ -0,0 +1 @@
+- Fix negated float comparisons with NaN: `!(x < 0.0)` compiled to `x >= 0.0`, which is `false` for NaN.

compiler/core/js_exp_make.ml

@@ -1555,8 +1555,15 @@ let bool_comp (cmp : Lambda.comparison) ?comment (e0 : t) (e1 : t) =
       bin ?comment (Lam_compile_util.jsop_of_comp cmp) e0 e1)
   | _, _ -> bin ?comment (Lam_compile_util.jsop_of_comp cmp) e0 e1
 
+let float_comp (cmp : Lambda.comparison) ?comment (e0 : t) (e1 : t) =
+  match cmp with
+  | Ceq | Cneq -> int_comp cmp ?comment e0 e1
+  | Clt | Cgt | Cle | Cge ->
+    bin ?comment (Lam_compile_util.jsop_of_float_comp cmp) e0 e1
+
+(* The operands may be of any type, so they may be NaN *)
 let js_comp cmp ?comment e0 e1 =
-  bin ?comment (Lam_compile_util.jsop_of_comp cmp) e0 e1
+  bin ?comment (Lam_compile_util.jsop_of_float_comp cmp) e0 e1
 
 let rec int32_lsr ?comment (e1 : J.expression) (e2 : J.expression) :
     J.expression =

compiler/core/js_exp_make.mli

@@ -244,6 +244,8 @@ val bigint_div : checked:bool -> ?comment:string -> t -> t -> t
 
 val bigint_mod : checked:bool -> ?comment:string -> t -> t -> t
 
+val float_comp : Lambda.comparison -> ?comment:string -> t -> t -> t
+
 val js_comp : Lambda.comparison -> ?comment:string -> t -> t -> t
 
 val not : t -> t

compiler/core/js_op.ml

@@ -38,6 +38,13 @@ type binop =
   | Le
   | Gt
   | Ge
+  | Float_lt
+  | Float_le
+  | Float_gt
+  | Float_ge
+    (* Printed as [<], [<=], [>], [>=], for operands that may be NaN, so
+       that negating them can't swap the operator: [!(a < b)] is not
+       [a >= b] when [a] or [b] is NaN *)
   | Bor
   | Bxor
   | Band

compiler/core/js_op_util.ml

@@ -34,7 +34,9 @@ let op_prec (op : Js_op.binop) =
   | Or -> (3, 3, 3)
   | And -> (4, 4, 4)
   | EqEqEq | NotEqEq -> (8, 8, 9)
-  | Gt | Ge | Lt | Le | InstanceOf -> (9, 9, 10)
+  | Gt | Ge | Lt | Le | Float_lt | Float_le | Float_gt | Float_ge | InstanceOf
+    ->
+    (9, 9, 10)
   | Bor -> (5, 5, 5)
   | Bxor -> (6, 6, 6)
   | Band -> (7, 7, 7)
@@ -62,10 +64,10 @@ let op_str (op : Js_op.binop) =
   | And -> "&&"
   | EqEqEq -> "==="
   | NotEqEq -> "!=="
-  | Lt -> "<"
-  | Le -> "<="
-  | Gt -> ">"
-  | Ge -> ">="
+  | Lt | Float_lt -> "<"
+  | Le | Float_le -> "<="
+  | Gt | Float_gt -> ">"
+  | Ge | Float_ge -> ">="
   | InstanceOf -> "instanceof"
 
 let update_used_stats (ident_info : J.ident_info) used_stats =

compiler/core/js_pass_flatten.ml

@@ -49,15 +49,20 @@ let flatten_map =
           S.block
             (Ext_list.map args (fun arg -> self.statement self (S.exp arg)))
         | Exp {expression_desc = Cond (a, b, c); comment; source_loc} ->
-          {
-            statement_desc =
-              If
-                ( a,
-                  [self.statement self (S.exp b)],
-                  [self.statement self (S.exp c)] );
-            comment;
-            source_loc;
-          }
+          (* A branch whose value is pure leaves nothing to run *)
+          let branch e =
+            if Js_analyzer.no_side_effect_expression e then []
+            else [self.statement self (S.exp e)]
+          in
+          let statement_desc : J.statement_desc =
+            match (branch b, branch c) with
+            | [], [] ->
+              if Js_analyzer.no_side_effect_expression a then Block []
+              else Exp a
+            | [], ifnot -> If (E.not a, ifnot, [])
+            | ifso, ifnot -> If (a, ifso, ifnot)
+          in
+          {statement_desc; comment; source_loc}
         | Exp
             {
               expression_desc = Bin (Eq, a, ({expression_desc = Seq _; _} as v));

compiler/core/js_stmt_make.ml

@@ -246,12 +246,24 @@ let if_ ?comment ?declaration ?else_ (e : J.expression) (then_ : J.block) : t =
   let declared = ref false in
   let rec aux ?comment (e : J.expression) (ifso : J.block) (ifnot : J.block) : t
       =
+    (* An empty block, such as a dropped condition of a nested [if], leaves
+       its branch empty *)
+    let is_not_empty (s : t) =
+      match s.statement_desc with
+      | Block [] -> false
+      | _ -> true
+    in
+    let ifso = List.filter is_not_empty ifso in
+    let ifnot = List.filter is_not_empty ifnot in
     match (e.expression_desc, ifnot) with
     | Bool boolean, _ -> block (if boolean then ifso else ifnot)
     | Js_not pred_not, _ :: _ -> aux ?comment pred_not ifnot ifso
     | _ -> (
       match (ifso, ifnot) with
-      | [], [] -> exp e
+      | [], [] ->
+        (* Both branches are empty: only a condition with side effects
+           needs to be kept *)
+        if Js_analyzer.no_side_effect_expression e then empty_stmt else exp e
       | [], _ -> aux ?comment (E.not e) ifnot [] (*Make sure no infinite loop*)
       | ( [{statement_desc = Return ret_ifso; _}],
           [{statement_desc = Return ret_ifnot; _}] ) ->

compiler/core/lam_compile.ml

@@ -1512,7 +1512,7 @@ let compile output_prefix =
             ( Js_exp_make.remove_pure_sub_exp out1,
               Js_exp_make.remove_pure_sub_exp out2 )
           with
-          | None, None -> Js_output.make (Ext_list.append_one b (S.exp e))
+          | None, None -> Js_output.make (Ext_list.append_one b (S.if_ e []))
           (* FIX #1762 *)
           | Some out1, Some out2 ->
             Js_output.make b ~value:(E.econd e out1 out2)

compiler/core/lam_compile_primitive.ml

@@ -337,13 +337,17 @@ let translate output_prefix loc (cxt : Lam_compile_context.t)
     match args with
     | [e1; e2] -> E.bool_comp cmp e1 e2
     | _ -> assert false)
-  | Pfloatcomp cmp | Pintcomp cmp -> (
+  | Pintcomp cmp -> (
     (* Global Builtin Exception is an int, like
        [Not_found] or [Invalid_argument] ?
     *)
     match args with
     | [e1; e2] -> E.int_comp cmp e1 e2
     | _ -> assert false)
+  | Pfloatcomp cmp -> (
+    match args with
+    | [e1; e2] -> E.float_comp cmp e1 e2
+    | _ -> assert false)
   | Pbigintcomp cmp -> (
     match args with
     | [e1; e2] -> E.bigint_comp cmp e1 e2

compiler/core/lam_compile_util.ml

@@ -31,6 +31,16 @@ let jsop_of_comp (cmp : Lambda.comparison) : Js_op.binop =
   | Cle -> Le
   | Cge -> Ge
 
+(* For operands that may be NaN, see [Js_op.Float_lt] *)
+let jsop_of_float_comp (cmp : Lambda.comparison) : Js_op.binop =
+  match cmp with
+  | Ceq -> EqEqEq
+  | Cneq -> NotEqEq
+  | Clt -> Float_lt
+  | Cgt -> Float_gt
+  | Cle -> Float_le
+  | Cge -> Float_ge
+
 let runtime_of_comp (cmp : Lambda.comparison) : string =
   match cmp with
   | Ceq -> "equal"

compiler/core/lam_compile_util.mli

@@ -26,4 +26,6 @@
 
 val jsop_of_comp : Lambda.comparison -> Js_op.binop
 
+val jsop_of_float_comp : Lambda.comparison -> Js_op.binop
+
 val runtime_of_comp : Lambda.comparison -> string

tests/tests/src/caml_compare_bigint_test.mjs

@@ -28,9 +28,9 @@ Mocha.describe("Caml_compare_bigint_test", () => {
     Test_utils.ok("File \"caml_compare_bigint_test.res\", line 27, characters 7-14", BigInt("123") !== BigInt("555555"));
   });
   Mocha.test("compare: 98765 and 98765", () => {
-    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 31, characters 7-14", BigInt("98765") >= BigInt("98765"));
+    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 31, characters 7-14", !(BigInt("98765") < BigInt("98765")));
     Test_utils.ok("File \"caml_compare_bigint_test.res\", line 32, characters 7-14", BigInt("98765") <= BigInt("98765"));
-    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 33, characters 7-14", BigInt("98765") <= BigInt("98765"));
+    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 33, characters 7-14", !(BigInt("98765") > BigInt("98765")));
     Test_utils.ok("File \"caml_compare_bigint_test.res\", line 34, characters 7-14", BigInt("98765") >= BigInt("98765"));
     Test_utils.eq("File \"caml_compare_bigint_test.res\", line 35, characters 7-14", Primitive_float.min(BigInt("98765"), BigInt("98765")), BigInt("98765"));
     Test_utils.eq("File \"caml_compare_bigint_test.res\", line 36, characters 7-14", Primitive_float.max(BigInt("98765"), BigInt("98765")), BigInt("98765"));
@@ -46,9 +46,9 @@ Mocha.describe("Caml_compare_bigint_test", () => {
     Test_utils.ok("File \"caml_compare_bigint_test.res\", line 46, characters 7-14", BigInt("98765") === BigInt("98765"));
   });
   Mocha.test("same instance", () => {
-    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 50, characters 7-14", five >= five);
+    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 50, characters 7-14", !(five < five));
     Test_utils.ok("File \"caml_compare_bigint_test.res\", line 51, characters 7-14", five <= five);
-    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 52, characters 7-14", five <= five);
+    Test_utils.ok("File \"caml_compare_bigint_test.res\", line 52, characters 7-14", !(five > five));
     Test_utils.ok("File \"caml_compare_bigint_test.res\", line 53, characters 7-14", five >= five);
     Test_utils.eq("File \"caml_compare_bigint_test.res\", line 54, characters 7-14", Primitive_float.min(five, five), five);
     Test_utils.eq("File \"caml_compare_bigint_test.res\", line 55, characters 7-14", Primitive_float.max(five, five), five);

tests/tests/src/fun_pattern_match.mjs

@@ -13,13 +13,8 @@ function f2(param, param$1) {
 function f3(param, param$1) {
   let lhs = param.rank;
   let rhs = param$1.rank;
-  if (typeof lhs !== "object") {
-    lhs === "Uninitialized";
-  } else {
-    if (typeof rhs === "object") {
-      return Primitive_int.compare(lhs._0, rhs._0);
-    }
-    rhs === "Uninitialized";
+  if (typeof lhs === "object" && typeof rhs === "object") {
+    return Primitive_int.compare(lhs._0, rhs._0);
   }
   throw {
     RE_EXN_ID: "Assert_failure",
@@ -35,13 +30,8 @@ function f3(param, param$1) {
 function f4(param, param$1) {
   let lhs = param.rank;
   let rhs = param$1.rank;
-  if (typeof lhs !== "object") {
-    lhs === "Uninitialized";
-  } else {
-    if (typeof rhs === "object") {
-      return Primitive_int.compare(lhs._0, rhs._0);
-    }
-    rhs === "Uninitialized";
+  if (typeof lhs === "object" && typeof rhs === "object") {
+    return Primitive_int.compare(lhs._0, rhs._0);
   }
   throw {
     RE_EXN_ID: "Assert_failure",

tests/tests/src/gpr_4491_test.mjs

@@ -5,8 +5,6 @@ function f(xs) {
   if (xs !== undefined) {
     console.log("side effect");
 
-  } else {
-
   }
   console.log("nothing to see here", xs);
 }

tests/tests/src/guard_action_test.mjs

@@ -12,9 +12,7 @@ function guard() {
 
 function both_guards_run() {
   calls.contents = 0;
-  if (guard()) {
-
-  } else {
+  if (!guard()) {
     guard();
   }
   return calls.contents;

tests/tests/src/mario_game.mjs

@@ -1991,9 +1991,7 @@ function process_collision(dir, c1, c2, state) {
       switch (t2$1) {
         case "Mushroom" :
           dec_health(o2$2);
-          if (o1$2.health === 2) {
-
-          } else {
+          if (o1$2.health !== 2) {
             o1$2.health = o1$2.health + 1 | 0;
           }
           o1$2.vel.x = 0;

tests/tests/src/negated_float_comparison_test.mjs

@@ -0,0 +1,71 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+import * as Mocha from "mocha";
+import * as Test_utils from "./test_utils.mjs";
+
+function notLt(x, y) {
+  return !(x < y);
+}
+
+function notLe(x, y) {
+  return !(x <= y);
+}
+
+function notGt(x, y) {
+  return !(x > y);
+}
+
+function notGe(x, y) {
+  return !(x >= y);
+}
+
+let log = {
+  contents: /* [] */0
+};
+
+function ifNotLt(x) {
+  if (!(x < 0.0)) {
+    log.contents = {hd: "then", tl: log.contents};
+  }
+  return log.contents;
+}
+
+function condNotLt(x) {
+  if (x < 0.0) {
+    return "else";
+  } else {
+    return "then";
+  }
+}
+
+Mocha.describe("Negated_float_comparison_test", () => {
+  Mocha.test("negated comparisons with NaN", () => {
+    let nan = NaN;
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 25, characters 7-14", true, !(nan < 0.0));
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 26, characters 7-14", true, !(nan <= 0.0));
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 27, characters 7-14", true, !(nan > 0.0));
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 28, characters 7-14", true, !(nan >= 0.0));
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 29, characters 7-14", true, !(0.0 < nan));
+  });
+  Mocha.test("negated comparisons with numbers", () => {
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 32, characters 7-14", false, false);
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 33, characters 7-14", true, true);
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 34, characters 7-14", false, false);
+  });
+  Mocha.test("negated comparison as a condition", () => {
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 37, characters 7-14", {hd: "then", tl: /* [] */0}, ifNotLt(NaN));
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 38, characters 7-14", "then", condNotLt(NaN));
+    Test_utils.eq("File \"negated_float_comparison_test.res\", line 39, characters 7-14", "else", "else");
+  });
+});
+
+export {
+  notLt,
+  notLe,
+  notGt,
+  notGe,
+  log,
+  ifNotLt,
+  condNotLt,
+}
+/*  Not a pure module */

tests/tests/src/negated_float_comparison_test.res

@@ -0,0 +1,41 @@
+open Mocha
+open Test_utils
+
+// Negating a float comparison must not flip its operator: for NaN,
+// !(x < y) is true while x >= y is false.
+let notLt = (x: float, y: float) => !(x < y)
+let notLe = (x: float, y: float) => !(x <= y)
+let notGt = (x: float, y: float) => !(x > y)
+let notGe = (x: float, y: float) => !(x >= y)
+
+let log = ref(list{})
+
+let ifNotLt = (x: float) => {
+  if !(x < 0.0) {
+    log := list{"then", ...log.contents}
+  }
+  log.contents
+}
+
+let condNotLt = (x: float) => !(x < 0.0) ? "then" : "else"
+
+describe(__MODULE__, () => {
+  test("negated comparisons with NaN", () => {
+    let nan = Float.Constants.nan
+    eq(__LOC__, true, notLt(nan, 0.0))
+    eq(__LOC__, true, notLe(nan, 0.0))
+    eq(__LOC__, true, notGt(nan, 0.0))
+    eq(__LOC__, true, notGe(nan, 0.0))
+    eq(__LOC__, true, notLt(0.0, nan))
+  })
+  test("negated comparisons with numbers", () => {
+    eq(__LOC__, false, notLt(-1.0, 0.0))
+    eq(__LOC__, true, notLt(1.0, 0.0))
+    eq(__LOC__, false, notGe(1.0, 0.0))
+  })
+  test("negated comparison as a condition", () => {
+    eq(__LOC__, list{"then"}, ifNotLt(Float.Constants.nan))
+    eq(__LOC__, "then", condNotLt(Float.Constants.nan))
+    eq(__LOC__, "else", condNotLt(-1.0))
+  })
+})

tests/tests/src/option_optimisation.mjs

@@ -13,7 +13,6 @@ function $$null(val1, val2) {
   if (val2 !== null && val2 !== undefined) {
     return "a";
   }
-  val2 === null;
   if (val1 == null) {
     return "b";
   } else {