Skip to content

Simplify conditionals codegen + add missing side effects checks. #7151

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.

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 10 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
- 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 forconditionals. https://github.com/rescript-lang/rescript-compiler/pull/7151


#### :house: Internal
Expand Down
33 changes: 24 additions & 9 deletions compiler/core/js_exp_make.ml
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ let rec push_negation (e : t) : t option =
Basic simplification rules:
- [false && e] -> [false]
- [true && e] -> [e]
- [e && false] -> [false]
- [e && false] -> [false] If [e] has no side effects
- [e && true] -> [e]
- [(a && b) && e] -> If either [a && e] or [b && e] can be simplified,
creates new AND expression with simplified parts: [(a' && b')]
Expand Down Expand Up @@ -743,7 +743,7 @@ let rec simplify_and ~n (e1 : t) (e2 : t) : t option =
let res =
match (e1.expression_desc, e2.expression_desc) with
| Bool false, _ -> Some false_
| _, Bool false -> Some false_
| _, Bool false when no_side_effect e1 -> Some false_
| Bool true, _ -> Some e2
| _, Bool true -> Some e1
| Bin (And, a, b), _ -> (
Expand Down Expand Up @@ -997,7 +997,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},
Expand All @@ -1022,7 +1022,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
Expand Down Expand Up @@ -1138,14 +1140,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
Expand Down Expand Up @@ -1179,10 +1189,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 ~n:0 pred ifso with
| Some e -> e
| None -> default ())
| _, Bool false, _ -> (
match simplify_and ~n:0 (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
Expand Down
6 changes: 1 addition & 5 deletions lib/es6/Belt_List.js
Original file line number Diff line number Diff line change
Expand Up @@ -988,11 +988,7 @@ function eq(_l1, _l2, p) {
let l2 = _l2;
let l1 = _l1;
if (!l1) {
if (l2) {
return false;
} else {
return true;
}
return !l2;
}
if (!l2) {
return false;
Expand Down
10 changes: 2 additions & 8 deletions lib/es6/Belt_Result.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ function isOk(x) {
}

function isError(x) {
if (x.TAG === "Ok") {
return false;
} else {
return true;
}
return x.TAG !== "Ok";
}

function eq(a, b, f) {
Expand All @@ -71,10 +67,8 @@ function eq(a, b, f) {
} else {
return false;
}
} else if (b.TAG === "Ok") {
return false;
} else {
return true;
return b.TAG !== "Ok";
}
}

Expand Down
6 changes: 1 addition & 5 deletions lib/es6/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -1000,11 +1000,7 @@ function equal(_l1, _l2, p) {
let l2 = _l2;
let l1 = _l1;
if (!l1) {
if (l2) {
return false;
} else {
return true;
}
return !l2;
}
if (!l2) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion lib/es6/Primitive_option.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function fromNull(x) {
}

function valFromOption(x) {
if (!(x !== null && x.BS_PRIVATE_NESTED_SOME_NONE !== undefined)) {
if (x === null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
return x;
}
let depth = x.BS_PRIVATE_NESTED_SOME_NONE;
Expand Down
10 changes: 2 additions & 8 deletions lib/es6/Result.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ function isOk(x) {
}

function isError(x) {
if (x.TAG === "Ok") {
return false;
} else {
return true;
}
return x.TAG !== "Ok";
}

function equal(a, b, f) {
Expand All @@ -65,10 +61,8 @@ function equal(a, b, f) {
} else {
return false;
}
} else if (b.TAG === "Ok") {
return false;
} else {
return true;
return b.TAG !== "Ok";
}
}

Expand Down
6 changes: 1 addition & 5 deletions lib/js/Belt_List.js
Original file line number Diff line number Diff line change
Expand Up @@ -988,11 +988,7 @@ function eq(_l1, _l2, p) {
let l2 = _l2;
let l1 = _l1;
if (!l1) {
if (l2) {
return false;
} else {
return true;
}
return !l2;
}
if (!l2) {
return false;
Expand Down
10 changes: 2 additions & 8 deletions lib/js/Belt_Result.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ function isOk(x) {
}

function isError(x) {
if (x.TAG === "Ok") {
return false;
} else {
return true;
}
return x.TAG !== "Ok";
}

function eq(a, b, f) {
Expand All @@ -71,10 +67,8 @@ function eq(a, b, f) {
} else {
return false;
}
} else if (b.TAG === "Ok") {
return false;
} else {
return true;
return b.TAG !== "Ok";
}
}

Expand Down
6 changes: 1 addition & 5 deletions lib/js/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -1000,11 +1000,7 @@ function equal(_l1, _l2, p) {
let l2 = _l2;
let l1 = _l1;
if (!l1) {
if (l2) {
return false;
} else {
return true;
}
return !l2;
}
if (!l2) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion lib/js/Primitive_option.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function fromNull(x) {
}

function valFromOption(x) {
if (!(x !== null && x.BS_PRIVATE_NESTED_SOME_NONE !== undefined)) {
if (x === null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
return x;
}
let depth = x.BS_PRIVATE_NESTED_SOME_NONE;
Expand Down
10 changes: 2 additions & 8 deletions lib/js/Result.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ function isOk(x) {
}

function isError(x) {
if (x.TAG === "Ok") {
return false;
} else {
return true;
}
return x.TAG !== "Ok";
}

function equal(a, b, f) {
Expand All @@ -65,10 +61,8 @@ function equal(a, b, f) {
} else {
return false;
}
} else if (b.TAG === "Ok") {
return false;
} else {
return true;
return b.TAG !== "Ok";
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/tests/src/UntaggedVariants.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ let TrickyNested = {
};

function checkEnum(e) {
if (!(e === "One" || e === "Three" || e === "Two")) {
if (e !== "One" && e !== "Three" && e !== "Two") {
return "Something else..." + e;
}
switch (e) {
Expand All @@ -252,7 +252,7 @@ let OverlapString = {
};

function checkEnum$1(e) {
if (!(e === 1.0 || e === "Three" || e === "Two")) {
if (e !== 1.0 && e !== "Three" && e !== "Two") {
return "Something else...";
}
switch (e) {
Expand All @@ -270,7 +270,7 @@ let OverlapNumber = {
};

function checkEnum$2(e) {
if (!(e === null || typeof e !== "object")) {
if (e !== null && typeof e === "object") {
return "Object...";
}
switch (e) {
Expand Down
4 changes: 1 addition & 3 deletions tests/tests/src/bdd.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,8 @@ function random_vars(n) {
function bool_equal(a, b) {
if (a) {
return b;
} else if (b) {
return false;
} else {
return true;
return !b;
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/tests/src/bs_poly_map_test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -210,21 +210,21 @@ let match$1 = match[0];

let match$2 = Belt_Map.get(v3, 10);

b("File \"bs_poly_map_test.res\", line 155, characters 4-11", match$2 !== undefined ? match$2 === 11 : false);
b("File \"bs_poly_map_test.res\", line 155, characters 4-11", match$2 === 11);

let match$3 = Belt_Map.get(v3, -10);

b("File \"bs_poly_map_test.res\", line 162, characters 4-11", match$3 === undefined);

let match$4 = Belt_Map.get(v4, -10);

b("File \"bs_poly_map_test.res\", line 169, characters 4-11", match$4 !== undefined ? match$4 === 0 : false);
b("File \"bs_poly_map_test.res\", line 169, characters 4-11", match$4 === 0);

b("File \"bs_poly_map_test.res\", line 175, characters 4-11", Belt_Map.isEmpty(Belt_Map.remove(Belt_Map.make(Icmp), 0)));

b("File \"bs_poly_map_test.res\", line 176, characters 4-11", Belt_Map.isEmpty(Belt_Map.removeMany(Belt_Map.make(Icmp), [0])));

b("File \"bs_poly_map_test.res\", line 178, characters 4-11", pres !== undefined ? pres === 5000 : false);
b("File \"bs_poly_map_test.res\", line 178, characters 4-11", pres === 5000);

b("File \"bs_poly_map_test.res\", line 184, characters 4-11", Belt_Array.eq(Belt_Map.keysToArray(match$1[0]), Belt_Array.makeBy(5000, i => i), (prim0, prim1) => prim0 === prim1));

Expand Down
6 changes: 3 additions & 3 deletions tests/tests/src/complex_while_loop.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ function f() {
let n = 0;
while ((() => {
let fib = x => {
if (x === 0 || x === 1) {
return 1;
} else {
if (x !== 0 && x !== 1) {
return fib(x - 1 | 0) + fib(x - 2 | 0) | 0;
} else {
return 1;
}
};
return fib(n) > 10;
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/src/core/Core_NullableTests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function shouldHandleNullableValues() {
let tUndefined = undefined;
let tValue = "hello";
let tmp;
tmp = (tNull == null) ? tNull === null : false;
tmp = tNull === null;
Test.run([
[
"Core_NullableTests.res",
Expand All @@ -29,7 +29,7 @@ function shouldHandleNullableValues() {
"Should handle undefined"
], tmp$1, (prim0, prim1) => prim0 === prim1, true);
let tmp$2;
tmp$2 = (tValue == null) ? false : tValue === "hello";
tmp$2 = tValue === "hello";
Test.run([
[
"Core_NullableTests.res",
Expand Down
6 changes: 3 additions & 3 deletions tests/tests/src/demo_page.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import * as React from "react";
import * as ReactDom from "react-dom";

function fib(x) {
if (x === 2 || x === 1) {
return 1;
} else {
if (x !== 2 && x !== 1) {
return fib(x - 1 | 0) + fib(x - 2 | 0) | 0;
} else {
return 1;
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/tests/src/exception_rebound_err_test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function test_js_error4() {
}
} else if (e.RE_EXN_ID === B) {
return 5;
} else if (e.RE_EXN_ID === C && !(e._1 !== 1 || e._2 !== 2)) {
} else if (e.RE_EXN_ID === C && e._1 === 1 && e._2 === 2) {
return 6;
} else {
return 7;
Expand Down
6 changes: 3 additions & 3 deletions tests/tests/src/fib.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@


function fib(x) {
if (x === 0 || x === 1) {
return 1;
} else {
if (x !== 0 && x !== 1) {
return fib(x - 1 | 0) + fib(x - 2 | 0) | 0;
} else {
return 1;
}
}

Expand Down
Loading
Loading