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

Aberta
#9,228 0 comentários 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 1 dia

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
4/5
Tempo estimado
3-5 dias
Facilidade para iniciantes
44/100
Tipo de issue
Bug
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
javascript, ocaml
Domínio
compilers

Direção de pesquisa

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.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

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.

Linguagem predominante
OCaml
Estrelas
7.5k
Forks
485
Merge médio
17h 9min
PRs com merge (30d)
264

Preparar o ambiente

Abrir no Codespaces

Inicia o contêiner de desenvolvimento do projeto no navegador, com a sua própria conta do GitHub.

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de rescript-lang/rescript

Todas as issues de rescript-lang/rescript

Issues semelhantes

Mais issues de Compilers

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.