Skip to content

Heuristic for pattern matching untagged variants. #7128

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 5 commits into from
Oct 26, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
- 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

#### :house: Internal

Expand Down
8 changes: 8 additions & 0 deletions compiler/core/js_exp_make.ml
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,14 @@ let rec filter_bool (e : t) ~j ~b =
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}},
Expand Down
43 changes: 36 additions & 7 deletions compiler/ml/ast_untagged_variants.ml
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,14 @@ module DynamicChecks = struct
| Not of 'a t
| Expr of 'a

let rec size = function
| BinOp (_, x, y) -> 1 + size x + size y
| TagType _ -> 1
| TypeOf x -> 1 + size x
| IsInstanceOf (_, x) -> 1 + size x
| Not x -> 1 + size x
| Expr _ -> 1

let bin op x y = BinOp (op, x, y)
let tag_type t = TagType t
let typeof x = TypeOf x
Expand All @@ -396,7 +404,7 @@ module DynamicChecks = struct
let ( &&& ) x y = bin And x y

let rec is_a_literal_case ~(literal_cases : tag_type list) ~block_cases
(e : _ t) =
~list_literal_cases (e : _ t) =
let literals_overlaps_with_string () =
Ext_list.exists literal_cases (function
| String _ -> true
Expand Down Expand Up @@ -458,12 +466,33 @@ module DynamicChecks = struct
Ext_list.fold_right others is_literal_1 (fun literal_n acc ->
is_literal_case literal_n ||| acc))
in
match block_cases with
| [c] -> is_not_block_case c
| c1 :: (_ :: _ as rest) ->
is_not_block_case c1
&&& is_a_literal_case ~literal_cases ~block_cases:rest e
| [] -> assert false
if list_literal_cases then
let rec mk cases =
match List.rev cases with
| [case] -> is_literal_case case
| case :: rest -> is_literal_case case ||| mk rest
| [] -> assert false
in
mk literal_cases
else
match block_cases with
| [c] -> is_not_block_case c
| c1 :: (_ :: _ as rest) ->
is_not_block_case c1
&&& is_a_literal_case ~literal_cases ~block_cases:rest
~list_literal_cases e
| [] -> assert false

let is_a_literal_case ~literal_cases ~block_cases e =
let with_literal_cases =
is_a_literal_case ~literal_cases ~block_cases ~list_literal_cases:true e
in
let without_literal_cases =
is_a_literal_case ~literal_cases ~block_cases ~list_literal_cases:false e
in
if size with_literal_cases <= size without_literal_cases then
with_literal_cases
else without_literal_cases

let is_int_tag ?(has_null_undefined_other = (false, false, false)) (e : _ t) :
_ t =
Expand Down
40 changes: 20 additions & 20 deletions tests/tests/src/UntaggedVariants.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as Primitive_array from "rescript/lib/es6/Primitive_array.js";
import * as Primitive_option from "rescript/lib/es6/Primitive_option.js";

function classify(x) {
if (x === "A" && typeof x !== "number") {
if (x === "A") {
return "A";
} else if (typeof x === "number") {
return "An integer";
Expand Down Expand Up @@ -40,13 +40,13 @@ let ListWithTuples = {};
let ListWithObjects = {};

function tuplesToObjects(l) {
if (Array.isArray(l)) {
if (l === undefined) {
return null;
} else {
return {
hd: l[0],
tl: tuplesToObjects(l[1])
};
} else {
return null;
}
}

Expand All @@ -68,7 +68,7 @@ console.log("l1", l1);
console.log("l2", l2);

function isTrue(x) {
if (typeof x !== "object") {
if (x === true) {
return true;
} else {
return x.flag;
Expand All @@ -80,7 +80,7 @@ let Truthy = {
};

function classify$1(x) {
if (x === null || typeof x !== "object") {
if (x === null || x === undefined) {
if (x === null) {
return "null";
} else {
Expand Down Expand Up @@ -112,7 +112,7 @@ let Unknown = {
};

function classify$3(x) {
if (typeof x !== "object" && typeof x !== "number" && (x === "C" || x === "B" || x === "A" || x === "D")) {
if (x === "A" || x === "D" || x === "B" || x === "C") {
switch (x) {
case "A" :
return "a";
Expand Down Expand Up @@ -173,7 +173,7 @@ let WithArray = {
};

function classify$6(x) {
if (!Array.isArray(x) && (x === null || typeof x !== "object") && typeof x !== "number" && typeof x !== "string") {
if (x === false || x === null || x === true) {
switch (x) {
case false :
return "JSONFalse";
Expand Down Expand Up @@ -214,18 +214,18 @@ let Json = {
};

function check(s, y) {
if (!Array.isArray(s)) {
if (s === "B") {
return 42;
}
let x = s[0];
if (!Array.isArray(x)) {
if (x === "B") {
return 42;
}
let tmp = s[1];
if (Array.isArray(tmp) || x === y) {
return 42;
} else {
if (tmp === "B" && x !== y) {
return 41;
} else {
return 42;
}
}

Expand All @@ -234,7 +234,7 @@ let TrickyNested = {
};

function checkEnum(e) {
if (!(e === "Two" || e === "One" || e === "Three")) {
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 === "Two" || e === 1.0 || e === "Three")) {
if (!(e === 1.0 || e === "Three" || e === "Two")) {
return "Something else...";
}
switch (e) {
Expand Down Expand Up @@ -376,7 +376,7 @@ let TestFunctionCase = {
let someJson = '[{"name": "Haan"}, {"name": "Mr"}, false]';

function check$1(s) {
if (!Array.isArray(s) && (s === null || typeof s !== "object") && typeof s !== "number" && typeof s !== "string") {
if (s === undefined || s === null || s === false || s === true) {
console.log("Nope...");
return;
}
Expand All @@ -390,7 +390,7 @@ function check$1(s) {
let match$1 = s[1];
if (match$1 === false) {
let match$2 = s[2];
if (!Array.isArray(match$2) && (match$2 === null || typeof match$2 !== "object") && typeof match$2 !== "number" && typeof match$2 !== "string") {
if (match$2 === undefined || match$2 === null || match$2 === false || match$2 === true) {
console.log("Nope...");
return;
}
Expand All @@ -400,13 +400,13 @@ function check$1(s) {
return;
}
let match$3 = match$2[0];
if (!Array.isArray(match$3) && (match$3 === null || typeof match$3 !== "object") && typeof match$3 !== "number" && typeof match$3 !== "string") {
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") {
let match$4 = match$2[1];
if (!Array.isArray(match$4) && (match$4 === null || typeof match$4 !== "object") && typeof match$4 !== "number" && typeof match$4 !== "string") {
if (match$4 === undefined || match$4 === null || match$4 === false || match$4 === true) {
console.log("Nope...");
return;
}
Expand Down Expand Up @@ -476,7 +476,7 @@ let PromiseSync = {
};

async function classify$10(a) {
if (typeof a !== "object" && !(a instanceof Promise) && (a === "test" || a === 12) && !Array.isArray(a)) {
if (a === "test" || a === 12) {
if (a === "test") {
console.log("testing");
return;
Expand Down
10 changes: 5 additions & 5 deletions tests/tests/src/core/Core_JsonTests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import * as Test from "./Test.mjs";
function decodeJsonTest() {
let json = {"someProp":{"otherProp": null, "thirdProp": [true, false]}};
let decodedCorrectly;
if (!Array.isArray(json) && (json === null || typeof json !== "object") && typeof json !== "number" && typeof json !== "string" && typeof json !== "boolean" || !(typeof json === "object" && !Array.isArray(json))) {
if (json === null || !(typeof json === "object" && !Array.isArray(json))) {
decodedCorrectly = false;
} else {
let match = json["someProp"];
if (match !== undefined && !(!Array.isArray(match) && (match === null || typeof match !== "object") && typeof match !== "number" && typeof match !== "string" && typeof match !== "boolean" || !(typeof match === "object" && !Array.isArray(match)))) {
if (match !== undefined && !(match === null || !(typeof match === "object" && !Array.isArray(match)))) {
let match$1 = match["thirdProp"];
if (match$1 !== undefined && !(!Array.isArray(match$1) && (match$1 === null || typeof match$1 !== "object") && typeof match$1 !== "number" && typeof match$1 !== "string" && typeof match$1 !== "boolean" || !(Array.isArray(match$1) && match$1.length === 2))) {
if (match$1 !== undefined && !(match$1 === null || !(Array.isArray(match$1) && match$1.length === 2))) {
let match$2 = match$1[0];
if (!Array.isArray(match$2) && (match$2 === null || typeof match$2 !== "object") && typeof match$2 !== "number" && typeof match$2 !== "string" && typeof match$2 !== "boolean" || !(typeof match$2 === "boolean" && match$2)) {
if (match$2 === null || !(typeof match$2 === "boolean" && match$2)) {
decodedCorrectly = false;
} else {
let match$3 = match$1[1];
decodedCorrectly = !Array.isArray(match$3) && (match$3 === null || typeof match$3 !== "object") && typeof match$3 !== "number" && typeof match$3 !== "string" && typeof match$3 !== "boolean" || !(typeof match$3 === "boolean" && !match$3) ? false : true;
decodedCorrectly = match$3 === null || !(typeof match$3 === "boolean" && !match$3) ? false : true;
}
} else {
decodedCorrectly = false;
Expand Down
22 changes: 22 additions & 0 deletions tests/tests/src/untagged_variants_heuristic.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Generated by ReScript, PLEASE EDIT WITH CARE


function f(x) {
if (x !== null) {
return;
}
console.log("abc");
}

function ff(x) {
if (typeof x !== "number") {
return;
}
console.log("abc");
}

export {
f,
ff,
}
/* No side effect */
22 changes: 22 additions & 0 deletions tests/tests/src/untagged_variants_heuristic.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
let f = (x: JSON.t) =>
switch x {
| Null => Console.log("abc")
| _ => ()
}

@unboxed
type t =
| A
| B
| C
| D
| E
| F
| G
| Int(int)

let ff = (x: t) =>
switch x {
| Int(_) => Console.log("abc")
| _ => ()
}
8 changes: 4 additions & 4 deletions tests/tests/src/variantsMatching.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -274,21 +274,21 @@ function isWhyNot(x) {
}

function plus$3(x, y) {
if (x === undefined || x === null || x === "WhyNotAnotherOne") {
if (x === null || x === "WhyNotAnotherOne" || x === undefined) {
switch (x) {
case null :
case undefined :
return y;
case "WhyNotAnotherOne" :
break;
}
} else if (!(y === undefined || y === null || y === "WhyNotAnotherOne")) {
} else if (!(y === null || y === "WhyNotAnotherOne" || y === undefined)) {
return {
x: x.x + y.x,
y: x.y + y.y
};
}
if (!(y === undefined || y === null || y === "WhyNotAnotherOne")) {
if (!(y === null || y === "WhyNotAnotherOne" || y === undefined)) {
return "WhyNotAnotherOne";
}
switch (y) {
Expand All @@ -301,7 +301,7 @@ function plus$3(x, y) {
}

function kind$1(x) {
if (!(x === undefined || x === null || x === "WhyNotAnotherOne")) {
if (!(x === null || x === "WhyNotAnotherOne" || x === undefined)) {
return "present";
}
switch (x) {
Expand Down
Loading