Improve option optimization for constants

[cknitt]

Optimizes the option alias pass so we skip the redundant is-not-none guard whenever the compiler already knows the value is Some, which trims the JS we emit for simple constant cases.

[cknitt]

Before this change:

function constant() {
  let opt = 42;
  if (opt !== undefined) {
    console.log(opt);
    return;
  }
  
}

[pkg-pr-new]

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7913

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7913

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7913

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7913

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7913

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7913

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7913

commit: 64b3b83

[cristianoc]

This looks great.
I have a process going trying to find adversarial attacks, as an experiment.
This change looks so simple and clearly right, I don't expect surprises.

[cristianoc]

CODEX review:

The new clause in compiler/core/lam_pass_remove_alias.ml:87 skips the Pis_not_none guard whenever the metadata says we have a constant Some. That’s fine for well-behaved values, but it removes the last runtime check that protects the non-nested option encoding from stray undefineds. Before this change, constants showed up in Lam_stats as Constant, so we kept emitting foo !== undefined ? … : …; with the new shortcut we collapse directly to the Some arm.

You can see the behavioural change with a tiny repro that smuggles an undefined payload through Obj.magic (something we allow today for interop):

let foo: option<int> = Obj.magic(Some(Obj.magic(None: option<int>)))

let res =
  switch foo {
  | Some(_) => "some"
  | None => "none"
  }

Js.log(res)

• Building the compiler before this patch still outputs code that performs foo !== undefined at runtime, so it prints "none" when the payload is actually undefined.
• After this patch lands, the generated JS becomes let res = "some";, so the same program now prints "some".

I verified this by locally reverting the new Constant c branch, rebuilding (make), and comparing the generated tmp_magic_weird.js/cjs. The regression only reproduces once the new shortcut is present.

Could we gate the optimisation on Lam_constant.is_some_none_aux (i.e. ensure we’re not in the non-nested encoding with an undefined payload), or fall back to the old behaviour when the payload can be undefined?

[cristianoc]

[cristianoc]

CODEX follow-up:

You’re absolutely right—the simplified JS (let res = "some";) was already produced before this patch. I rebuilt the parent commit to double-check and confirmed that I’d misread the before/after comparison. Sorry for the noise!

[cristianoc]

However, the report is legit in a wider sense.
It opens a question of what we want to do with incorrect types coming from obj magic or incorrect externals.
I think we need a clear position on that.

[cknitt]

However, the report is legit in a wider sense. It opens a question of what we want to do with incorrect types coming from obj magic or incorrect externals. I think we need a clear position on that.

I think with incorrect externals, all bets are off.