Skip to content

Negation #7138

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.

Sign up for GitHub

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 11 commits into from
Oct 31, 2024
Merged

Negation #7138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4b4b7b0
Better logic to simplify negation.
cristianoc Oct 31, 2024
e582405
Fix Array.isArray.
cristianoc Oct 31, 2024
eae1f41
number
cristianoc Oct 31, 2024
a417524
Handle combination of typeof and != const
cristianoc Oct 31, 2024
6a8dde2
case isArray
cristianoc Oct 31, 2024
194846d
More interesting example.
cristianoc Oct 31, 2024
036a32d
Update CHANGELOG.md
cristianoc Oct 31, 2024
39dc804
Update Core_NullableTests.mjs
cristianoc Oct 31, 2024
cfed809
Fix stale tests.
cristianoc Oct 31, 2024
4e1120d
Fix logic.
cristianoc Oct 31, 2024
e07ee33
Simplfy e?true:false to e
cristianoc Oct 31, 2024
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 CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
- 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


#### :house: Internal

Expand Down
198 changes: 151 additions & 47 deletions compiler/core/js_exp_make.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,36 @@ let bin ?comment (op : J.binop) (e0 : t) (e1 : t) : t =
let string_of_expression = ref (fun _ -> "")
let debug = false

(**
[push_negation e] attempts to simplify a negated expression by pushing the negation
deeper into the expression tree. Returns [Some simplified] if simplification is possible,
[None] otherwise.

Simplification rules:
- [!(not e)] -> [e]
- [!true] -> [false]
- [!false] -> [true]
- [!(a === b)] -> [a !== b]
- [!(a !== b)] -> [a === b]
- [!(a && b)] -> [!a || !b] (De Morgan's law)
- [!(a || b)] -> [!a && !b] (De Morgan's law)
*)
let rec push_negation (e : t) : t option =
match e.expression_desc with
| Js_not e -> Some e
| Bool b -> Some (bool (not b))
| Bin (EqEqEq, a, b) -> Some {e with expression_desc = Bin (NotEqEq, a, b)}
| Bin (NotEqEq, a, b) -> Some {e with expression_desc = Bin (EqEqEq, a, b)}
| Bin (And, a, b) -> (
match (push_negation a, push_negation b) with
| Some a_, Some b_ -> Some {e with expression_desc = Bin (Or, a_, b_)}
| _ -> None)
| Bin (Or, a, b) -> (
match (push_negation a, push_negation b) with
| Some a_, Some b_ -> Some {e with expression_desc = Bin (And, a_, b_)}
| _ -> None)
| _ -> None

(**
[simplify_and e1 e2] attempts to simplify the boolean AND expression [e1 && e2].
Returns [Some simplified] if simplification is possible, [None] otherwise.
Expand All @@ -681,7 +711,16 @@ let debug = false

Type check optimizations:
- [(typeof x === "boolean") && (x === true/false)] -> [x === true/false]
- [(typeof x === "string" || Array.isArray(x)) && (x === boolean/null/undefined)] -> [false]
- [(typeof x ==="boolean" | "string" | "number") && (x === boolean/null/undefined)] -> [false]
- [(Array.isArray(x)) && (x === boolean/null/undefined)] -> [false]

- [(typeof x === "boolean") && (x !== true/false)] -> unchanged
- [(typeof x === "boolean" | "string" | "number") && (x !== boolean/null/undefined)] -> [typeof x === ...]
- [(Array.isArray(x)) && (x !== boolean/null/undefined)] -> [Array.isArray(x)]

Equality optimizations:
- [e && e] -> [e]
- [(x === boolean/null/undefined) && (x === boolean/null/undefined)] -> [false] (when not equal)

Note: The function preserves the semantics of the original expression while
attempting to reduce it to a simpler form. If no simplification is possible,
Expand All @@ -691,7 +730,6 @@ 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_
Expand All @@ -700,39 +738,21 @@ let rec simplify_and (e1 : t) (e2 : t) : t option =
| 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 (ao, bo) with
| None, _ | _, None -> None
| Some a_, Some b_ -> (
match simplify_and a_ b_ with
| None -> Some {expression_desc = Bin (And, a_, b_); comment = None}
| Some e -> Some e)
| 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 (ao, bo) with
| None, _ | _, None -> None
| Some a_, Some b_ -> (
match simplify_or a_ b_ with
| None -> Some {expression_desc = Bin (Or, a_, b_); comment = None}
| Some e -> Some e)
| Some e -> Some e))
| ( Bin
( ((EqEqEq | NotEqEq) as op1),
{expression_desc = Var i1},
Expand Down Expand Up @@ -760,15 +780,8 @@ let rec simplify_and (e1 : t) (e2 : t) : t option =
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"}} ),
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean" | "string" | "number"}} ),
Bin
( EqEqEq,
{expression_desc = Var ib},
Expand All @@ -779,17 +792,91 @@ let rec simplify_and (e1 : t) (e2 : t) : t option =
{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"}} ) )
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean" | "string" | "number"}} ) )
when Js_op_util.same_vident ia ib ->
(* Note: case boolean / Bool _ is handled above *)
Some false_
| ( Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var ia}],
_ ),
Bin
( EqEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ) )
| ( Bin
( EqEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ),
Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var ia}],
_ ) )
when Js_op_util.same_vident ia ib ->
Some false_
| ( Bin
( EqEqEq,
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean"}} ),
Bin (NotEqEq, {expression_desc = Var ib}, {expression_desc = Bool _}) )
| ( Bin (NotEqEq, {expression_desc = Var ib}, {expression_desc = Bool _}),
Bin
( EqEqEq,
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean"}} ) )
when Js_op_util.same_vident ia ib ->
None
| ( (Bin
( EqEqEq,
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean" | "string" | "number"}} ) as
typeof),
Bin
( NotEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ) )
| ( Bin
( NotEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ),
(Bin
( EqEqEq,
{expression_desc = Typeof {expression_desc = Var ia}},
{expression_desc = Str {txt = "boolean" | "string" | "number"}} ) as
typeof) )
when Js_op_util.same_vident ia ib ->
(* Note: case boolean / Bool _ is handled above *)
Some {expression_desc = typeof; comment = None}
| ( (Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var ia}],
_ ) as is_array),
Bin
( NotEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ) )
| ( Bin
( NotEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ),
(Call
( {expression_desc = Str {txt = "Array.isArray"}},
[{expression_desc = Var ia}],
_ ) 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
| ( Bin
( EqEqEq,
{expression_desc = Var ia},
{expression_desc = Bool _ | Null | Undefined _} ),
Bin
( EqEqEq,
{expression_desc = Var ib},
{expression_desc = Bool _ | Null | Undefined _} ) )
when Js_op_util.same_vident ia ib ->
(* Note: case x = y is handled above *)
Some false_
| _ -> None

Expand All @@ -802,6 +889,16 @@ let rec simplify_and (e1 : t) (e2 : t) : t option =
- [e || true] -> [true]
- [false || e] -> [e]
- [e || false] -> [e]

Algebraic transformation strategy:
When basic rules don't apply, attempts to leverage [simplify_and] by:
1. Using the equivalence: [e1 || e2 ≡ !(!(e1) && !(e2))]
2. Applying [push_negation] to get [!(e1)] and [!(e2)]
3. Using [simplify_and] on these negated terms
4. If successful, applying [push_negation] again to get the final result

This transformation allows reuse of [simplify_and]'s more extensive optimizations
in the context of OR expressions.
*)
and simplify_or (e1 : t) (e2 : t) : t option =
if debug then
Expand All @@ -813,7 +910,13 @@ and simplify_or (e1 : t) (e2 : t) : t option =
| _, Bool true -> Some true_
| Bool false, _ -> Some e2
| _, Bool false -> Some e1
| _ -> None
| _ -> (
match (push_negation e1, push_negation e2) with
| Some e1_, Some e2_ -> (
match simplify_and e1_ e2_ with
| Some e -> push_negation e
| None -> None)
| _ -> None)

let and_ ?comment (e1 : t) (e2 : t) : t =
match (e1.expression_desc, e2.expression_desc) with
Expand Down Expand Up @@ -874,6 +977,7 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
| (Number _ | Array _ | Caml_block _), _, _ when no_side_effect pred ->
ifso (* a block can not be false in OCAML, CF - relies on flow inference*)
| Bool true, _, _ -> ifso
| _, Bool true, Bool false -> pred
| _, Cond (pred1, ifso1, ifnot1), _
when Js_analyzer.eq_expression ifnot1 ifnot ->
(* {[
Expand Down
6 changes: 1 addition & 5 deletions lib/es6/Belt_Result.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ function getWithDefault(opt, $$default) {
}

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

function isError(x) {
Expand Down
6 changes: 1 addition & 5 deletions lib/es6/Result.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ function getOr(opt, $$default) {
}

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

function isError(x) {
Expand Down
6 changes: 1 addition & 5 deletions lib/js/Belt_Result.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ function getWithDefault(opt, $$default) {
}

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

function isError(x) {
Expand Down
6 changes: 1 addition & 5 deletions lib/js/Result.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ function getOr(opt, $$default) {
}

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

function isError(x) {
Expand Down
7 changes: 6 additions & 1 deletion scripts/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,12 @@ async function runTests() {
}

if (mochaTest) {
cp.execSync(rescript_exe, {
cp.execSync(`${rescript_exe} clean`, {
cwd: path.join(__dirname, "..", "tests/tests"),
stdio: [0, 1, 2],
});

cp.execSync(`${rescript_exe} build`, {
cwd: path.join(__dirname, "..", "tests/tests"),
stdio: [0, 1, 2],
});
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/src/UntaggedVariants.mjs
View file
Open in desktop
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 === undefined || match === null || match === true) {
if (match === true) {
let match$1 = s[1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't this actually incorrect before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it was incorrect

if (match$1 === undefined || match$1 === null || match$1 === false) {
if (match$1 === false) {
let match$2 = s[2];
if (match$2 === undefined || match$2 === null || match$2 === false || match$2 === true) {
console.log("Nope...");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't comment further below for some reason, and probably out of scope of this PR, but

          if (match$3 === undefined || match$3 === null || match$3 === false || match$3 === true) {
            console.log("Nope...");
            return;
          }
          if (typeof match$3 === "string" && match$3 === "My name is") {

could just be

          if (match$3 === "My name is") {

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this to an issue? after this PR
to keep things from growing too much

Expand Down
16 changes: 4 additions & 12 deletions tests/tests/src/and_or_simplify.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,23 @@


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

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

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

function check_undefined_neq_typeof(x) {
if (typeof x !== "boolean" || x === undefined) {
if (typeof x !== "boolean") {
return 4;
} else {
return 3;
Expand Down
2 changes: 1 addition & 1 deletion tests/tests/src/and_or_simplify.res
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@unboxed
type t = | @as(null) Null | @as(undefined) Undefined | B(bool) //| S(string)
type t = | @as(null) Null | @as(undefined) Undefined | B(bool) | S(string) | I(int)

let check_null_eq_typeof = x =>
switch x {
Expand Down
Loading
Loading