Inline external aliases as early as we can

[TheSpyder]

I upgraded to version 8 today, I haven't upgraded since 7.2.2 and it turns out this changed in 7.3.0.

I'm dealing with a protobuf API. The objects it uses have all fields set, the active protocol is a real object and the rest are null. I settled on some nice-looking code that does a pattern match on multiple fields at the same time, but 7.3.0 introduces what could arguably be called a regression. This is a very simplified version of what I'm doing:

type protocol = {
  first: Js.null(int)
};
[@bs.val] external thing: protocol = "thing";

type variant = First(int) | Empty;

let toOpt = Js.Null.toOption;
let value =
  switch (thing.first->toOpt) {
  | Some(i) => First(i)
  | None =>Empty
  };
Js.log(value)

7.2.2 produces this JS:

var match = thing.first;

var value = match !== null ? /* First */[match] : /* Empty */0;

console.log(value);

7.3.0 adds unnecessary extra checks on what is now the i variable:

var i = thing.first;

var value = i !== null ? /* First */[i === null ? undefined : Caml_option.some(i)] : /* Empty */0;

console.log(value);

In particular, inside the i !== null check it now does a i === null check. Also the Caml_option.some() call is probably unnecessary? I don't mind if it has to stay, though. I'd just like the duplicate null check removed.

Here's a slightly more complex example running in the playground. It better matches my real code with multiple field checks, but only one check is required to reproduce the issue.

[bobzhang]

What will happen if you in-line toOpt

[TheSpyder]

Ah, on the playground that does appear to fix it. It's a lot more verbose in my real code but I'll give it a go there too. So I guess what changed is how that's inlined by the compiler?

[bobzhang]

There is a conflict between various optimizations

[TheSpyder]

I've managed to work around it in my code without too much difficulty, so this is probably a low priority.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[TheSpyder]

I no longer develop the code with the issue but this change does still seem worth looking into

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.