Simplify conditionals codegen + add missing side effects checks.

CHANGELOG.md

@@ -16,6 +16,20 @@
 
 - Introduce "Unified operators" for arithmetic operators (`+`, `-`, `*`, `/`, `mod`). See https://github.com/rescript-lang/rescript-compiler/pull/7057
 
+
+#### :bug: Bug fix
+- Fix and clean up boolean and/or optimizations. https://github.com/rescript-lang/rescript-compiler/pull/7134 https://github.com/rescript-lang/rescript-compiler/pull/7151
+
+#### :nail_care: Polish
+- Improve code generation for pattern matching of untagged variants. https://github.com/rescript-lang/rescript-compiler/pull/7128
+- Improve negation handling in combination with and/or to simplify generated code (especially coming out of pattern matching). https://github.com/rescript-lang/rescript-compiler/pull/7138
+- optimize JavaScript code generation by using x == null checks and improving type-based optimizations for string/number literals. https://github.com/rescript-lang/rescript-compiler/pull/7141
+- Improve pattern matching on optional fields. https://github.com/rescript-lang/rescript-compiler/pull/7143 https://github.com/rescript-lang/rescript-compiler/pull/7144
+- Optimize compilation of switch statements for untagged variants when there are no literal cases. https://github.com/rescript-lang/rescript-compiler/pull/7135
+- Further improve boolean optimizations. https://github.com/rescript-lang/rescript-compiler/pull/7149
+- Simplify code generated for conditionals. https://github.com/rescript-lang/rescript-compiler/pull/7151
+
+
 # 12.0.0-alpha.4
 
 #### :boom: Breaking Change
@@ -44,10 +58,8 @@
 - Fix variant cast to int. https://github.com/rescript-lang/rescript-compiler/pull/7058
 - Fix comments formatted away in function without arguments. https://github.com/rescript-lang/rescript-compiler/pull/7095
 - Fix genType JSX component compilation. https://github.com/rescript-lang/rescript-compiler/pull/7107
-- Fix and clean up boolean and/or optimizations. https://github.com/rescript-lang/rescript-compiler/pull/7134
 
 #### :nail_care: Polish
-
 - Add some context to error message for unused variables. https://github.com/rescript-lang/rescript-compiler/pull/7050
 - Improve error message when passing `children` prop to a component that doesn't accept it. https://github.com/rescript-lang/rescript-compiler/pull/7044
 - Improve error messages for pattern matching on option vs non-option, and vice versa. https://github.com/rescript-lang/rescript-compiler/pull/7035
@@ -57,12 +69,6 @@
 - Improve output of record copying. https://github.com/rescript-lang/rescript-compiler/pull/7043
 - Provide additional context in error message when `unit` is expected. https://github.com/rescript-lang/rescript-compiler/pull/7045
 - Improve error message when passing an object where a record is expected. https://github.com/rescript-lang/rescript-compiler/pull/7101
-- Improve code generation or pattern matching of untagged variants. https://github.com/rescript-lang/rescript-compiler/pull/7128
-- Improve negation handling in combination with and/or to simplify generated code (especially coming out of pattern matching). https://github.com/rescript-lang/rescript-compiler/pull/7138
-- optimize JavaScript code generation by using x == null checks and improving type-based optimizations for string/number literals. https://github.com/rescript-lang/rescript-compiler/pull/7141
-- Improve pattern matching on optional fields. https://github.com/rescript-lang/rescript-compiler/pull/7143 https://github.com/rescript-lang/rescript-compiler/pull/7144
-- Optimize compilation of switch statements for untagged variants when there are no literal cases. https://github.com/rescript-lang/rescript-compiler/pull/7135
-- Further improve boolean optimizations. https://github.com/rescript-lang/rescript-compiler/pull/7149
 
 
 #### :house: Internal

compiler/core/js_analyzer.ml

@@ -109,9 +109,11 @@ let rec no_side_effect_expression_desc (x : J.expression_desc) =
     no_side_effect call_expr
     && Ext_list.for_all strings no_side_effect
     && Ext_list.for_all values no_side_effect
-  | Js_not _ | Cond _ | FlatCall _ | Call _ | New _ | Raw_js_code _
-  (* actually true? *) ->
-    false
+  | Js_not e -> no_side_effect e
+  | Cond (a, b, c) -> no_side_effect a && no_side_effect b && no_side_effect c
+  | Call ({expression_desc = Str {txt = "Array.isArray"}}, [e], _) ->
+    no_side_effect e
+  | FlatCall _ | Call _ | New _ | Raw_js_code _ (* actually true? *) -> false
   | Await _ -> false
   | Spread _ -> false
 

compiler/core/js_exp_make.ml

@@ -556,12 +556,6 @@ let string_length ?comment (e : t) : t =
   (* No optimization for {j||j}*)
   | _ -> {expression_desc = Length (e, String); comment}
 
-(* TODO: use [Buffer] instead? *)
-let bytes_length ?comment (e : t) : t =
-  match e.expression_desc with
-  | Array (l, _) -> int ?comment (Int32.of_int (List.length l))
-  | _ -> {expression_desc = Length (e, Bytes); comment}
-
 let function_length ?comment (e : t) : t =
   match e.expression_desc with
   | Fun {is_method; params} ->
@@ -624,7 +618,8 @@ let rec triple_equal ?comment (e0 : t) (e1 : t) : t =
   | Undefined _, Optional_block _
   | Optional_block _, Undefined _
   | Null, Undefined _
-  | Undefined _, Null ->
+  | Undefined _, Null
+    when no_side_effect e0 && no_side_effect e1 ->
     false_
   | Null, Null | Undefined _, Undefined _ -> true_
   | _ -> {expression_desc = Bin (EqEqEq, e0, e1); comment}
@@ -696,7 +691,7 @@ let rec push_negation (e : t) : t option =
   | _ -> None
 
 (**
-  [simplify_and e1 e2] attempts to simplify the boolean AND expression [e1 && e2].
+  [simplify_and_ e1 e2] attempts to simplify the boolean AND expression [e1 && e2].
   Returns [Some simplified] if simplification is possible, [None] otherwise.
 
   Basic simplification rules:
@@ -735,7 +730,7 @@ let rec push_negation (e : t) : t option =
   attempting to reduce it to a simpler form. If no simplification is possible,
   returns [None].
 *)
-let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
+let rec simplify_and_ ~n (e1 : t) (e2 : t) : t option =
   if debug then
     Printf.eprintf "%s simplify_and %s %s\n"
       (String.make (n * 2) ' ')
@@ -747,30 +742,30 @@ let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
     | Bool true, _ -> Some e2
     | _, Bool true -> Some e1
     | Bin (And, a, b), _ -> (
-      let ao = simplify_and ~n:(n + 1) a e2 in
-      let bo = simplify_and ~n:(n + 1) b e2 in
+      let ao = simplify_and_ ~n:(n + 1) a e2 in
+      let bo = simplify_and_ ~n:(n + 1) b e2 in
       match (ao, bo) with
       | None, None -> (
-        match simplify_and ~n:(n + 1) a b with
+        match simplify_and_ ~n:(n + 1) a b with
         | None -> None
         | Some e -> simplify_and_force ~n:(n + 1) e e2)
       | Some a_, None -> simplify_and_force ~n:(n + 1) a_ b
       | None, Some b_ -> simplify_and_force ~n:(n + 1) a b_
       | Some a_, Some b_ -> simplify_and_force ~n:(n + 1) a_ b_)
     | _, Bin (And, a, b) ->
-      simplify_and ~n:(n + 1)
+      simplify_and_ ~n:(n + 1)
         {expression_desc = Bin (And, e1, a); comment = None}
         b
     | Bin (Or, a, b), _ -> (
-      let ao = simplify_and ~n:(n + 1) a e2 in
-      let bo = simplify_and ~n:(n + 1) b e2 in
+      let ao = simplify_and_ ~n:(n + 1) a e2 in
+      let bo = simplify_and_ ~n:(n + 1) b e2 in
       match (ao, bo) with
       | Some {expression_desc = Bool false}, None ->
         Some {expression_desc = Bin (And, b, e2); comment = None}
       | None, Some {expression_desc = Bool false} ->
         Some {expression_desc = Bin (And, a, e2); comment = None}
       | None, _ | _, None -> (
-        match simplify_or ~n:(n + 1) a b with
+        match simplify_or_ ~n:(n + 1) a b with
         | None -> None
         | Some e -> simplify_and_force ~n:(n + 1) e e2)
       | Some a_, Some b_ -> simplify_or_force ~n:(n + 1) a_ b_)
@@ -997,7 +992,7 @@ let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
              _ ) as is_array) )
       when Js_op_util.same_vident ia ib ->
       Some {expression_desc = is_array; comment = None}
-    | x, y when x = y -> Some e1
+    | _ when Js_analyzer.eq_expression e1 e2 -> Some e1
     | ( Bin
           ( EqEqEq,
             {expression_desc = Var ia},
@@ -1022,7 +1017,9 @@ let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
             ({expression_desc = Bool _ | Null | Undefined _ | Number _ | Str _}
              as v2) ) )
       when Js_op_util.same_vident ia ib && op1 != op2 ->
-      if v1 = v2 then Some false_ else if op1 = EqEqEq then Some e1 else Some e2
+      if Js_analyzer.eq_expression v1 v2 then Some false_
+      else if op1 = EqEqEq then Some e1
+      else Some e2
     | _ -> None
   in
   (if debug then
@@ -1035,12 +1032,12 @@ let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
   res
 
 and simplify_and_force ~n (e1 : t) (e2 : t) : t option =
-  match simplify_and ~n e1 e2 with
+  match simplify_and_ ~n e1 e2 with
   | None -> Some {expression_desc = Bin (And, e1, e2); comment = None}
   | x -> x
 
 (**
-  [simplify_or e1 e2] attempts to simplify the boolean OR expression [e1 || e2].
+  [simplify_or_ e1 e2] attempts to simplify the boolean OR expression [e1 || e2].
   Returns [Some simplified] if simplification is possible, [None] otherwise.
 
   Basic simplification rules:
@@ -1059,7 +1056,7 @@ and simplify_and_force ~n (e1 : t) (e2 : t) : t option =
   This transformation allows reuse of [simplify_and]'s more extensive optimizations
   in the context of OR expressions.
 *)
-and simplify_or ~n (e1 : t) (e2 : t) : t option =
+and simplify_or_ ~n (e1 : t) (e2 : t) : t option =
   if debug then
     Printf.eprintf "%ssimplify_or %s %s\n"
       (String.make (n * 2) ' ')
@@ -1074,7 +1071,7 @@ and simplify_or ~n (e1 : t) (e2 : t) : t option =
     | _ -> (
       match (push_negation e1, push_negation e2) with
       | Some e1_, Some e2_ -> (
-        match simplify_and ~n:(n + 1) e1_ e2_ with
+        match simplify_and_ ~n:(n + 1) e1_ e2_ with
         | Some e -> push_negation e
         | None -> None)
       | _ -> None)
@@ -1089,10 +1086,18 @@ and simplify_or ~n (e1 : t) (e2 : t) : t option =
   res
 
 and simplify_or_force ~n (e1 : t) (e2 : t) : t option =
-  match simplify_or ~n e1 e2 with
+  match simplify_or_ ~n e1 e2 with
   | None -> Some {expression_desc = Bin (Or, e1, e2); comment = None}
   | x -> x
 
+let simplify_and (e1 : t) (e2 : t) : t option =
+  if no_side_effect e1 && no_side_effect e2 then simplify_and_ ~n:0 e1 e2
+  else None
+
+let simplify_or (e1 : t) (e2 : t) : t option =
+  if no_side_effect e1 && no_side_effect e2 then simplify_or_ ~n:0 e1 e2
+  else None
+
 let and_ ?comment (e1 : t) (e2 : t) : t =
   match (e1.expression_desc, e2.expression_desc) with
   | Var i, Var j when Js_op_util.same_vident i j -> e1
@@ -1109,7 +1114,7 @@ let and_ ?comment (e1 : t) (e2 : t) : t =
     when Js_op_util.same_vident i j ->
     e2
   | _, _ -> (
-    match simplify_and ~n:0 e1 e2 with
+    match simplify_and e1 e2 with
     | Some e -> e
     | None -> {expression_desc = Bin (And, e1, e2); comment})
 
@@ -1123,7 +1128,7 @@ let or_ ?comment (e1 : t) (e2 : t) =
     when Js_op_util.same_vident i j ->
     {e2 with expression_desc = Bin (Or, r, l)}
   | _, _ -> (
-    match simplify_or ~n:0 e1 e2 with
+    match simplify_or e1 e2 with
     | Some e -> e
     | None -> {expression_desc = Bin (Or, e1, e2); comment})
 
@@ -1138,14 +1143,22 @@ let not (e : t) : t =
   | Bin (Ge, a, b) -> {e with expression_desc = Bin (Lt, a, b)}
   | Bin (Le, a, b) -> {e with expression_desc = Bin (Gt, a, b)}
   | Bin (Gt, a, b) -> {e with expression_desc = Bin (Le, a, b)}
-  | _ -> {expression_desc = Js_not e; comment = None}
+  | _ -> (
+    match push_negation e with
+    | Some e_ -> e_
+    | None -> {expression_desc = Js_not e; comment = None})
 
 let not_empty_branch (x : t) =
   match x.expression_desc with
   | Number (Int {i = 0l}) | Undefined _ -> false
   | _ -> true
 
 let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
+  let default () =
+    if Js_analyzer.eq_expression ifso ifnot then
+      if no_side_effect pred then ifso else seq ?comment pred ifso
+    else {expression_desc = Cond (pred, ifso, ifnot); comment}
+  in
   match (pred.expression_desc, ifso.expression_desc, ifnot.expression_desc) with
   | Bool false, _, _ -> ifnot
   | Number (Int {i = 0l; _}), _, _ -> ifnot
@@ -1179,10 +1192,15 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
       Seq (a, {expression_desc = Undefined _}),
       Seq (b, {expression_desc = Undefined _}) ) ->
     seq (econd ?comment pred a b) undefined
-  | _ ->
-    if Js_analyzer.eq_expression ifso ifnot then
-      if no_side_effect pred then ifso else seq ?comment pred ifso
-    else {expression_desc = Cond (pred, ifso, ifnot); comment}
+  | _, _, Bool false -> (
+    match simplify_and pred ifso with
+    | Some e -> e
+    | None -> default ())
+  | _, Bool false, _ -> (
+    match simplify_and (not pred) ifnot with
+    | Some e -> e
+    | None -> default ())
+  | _ -> default ()
 
 let rec float_equal ?comment (e0 : t) (e1 : t) : t =
   match (e0.expression_desc, e1.expression_desc) with
@@ -1371,13 +1389,15 @@ let rec int_comp (cmp : Lam_compat.comparison) ?comment (e0 : t) (e1 : t) =
           _ ),
       Number (Int {i = 0l}) ) ->
     int_comp cmp l r (* = 0 > 0 < 0 *)
-  | Ceq, Optional_block _, Undefined _ | Ceq, Undefined _, Optional_block _ ->
+  | (Ceq, Optional_block _, Undefined _ | Ceq, Undefined _, Optional_block _)
+    when no_side_effect e0 && no_side_effect e1 ->
     false_
   | Ceq, _, _ -> int_equal e0 e1
   | Cneq, Optional_block _, Undefined _
   | Cneq, Undefined _, Optional_block _
   | Cneq, Caml_block _, Number _
-  | Cneq, Number _, Caml_block _ ->
+  | Cneq, Number _, Caml_block _
+    when no_side_effect e0 && no_side_effect e1 ->
     true_
   | _ -> bin ?comment (Lam_compile_util.jsop_of_comp cmp) e0 e1
 
@@ -1579,11 +1599,9 @@ let is_pos_pow n =
 
 let int32_mul ?comment (e1 : J.expression) (e2 : J.expression) : J.expression =
   match (e1, e2) with
-  | {expression_desc = Number (Int {i = 0l}); _}, x
-    when Js_analyzer.no_side_effect_expression x ->
+  | {expression_desc = Number (Int {i = 0l}); _}, x when no_side_effect x ->
     zero_int_literal
-  | x, {expression_desc = Number (Int {i = 0l}); _}
-    when Js_analyzer.no_side_effect_expression x ->
+  | x, {expression_desc = Number (Int {i = 0l}); _} when no_side_effect x ->
     zero_int_literal
   | ( {expression_desc = Number (Int {i = i0}); _},
       {expression_desc = Number (Int {i = i1}); _} ) ->
@@ -1695,7 +1713,7 @@ let of_block ?comment ?e block : t =
 let is_null ?comment (x : t) = triple_equal ?comment x nil
 let is_undef ?comment x = triple_equal ?comment x undefined
 
-let for_sure_js_null_undefined (x : t) =
+let is_null_undefined_constant (x : t) =
   match x.expression_desc with
   | Null | Undefined _ -> true
   | _ -> false
@@ -1707,28 +1725,36 @@ let is_null_undefined ?comment (x : t) : t =
   | _ -> {comment; expression_desc = Is_null_or_undefined x}
 
 let eq_null_undefined_boolean ?comment (a : t) (b : t) =
+  (* [a == b] when either a or b is null or undefined *)
   match (a.expression_desc, b.expression_desc) with
   | ( (Null | Undefined _),
-      (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) ) ->
+      (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) )
+    when no_side_effect b ->
     false_
   | ( (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _),
-      (Null | Undefined _) ) ->
+      (Null | Undefined _) )
+    when no_side_effect a ->
     false_
   | Null, Undefined _ | Undefined _, Null -> false_
   | Null, Null | Undefined _, Undefined _ -> true_
   | _ -> {expression_desc = Bin (EqEqEq, a, b); comment}
 
 let neq_null_undefined_boolean ?comment (a : t) (b : t) =
+  (* [a != b] when either a or b is null or undefined *)
   match (a.expression_desc, b.expression_desc) with
   | ( (Null | Undefined _),
-      (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) ) ->
+      (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) )
+    when no_side_effect b ->
     true_
   | ( (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _),
-      (Null | Undefined _) ) ->
+      (Null | Undefined _) )
+    when no_side_effect a ->
     true_
   | Null, Null | Undefined _, Undefined _ -> false_
   | Null, Undefined _ | Undefined _, Null -> true_
-  | _ -> {expression_desc = Bin (NotEqEq, a, b); comment}
+  | _ ->
+    let _ = assert false in
+    {expression_desc = Bin (NotEqEq, a, b); comment}
 
 let make_exception (s : string) =
   pure_runtime_call Primitive_modules.exceptions Literals.create [str s]

compiler/core/js_exp_make.mli

@@ -143,8 +143,6 @@ val array_length : ?comment:string -> t -> t
 
 val string_length : ?comment:string -> t -> t
 
-val bytes_length : ?comment:string -> t -> t
-
 val function_length : ?comment:string -> t -> t
 
 val string_append : ?comment:string -> t -> t -> t
@@ -365,7 +363,7 @@ val is_null : ?comment:string -> t -> t
 
 val is_undef : ?comment:string -> t -> t
 
-val for_sure_js_null_undefined : J.expression -> bool
+val is_null_undefined_constant : J.expression -> bool
 
 val is_null_undefined : ?comment:string -> t -> t
 

compiler/core/lam_compile_primitive.ml

@@ -373,13 +373,13 @@ let translate output_prefix loc (cxt : Lam_compile_context.t)
     match args with
     | [e1; e2]
       when cmp = Ceq
-           && (E.for_sure_js_null_undefined e1
-              || E.for_sure_js_null_undefined e2) ->
+           && (E.is_null_undefined_constant e1
+              || E.is_null_undefined_constant e2) ->
       E.eq_null_undefined_boolean e1 e2
     | [e1; e2]
       when cmp = Cneq
-           && (E.for_sure_js_null_undefined e1
-              || E.for_sure_js_null_undefined e2) ->
+           && (E.is_null_undefined_constant e1
+              || E.is_null_undefined_constant e2) ->
       E.neq_null_undefined_boolean e1 e2
     | [e1; e2] ->
       Location.prerr_warning loc Warnings.Bs_polymorphic_comparison;