A local ref's value stored in a tuple or passed to a local function changes when the ref is assigned later

オープン
#9,227 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
45/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
javascript, ocaml
領域
compilers

調査の方向性

Start by reading compiler/core/js_pass_flatten_and_mark_dead.ml around subst_map and mark_dead_code, then compare the local function path in compiler/core/js_pass_tailcall_inline.ml around subst and inline_call. Reproduce the tuple and local function examples in the issue and inspect the generated JavaScript. Done means values captured before a later assignment remain unchanged in both cases, with regression coverage for the reported cases.

索引モデルが issue の本文から書いたものです。

説明

When a local ref compiles to a JavaScript variable, a value read from it and stored in a tuple, record or variant, or passed to a local function that gets inlined, is later read from the variable itself. If the ref is assigned in between, the code gets the new value instead of the one that was stored or passed.

A tuple built from x.contents and taken apart after x := 2:

let f = () => {
  let x = ref(1)
  let t = (x.contents, "b")
  x := 2
  let (a, _) = t
  a
}
Console.log(f())
function f() {
  let x = 1;
  x = 2;
  return x;
}

This prints 2. a is the first element of t, which was built while x was 1, so it should print 1. The same happens with a record that has two or more fields and no mutable one, read as p.x, and with a constructor that has two or more payloads, taken apart with switch: {x: x.contents, label: "p"} and Pair(x.contents, "b") followed by x := 2 both give 2.

When the use is in a closure, the closure reads the variable's last value:

let snapshots = () => {
  let i = ref(0)
  let out = []
  while i.contents < 3 {
    let pair = (i.contents, "x")
    out->Array.push(() => {
      let (n, _) = pair
      n
    })
    i := i.contents + 1
  }
  out->Array.map(get => get())
}
Console.log(snapshots())
function snapshots() {
  let i = 0;
  let out = [];
  while (i < 3) {
    out.push(() => i);
    i = i + 1 | 0;
  };
  return out.map(get => get());
}

This prints [ 3, 3, 3 ] instead of [ 0, 1, 2 ].

A local function called in tail position with count.contents, whose body assigns the ref:

let run = () => {
  let count = ref(0)
  let bump = before => {
    count := count.contents + 1
    (before, count.contents)
  }
  bump(count.contents)
}
Console.log(run())
function run() {
  let count = 0;
  count = count + 1 | 0;
  return [
    count,
    count
  ];
}

This prints [ 1, 1 ] instead of [ 0, 1 ]: before should be the value of count at the call. 12.3.1 compiles run with let before = count.contents; before the assignment and prints [ 0, 1 ].

In the tuple, closure, record and variant cases, subst_map in compiler/core/js_pass_flatten_and_mark_dead.ml handles let t = <immutable block> with two or more elements. It keeps the elements that are variables or constants, binds the other elements to new variables (t_0, ...), and records the block. A later t[i] or t.field (Array_index or Static_index on t) is replaced by the recorded element when that element is a variable or a constant, and mark_dead_code then marks t as unused, so it is not printed. Nothing checks whether that variable is assigned between the definition of t and the read. Before this pass, f is still let t = [x, "b"]; x = 2; return t[0];, which returns 1.

In the local function case, Js_pass_tailcall_inline.subst (compiler/core/js_pass_tailcall_inline.ml) replaces return bump(count) with the body of bump, which is used once. inline_call maps a parameter whose argument is a variable to that variable (Var (Id id) -> Map_ident.add param id vars), and substitute_arguments renames the parameter's uses in the body. Number, boolean, undefined and null arguments are substituted too; only other arguments get a let binding. The Js_fun_env.no_tailcall mask keeps the binding for a parameter that a tail call reassigns, but nothing checks whether the argument variable is assigned in the body. The Lambda inliner binds such an argument: since #9177, Lam_beta_reduce.can_move puts a variable that is assigned somewhere (Lam_stats.assignable) in place of the parameter only when the body reads the parameter before anything with side effects.

Both substitutions are only correct for a variable that nothing assigns. For an assigned variable, the element or argument needs the binding that the passes already create for other expressions.

A local ref becomes a JavaScript variable when it does not escape. Up to 13.0.0-alpha.5 this excluded refs captured by a closure, so count in the local function case stayed {contents: 0}, the argument count.contents was not a variable, and it got let before = count.contents. #8617 (13.0.0-alpha.6) also turns captured refs into variables, which exposes that case.

The tuple, closure, record and variant cases reproduce with 11.1.4, 12.3.1, 13.0.0-alpha.5, 13.0.0-alpha.6 and master (fc7444266). The local function case reproduces with 13.0.0-alpha.6 and master (fc7444266), and compiles correctly with 12.3.1 and 13.0.0-alpha.5.

Related: #8617, #9159, #9177.

主要言語
OCaml
スター
7.5k
フォーク
485
平均マージ
17時間 9分
マージ済み PR(30日)
264

環境構築

Codespaces で開く

このプロジェクトの開発コンテナを、あなたの GitHub アカウントでブラウザ上に起動します。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

rescript-lang/rescript のほかの issue

rescript-lang/rescript の issue をすべて見る

似ている issue

Compilers の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。