Skip to content

Commit 51d1abe

Browse files
committed
Only apply optimisations when there are no side effects.
1 parent f668c4f commit 51d1abe

File tree

3 files changed

+35
-25
lines changed

3 files changed

+35
-25
lines changed

compiler/core/js_analyzer.ml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,11 @@ let rec no_side_effect_expression_desc (x : J.expression_desc) =
109109
no_side_effect call_expr
110110
&& Ext_list.for_all strings no_side_effect
111111
&& Ext_list.for_all values no_side_effect
112-
| Js_not _ | Cond _ | FlatCall _ | Call _ | New _ | Raw_js_code _
113-
(* actually true? *) ->
114-
false
112+
| Js_not e -> no_side_effect e
113+
| Cond (a, b, c) -> no_side_effect a && no_side_effect b && no_side_effect c
114+
| Call ({expression_desc = Str {txt = "Array.isArray"}}, [e], _) ->
115+
no_side_effect e
116+
| FlatCall _ | Call _ | New _ | Raw_js_code _ (* actually true? *) -> false
115117
| Await _ -> false
116118
| Spread _ -> false
117119

compiler/core/js_exp_make.ml

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -696,13 +696,13 @@ let rec push_negation (e : t) : t option =
696696
| _ -> None
697697

698698
(**
699-
[simplify_and e1 e2] attempts to simplify the boolean AND expression [e1 && e2].
699+
[simplify_and_ e1 e2] attempts to simplify the boolean AND expression [e1 && e2].
700700
Returns [Some simplified] if simplification is possible, [None] otherwise.
701701
702702
Basic simplification rules:
703703
- [false && e] -> [false]
704704
- [true && e] -> [e]
705-
- [e && false] -> [false] If [e] has no side effects
705+
- [e && false] -> [false]
706706
- [e && true] -> [e]
707707
- [(a && b) && e] -> If either [a && e] or [b && e] can be simplified,
708708
creates new AND expression with simplified parts: [(a' && b')]
@@ -735,42 +735,42 @@ let rec push_negation (e : t) : t option =
735735
attempting to reduce it to a simpler form. If no simplification is possible,
736736
returns [None].
737737
*)
738-
let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
738+
let rec simplify_and_ ~n (e1 : t) (e2 : t) : t option =
739739
if debug then
740740
Printf.eprintf "%s simplify_and %s %s\n"
741741
(String.make (n * 2) ' ')
742742
(!string_of_expression e1) (!string_of_expression e2);
743743
let res =
744744
match (e1.expression_desc, e2.expression_desc) with
745745
| Bool false, _ -> Some false_
746-
| _, Bool false when no_side_effect e1 -> Some false_
746+
| _, Bool false -> Some false_
747747
| Bool true, _ -> Some e2
748748
| _, Bool true -> Some e1
749749
| Bin (And, a, b), _ -> (
750-
let ao = simplify_and ~n:(n + 1) a e2 in
751-
let bo = simplify_and ~n:(n + 1) b e2 in
750+
let ao = simplify_and_ ~n:(n + 1) a e2 in
751+
let bo = simplify_and_ ~n:(n + 1) b e2 in
752752
match (ao, bo) with
753753
| None, None -> (
754-
match simplify_and ~n:(n + 1) a b with
754+
match simplify_and_ ~n:(n + 1) a b with
755755
| None -> None
756756
| Some e -> simplify_and_force ~n:(n + 1) e e2)
757757
| Some a_, None -> simplify_and_force ~n:(n + 1) a_ b
758758
| None, Some b_ -> simplify_and_force ~n:(n + 1) a b_
759759
| Some a_, Some b_ -> simplify_and_force ~n:(n + 1) a_ b_)
760760
| _, Bin (And, a, b) ->
761-
simplify_and ~n:(n + 1)
761+
simplify_and_ ~n:(n + 1)
762762
{expression_desc = Bin (And, e1, a); comment = None}
763763
b
764764
| Bin (Or, a, b), _ -> (
765-
let ao = simplify_and ~n:(n + 1) a e2 in
766-
let bo = simplify_and ~n:(n + 1) b e2 in
765+
let ao = simplify_and_ ~n:(n + 1) a e2 in
766+
let bo = simplify_and_ ~n:(n + 1) b e2 in
767767
match (ao, bo) with
768768
| Some {expression_desc = Bool false}, None ->
769769
Some {expression_desc = Bin (And, b, e2); comment = None}
770770
| None, Some {expression_desc = Bool false} ->
771771
Some {expression_desc = Bin (And, a, e2); comment = None}
772772
| None, _ | _, None -> (
773-
match simplify_or ~n:(n + 1) a b with
773+
match simplify_or_ ~n:(n + 1) a b with
774774
| None -> None
775775
| Some e -> simplify_and_force ~n:(n + 1) e e2)
776776
| Some a_, Some b_ -> simplify_or_force ~n:(n + 1) a_ b_)
@@ -1037,12 +1037,12 @@ let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
10371037
res
10381038

10391039
and simplify_and_force ~n (e1 : t) (e2 : t) : t option =
1040-
match simplify_and ~n e1 e2 with
1040+
match simplify_and_ ~n e1 e2 with
10411041
| None -> Some {expression_desc = Bin (And, e1, e2); comment = None}
10421042
| x -> x
10431043

10441044
(**
1045-
[simplify_or e1 e2] attempts to simplify the boolean OR expression [e1 || e2].
1045+
[simplify_or_ e1 e2] attempts to simplify the boolean OR expression [e1 || e2].
10461046
Returns [Some simplified] if simplification is possible, [None] otherwise.
10471047
10481048
Basic simplification rules:
@@ -1061,7 +1061,7 @@ and simplify_and_force ~n (e1 : t) (e2 : t) : t option =
10611061
This transformation allows reuse of [simplify_and]'s more extensive optimizations
10621062
in the context of OR expressions.
10631063
*)
1064-
and simplify_or ~n (e1 : t) (e2 : t) : t option =
1064+
and simplify_or_ ~n (e1 : t) (e2 : t) : t option =
10651065
if debug then
10661066
Printf.eprintf "%ssimplify_or %s %s\n"
10671067
(String.make (n * 2) ' ')
@@ -1076,7 +1076,7 @@ and simplify_or ~n (e1 : t) (e2 : t) : t option =
10761076
| _ -> (
10771077
match (push_negation e1, push_negation e2) with
10781078
| Some e1_, Some e2_ -> (
1079-
match simplify_and ~n:(n + 1) e1_ e2_ with
1079+
match simplify_and_ ~n:(n + 1) e1_ e2_ with
10801080
| Some e -> push_negation e
10811081
| None -> None)
10821082
| _ -> None)
@@ -1091,10 +1091,18 @@ and simplify_or ~n (e1 : t) (e2 : t) : t option =
10911091
res
10921092

10931093
and simplify_or_force ~n (e1 : t) (e2 : t) : t option =
1094-
match simplify_or ~n e1 e2 with
1094+
match simplify_or_ ~n e1 e2 with
10951095
| None -> Some {expression_desc = Bin (Or, e1, e2); comment = None}
10961096
| x -> x
10971097

1098+
let simplify_and (e1 : t) (e2 : t) : t option =
1099+
if no_side_effect e1 && no_side_effect e2 then simplify_and_ ~n:0 e1 e2
1100+
else None
1101+
1102+
let simplify_or (e1 : t) (e2 : t) : t option =
1103+
if no_side_effect e1 && no_side_effect e2 then simplify_or_ ~n:0 e1 e2
1104+
else None
1105+
10981106
let and_ ?comment (e1 : t) (e2 : t) : t =
10991107
match (e1.expression_desc, e2.expression_desc) with
11001108
| Var i, Var j when Js_op_util.same_vident i j -> e1
@@ -1111,7 +1119,7 @@ let and_ ?comment (e1 : t) (e2 : t) : t =
11111119
when Js_op_util.same_vident i j ->
11121120
e2
11131121
| _, _ -> (
1114-
match simplify_and ~n:0 e1 e2 with
1122+
match simplify_and e1 e2 with
11151123
| Some e -> e
11161124
| None -> {expression_desc = Bin (And, e1, e2); comment})
11171125

@@ -1125,7 +1133,7 @@ let or_ ?comment (e1 : t) (e2 : t) =
11251133
when Js_op_util.same_vident i j ->
11261134
{e2 with expression_desc = Bin (Or, r, l)}
11271135
| _, _ -> (
1128-
match simplify_or ~n:0 e1 e2 with
1136+
match simplify_or e1 e2 with
11291137
| Some e -> e
11301138
| None -> {expression_desc = Bin (Or, e1, e2); comment})
11311139

@@ -1190,11 +1198,11 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
11901198
Seq (b, {expression_desc = Undefined _}) ) ->
11911199
seq (econd ?comment pred a b) undefined
11921200
| _, _, Bool false -> (
1193-
match simplify_and ~n:0 pred ifso with
1201+
match simplify_and pred ifso with
11941202
| Some e -> e
11951203
| None -> default ())
11961204
| _, Bool false, _ -> (
1197-
match simplify_and ~n:0 (not pred) ifnot with
1205+
match simplify_and (not pred) ifnot with
11981206
| Some e -> e
11991207
| None -> default ())
12001208
| _ -> default ()

tests/tests/src/option_repr_test.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,9 @@ b("File \"option_repr_test.res\", line 127, characters 3-10", Belt_List.every(xs
221221
let xs_1$1 = {
222222
hd: neqx(undefined, null),
223223
tl: {
224-
hd: Primitive_object.equal(Primitive_option.some(undefined), Primitive_option.some(undefined)),
224+
hd: Primitive_object.equal(Primitive_option.some(undefined), Primitive_option.some(undefined)) && Primitive_object.equal(Primitive_option.some(undefined), Primitive_option.some(undefined)),
225225
tl: {
226-
hd: Primitive_object.equal(Primitive_option.some(Primitive_option.some(undefined)), Primitive_option.some(Primitive_option.some(undefined))),
226+
hd: Primitive_object.equal(Primitive_option.some(Primitive_option.some(undefined)), Primitive_option.some(Primitive_option.some(undefined))) && Primitive_object.equal(Primitive_option.some(Primitive_option.some(undefined)), Primitive_option.some(Primitive_option.some(undefined))),
227227
tl: {
228228
hd: Primitive_object.notequal(Primitive_option.some(Primitive_option.some(Primitive_option.some(undefined))), Primitive_option.some(Primitive_option.some(undefined))) && Primitive_object.notequal(Primitive_option.some(Primitive_option.some(undefined)), Primitive_option.some(Primitive_option.some(Primitive_option.some(undefined)))),
229229
tl: /* [] */0

0 commit comments

Comments
 (0)