rescript-lang/rescript

Inefficient code emitted for pattern match on unboxed variants

Closed this issue · 6 comments

let f = (x: JSON.t) => switch x {
  | Null => Console.log("null")
  | _ => ()
}

is compiled to

function f(x) {
  if (!(!Array.isArray(x) && (x === null || typeof x !== "object") && typeof x !== "number" && typeof x !== "string" && typeof x !== "boolean")) {
    return ;
  }
  console.log("null");
}

Ideally, this would just be

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

Once the conditional is expressed in this order, no further simplification is possible (the cases are exhaustive only if you carry around the type, which you don't).

So an intervention needs to happen at the point where the conditional is generated.

If one writes the conditional by hand, with variations on negation, order, etc, it would be interesting to see which cases are problematic.

Here's another example:

@unboxed
type rec t =
  | Boolean(bool)
  | @as(null) Null
  | @as(undefined) Undefined
  | String(string)
  | Number(float)
  | Object(Js_dict.t<t>)
  | Array(array<t>)
  
let f = (x: t) =>
  switch x {
  | Null | Undefined => Console.log("abc")
  | _ => ()
  }

It seems to happen when all the nullary cases have the same rhs.

@cknitt have you observed similar, but not identical cases?

I cannot say for sure. I have observed this behavior (or similar?) a few times before creating this issue, but unfortunately did not collect the exact code involved.
Will let you know when I find a different example.

It seems to happen when all the nullary cases have the same rhs.

But even with different rhs,

@unboxed
type rec t =
  | Boolean(bool)
  | @as(null) Null
  | @as(undefined) Undefined
  | String(string)
  | Number(float)
  | Object(Js_dict.t<t>)
  | Array(array<t>)
  
let f = (x: t) =>
  switch x {
  | Null  => Console.log("abc")
  | Undefined  => Console.log("def")
  | _ => ()
  }

yields

function f(x) {
  if (!(!Array.isArray(x) && (x === null || typeof x !== "object") && typeof x !== "number" && typeof x !== "string" && typeof x !== "boolean")) {
    return ;
  }
  if (x === null) {
    console.log("abc");
    return ;
  }
  console.log("def");
}

but could be

function f(x) {
  if (x === null) {
    console.log("abc");
  }
  else if (x === undefined) {
    console.log("def");
  }
}

?

OK the behaviour makes sense.
What the compiler is saying is: if "it's one of the cases with payload" then....
This would be if "x is an object" without unboxed, which is very simple.

With unboxed, there are 2 possibilities of saying the same thing:

  1. it's one of the cases with payloads
  2. it's not one of the cases without payload
    Depending on the specific definition, either 1 or 2 is better.

In these examples, 2 would be better.
Choosing between 1 and 2 seems simpler than trying to change the order, as the order comes out of the pattern matching compiler, and there are many cases one could consider.

@cknitt thoughts?