`let x = e; f(x)` evaluates `e` after the callee `f`

Start in compiler/core/lam_util.ml at Lam_util.refine_let, especially its (Call) and (Prim) clauses; trace the callers named in the issue, including deep_flatten, simplify_lets, beta reduction, and SROA. Check the #9177 delay_safe behavior and the examples in the issue, including the @val call and record-field cases. Done means generated JavaScript preserves the source evaluation order across these cases while retaining the reported cb(g(1)) optimization.

let x = e; f(x), where the call is the rest of the block and x is its only argument, compiles to f(e) even when f is itself an expression such as a call, an await or a record field read. JavaScript evaluates the callee before the arguments, so e now runs after f: a let that reads mutable state sees the state the callee left behind, and an effect of the let happens after the effects of the callee or after an await.

type counter = {mutable n: int}
let c = {n: 0}
let next = () => {
  c.n = c.n + 1
  (x: int) => x
}
let test = () => {
  let before = c.n
  next()(before)
}
Console.log(test())

function test() {
  return next()(c.n);
}

console.log(next()(c.n));

This prints 1. before is read before next() increments c.n, so test() should return 0.

The callee does not need effects of its own. With type t = {mutable f: int => int} and a swap() that assigns r.f and returns 1, let x = swap(); r.f(x) compiles to return r.f(swap());, which calls the function that r.f held before swap() replaced it.

An effect of the let moves past an await in the callee:

let log = (s: string) => {
  Console.log(s)
  s
}
let getHandler = async () => {
  Console.log("getHandler")
  (s: string) => s
}
let test = async () => {
  let x = log("argument")
  (await getHandler())(x)
}
test()->ignore

async function test() {
  return (await getHandler())((console.log("argument"), "argument"));
}

This prints getHandler, then argument. In the source, log("argument") runs before test suspends; in the output it runs after the await resumes.

Inlining produces the same shape, and the binding that #9177 adds for it is removed again. With c and next from the first example:

let callNext = (x: int) => next()(x)
let test = () => callNext(c.n)
Console.log(test())

function test() {
  return next()(c.n);
}

This prints 1. callNext(c.n) reads c.n before the body of callNext calls next(), so it should print 0.

Since 13.0.0-alpha.6 the same happens to a field of a local record that has a mutable field. With c and next from the first example:

type snap = {mutable unused: int, saved: int}
let test = () => {
  let s = {unused: 0, saved: c.n}
  next()(s.saved)
}
Console.log(test())

function test() {
  return next()(c.n);
}

This prints 1. 12.3.1 and 13.0.0-alpha.5 keep the record and print 0:

function test() {
  let s = {
    unused: 0,
    saved: c.n
  };
  return next()(s.saved);
}

Lam_util.refine_let (compiler/core/lam_util.ml) rewrites let x = e in f x to f e in its (Call) clause, Lapply {ap_func = fn; ap_args = [Lvar w]}, after checking only that fn does not mention x. That moves e from before fn to after it. Lam_pass_deep_flatten (in the aux of deep_flatten and in lambda_of_groups) and Lam_pass_lets_dce (in lets_helper, behind simplify_lets) call refine_let on the lets they keep; the examples before the inlining one are rewritten by the first deep_flatten pass. Lam_beta_reduce.propagate_beta_reduce, propagate_beta_reduce_with_map and no_names_beta_reduce call it for the bindings they create for inlined arguments, in call order. In the inlining example, propagate_beta_reduce_with_map binds c.n because #9177 does not let it move past next(), and refine_let folds it back in. bind_fields in Lam_pass_sroa.replace (compiler/core/lam_pass_sroa.ml) calls it for every field that is read but never written. The record example is new in 13.0.0-alpha.6 because #8617 made Lam_pass_sroa replace records with several fields by variables and bind the read-only fields through refine_let.

Folding in the (Call) clause only when the callee or the argument is Lam_analysis.delay_safe, the predicate #9177 uses before moving an argument, gives the expected output for the examples above and still compiles let x = g(1); cb(x) with a parameter cb to cb(g(1)). A call through a record field, even an immutable one, then keeps its binding, because delay_safe treats every record field read as a read of mutable state (#9223).

The (Prim) clause just above the (Call) clause has the same problem. Its comment says it is for pure primitives, but it applies to every primitive except Pmakeblock and Pjsx, including the Pjs_call of a @val or @scope external, which reads the function from a global before the argument. With @scope("globalThis") @val external hook: int => int = "hook", let x = install(); hook(x) compiles to globalThis.hook(install()), which calls the hook that was there before install() replaced it.

Reproduced with 12.0.0-alpha.1, 12.3.1, 13.0.0-alpha.5, 13.0.0-alpha.6 and master (fc7444266); the record example only with 13.0.0-alpha.6 and master. 11.1.4 shows the (Call) examples only in curried mode: in uncurried mode it compiled each call through Js.Internal.opaqueFullApply, which #6893 removed for 12.0.0-alpha.1. The @val example reproduces with 11.1.4 in both modes.

Related: #9175, #9177, #9223.