Skip to content
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

Better logic to simplify and/or. #7134

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions compiler/core/js_dump.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1348,3 +1348,5 @@ let string_of_expression (e : J.expression) =
let (_ : cxt) = expression ~level:0 Ext_pp_scope.empty f e in
P.flush f ();
Buffer.contents buffer

let () = E.string_of_expression := string_of_expression
170 changes: 128 additions & 42 deletions compiler/core/js_exp_make.ml
Original file line number Diff line number Diff line change
Expand Up @@ -662,42 +662,126 @@ let bin ?comment (op : J.binop) (e0 : t) (e1 : t) : t =
be careful for side effect
*)

let rec filter_bool (e : t) ~j ~b =
match e.expression_desc with
| Bin (And, e1, e2) -> (
match (filter_bool e1 ~j ~b, filter_bool e2 ~j ~b) with
| None, None -> None
| Some e, None | None, Some e -> Some e
| Some e1, Some e2 -> Some {e with expression_desc = Bin (And, e1, e2)})
| Bin (Or, e1, e2) -> (
match (filter_bool e1 ~j ~b, filter_bool e2 ~j ~b) with
| None, _ | _, None -> None
| Some e1, Some e2 -> Some {e with expression_desc = Bin (Or, e1, e2)})
| Bin (EqEqEq, {expression_desc = Var i}, {expression_desc = Bool b1})
| Bin (EqEqEq, {expression_desc = Bool b1}, {expression_desc = Var i})
when Js_op_util.same_vident i j ->
if b1 = b then None else Some e
| Bin (NotEqEq, {expression_desc = Var i}, {expression_desc = Bool b1})
| Bin (NotEqEq, {expression_desc = Bool b1}, {expression_desc = Var i})
when Js_op_util.same_vident i j ->
if b1 <> b then None else Some e
| Bin
( NotEqEq,
{expression_desc = Typeof {expression_desc = Var i}},
{expression_desc = Str {txt}} )
when Js_op_util.same_vident i j ->
if txt <> "bool" then None else assert false
| Js_not
{
expression_desc =
Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var i}],
_ );
}
when Js_op_util.same_vident i j ->
None
| _ -> Some e
let string_of_expression = ref (fun _ -> "")
let debug = false

let rec simplify_and (e1 : t) (e2 : t) : t option =
if debug then
Printf.eprintf "simplify_and %s %s\n" (!string_of_expression e1)
(!string_of_expression e2);

match (e1.expression_desc, e2.expression_desc) with
| Bool false, _ -> Some false_
| _, Bool false -> Some false_
| Bool true, _ -> Some e2
| _, Bool true -> Some e1
| Bin (And, a, b), _ -> (
let ao = simplify_and a e2 in
let bo = simplify_and b e2 in
if ao = None && bo = None then None
else
let a_ =
match ao with
| None -> a
| Some a_ -> a_
in
let b_ =
match bo with
| None -> b
| Some b_ -> b_
in
match simplify_and a_ b_ with
| None -> Some {expression_desc = Bin (And, a_, b_); comment = None}
| Some e -> Some e)
| Bin (Or, a, b), _ -> (
let ao = simplify_and a e2 in
let bo = simplify_and b e2 in
if ao = None && bo = None then None
else
let a_ =
match ao with
| None -> a
| Some a_ -> a_
in
let b_ =
match bo with
| None -> b
| Some b_ -> b_
in
match simplify_or a_ b_ with
| None -> Some {expression_desc = Bin (Or, a_, b_); comment = None}
| Some e -> Some e)
| ( Bin
( ((EqEqEq | NotEqEq) as op1),
{expression_desc = Var i1},
{expression_desc = Bool b1} ),
Bin
( ((EqEqEq | NotEqEq) as op2),
{expression_desc = Var i2},
{expression_desc = Bool b2} ) )
when Js_op_util.same_vident i1 i2 ->
let op_eq = op1 = op2 in
let consistent = if op_eq then b1 = b2 else b1 <> b2 in
if consistent then Some e1 else Some false_
| ( Bin
( EqEqEq,
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean"}} ),
(Bin (EqEqEq, {expression_desc = Var ib}, {expression_desc = Bool _}) as b)
)
| ( (Bin (EqEqEq, {expression_desc = Var ib}, {expression_desc = Bool _}) as b),
Bin
( EqEqEq,
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean"}} ) )
when Js_op_util.same_vident ia ib ->
Some {expression_desc = b; comment = None}
| ( Bin
( EqEqEq,
{
expression_desc =
( Typeof {expression_desc = Var ia}
| Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var ia}],
_ ) );
},
{expression_desc = Str {txt = "boolean" | "string"}} ),
Bin
( EqEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ) )
| ( Bin
( EqEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ),
Bin
( EqEqEq,
{
expression_desc =
( Typeof {expression_desc = Var ia}
| Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var ia}],
_ ) );
},
{expression_desc = Str {txt = "boolean" | "string"}} ) )
when Js_op_util.same_vident ia ib ->
(* Note: case boolean / Bool _ is handled above *)
Some false_
| _ -> None

and simplify_or (e1 : t) (e2 : t) : t option =
if debug then
Printf.eprintf "simplify_or %s %s\n" (!string_of_expression e1)
(!string_of_expression e2);

match (e1.expression_desc, e2.expression_desc) with
| Bool true, _ -> Some true_
| _, Bool true -> Some true_
| Bool false, _ -> Some e2
| _, Bool false -> Some e1
| _ -> None

let and_ ?comment (e1 : t) (e2 : t) : t =
match (e1.expression_desc, e2.expression_desc) with
Expand All @@ -714,11 +798,10 @@ let and_ ?comment (e1 : t) (e2 : t) : t =
)
when Js_op_util.same_vident i j ->
e2
| _, Bin (EqEqEq, {expression_desc = Var j}, {expression_desc = Bool b}) -> (
match filter_bool e1 ~j ~b with
| None -> e2
| Some e1 -> {expression_desc = Bin (And, e1, e2); comment})
| _, _ -> {expression_desc = Bin (And, e1, e2); comment}
| _, _ -> (
match simplify_and e1 e2 with
| Some e -> e
| None -> {expression_desc = Bin (And, e1, e2); comment})

let or_ ?comment (e1 : t) (e2 : t) =
match (e1.expression_desc, e2.expression_desc) with
Expand All @@ -729,7 +812,10 @@ let or_ ?comment (e1 : t) (e2 : t) =
| Var i, Bin (Or, l, ({expression_desc = Var j; _} as r))
when Js_op_util.same_vident i j ->
{e2 with expression_desc = Bin (Or, r, l)}
| _, _ -> {expression_desc = Bin (Or, e1, e2); comment}
| _, _ -> (
match simplify_or e1 e2 with
| Some e -> e
| None -> {expression_desc = Bin (Or, e1, e2); comment})

(* return a value of type boolean *)
(* TODO:
Expand Down
2 changes: 2 additions & 0 deletions compiler/core/js_exp_make.mli
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,5 @@ val is_null_undefined : ?comment:string -> t -> t
val make_exception : string -> t

val variadic_args : t list -> t list

val string_of_expression : (t -> string) ref
4 changes: 2 additions & 2 deletions tests/tests/src/UntaggedVariants.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,9 @@ function check$1(s) {
return;
}
let match = s[0];
if (match === true) {
if (match === undefined || match === null || match === true) {
let match$1 = s[1];
if (match$1 === false) {
if (match$1 === undefined || match$1 === null || match$1 === false) {
let match$2 = s[2];
if (match$2 === undefined || match$2 === null || match$2 === false || match$2 === true) {
console.log("Nope...");
Expand Down
42 changes: 42 additions & 0 deletions tests/tests/src/and_or_simplify.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Generated by ReScript, PLEASE EDIT WITH CARE


function check_null_eq_typeof(x) {
if (typeof x !== "boolean" || x !== null) {
return 4;
} else {
return 3;
}
}

function check_null_neq_typeof(x) {
if (typeof x !== "boolean" || x === null) {
return 4;
} else {
return 3;
}
}

function check_undefined_eq_typeof(x) {
if (typeof x !== "boolean" || x !== undefined) {
return 4;
} else {
return 3;
}
}

function check_undefined_neq_typeof(x) {
if (typeof x !== "boolean" || x === undefined) {
return 4;
} else {
return 3;
}
}

export {
check_null_eq_typeof,
check_null_neq_typeof,
check_undefined_eq_typeof,
check_undefined_neq_typeof,
}
/* No side effect */
26 changes: 26 additions & 0 deletions tests/tests/src/and_or_simplify.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
@unboxed
type t = | @as(null) Null | @as(undefined) Undefined | B(bool) //| S(string)

let check_null_eq_typeof = x =>
switch x {
| B(_) if x == Null => 3
| _ => 4
}

let check_null_neq_typeof = x =>
switch x {
| B(_) if x != Null => 3
| _ => 4
}

let check_undefined_eq_typeof = x =>
switch x {
| B(_) if x == Undefined => 3
| _ => 4
}

let check_undefined_neq_typeof = x =>
switch x {
| B(_) if x != Undefined => 3
| _ => 4
}
4 changes: 1 addition & 3 deletions tests/tests/src/option_repr_test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,6 @@ let xs$1 = {

b("File \"option_repr_test.res\", line 127, characters 3-10", Belt_List.every(xs$1, x => x));

let xs_0$2 = true && true;

let xs_1$1 = {
hd: neqx(undefined, null),
tl: {
Expand All @@ -235,7 +233,7 @@ let xs_1$1 = {
};

let xs$2 = {
hd: xs_0$2,
hd: true,
tl: xs_1$1
};

Expand Down
Loading