【Firefox】クラスの再宣言時のエラーメッセージを改善した【SpiderMonkey】

# はじめに

大学の実験でFirefoxのJavaScriptエンジン「SpiderMonkey」にコントリビュートする機会が得られたので、そのときに行った手順を具体的に残したいと思います。
SpiderMonkeyに変更を加えたい人の参考に少しでもなれば幸いです。

# あわせて読みたい

この記事は実験で実際に解決したSpiderMonkeyのIssueのうち「その1」についてです。

- [この実験についての詳細](http://snowman-mh.hatenablog.com/entry/2017/11/14/172328)
- [SpiderMonkeyのコミッターになる方法](https://qiita.com/snowman_mh/items/b2bc19d2af54d79c271a)
- [解決したIssueその2](https://qiita.com/snowman_mh/items/e33f0b57e31f59acdc2d)
- [解決したIssueその3](https://qiita.com/snowman_mh/items/f71029c1bb3b942295d1)

# 変更内容

## バグレポート

今回取り組んだIssueはこちら。
https://bugzilla.mozilla.org/show_bug.cgi?id=1282431

つまり、

```js
js> class Foo {}; class Foo {};
SyntaxError: redeclaration of let Foo
```

となっていたものを

```js
js> class Foo {}; class Foo {};
SyntaxError: redeclaration of class Foo
```

となるように変更しました。

## パッチ

今回行った変更内容のdiffはこちら。
https://hg.mozilla.org/mozilla-central/rev/dafc1282a533

## 変数、クラス宣言の仕組み

2, 3日ソースコードを眺めただけのやつが言うことなので、間違っているところ、勘違いしているところは多々あるかと思うので、優しく指摘していただけると嬉しいです。

- SpiderMonkeyでは、変数宣言をコンパイラがパースしてBytecodeと呼ばれる中間言語に変換する
- インタプリタがそのバイトコードを解釈する
- コンパイラがパースするときやインタプリタが解釈するときに何か例外が起こるとエラーが出力される
- `enum class DeclarationKind`で宣言を種類分けしている(`Let`, `Var`, `Const`など)
- `Parser.cpp`というファイルの関数`classDefinition(...)`でクラスを定義している
- `DeclarationKind`で管理されている変数が再宣言されたとき、`static const char* DeclarationKindString(DeclarationKind)`という関数で再宣言に対応する文字列を生成する

## なぜ`class`は`let`として扱われていたか

- `let`で宣言した変数は`DeclarationKind::Let`で管理されていたが、`class`で宣言された場合も同じように`DeclarationKind::Let`で管理されていたから
  - 今回の変更を加えたあとも、パースされたあとは従来通り`class`も`let`として扱われるようにする必要がある

## どう変更したか

具体的にどのような変更を行ったのか、先に簡単に紹介します。

1. `NameAnalysisTypes.h`というファイルの`enum class DeclarationKind`に
`case Class`を追加
1. `Parser.cpp`というファイル内の関数`classDefinition(...)`の中で呼ばれている関数`noteDeclaredName(...)`に`DeclarationKind::Class`を渡すように変更(以前は`DeclarationKind::Let`を渡していた)
1. それ以外の部分では、`DeclarationKind::Class`は`DeclarationKind::Let`と同様に扱われるようにした

# どう手探ったか

それでは上記の変更箇所を具体的にどのように手探っていったかを残していきます。
手探る前に、[DXR](https://dxr.mozilla.org/mozilla-central/source/)というサイトを紹介します。
DXRでは、SpiderMonkeyのコードを検索することができます。
関数・変数の定義元に飛べたり、関数・変数が使われているところの一覧を出すことができたりして、該当のコードを探すときに超便利です。
検索のテクニックとして、`path:js/src`を頭につけて検索するとパスを制限できるので、応答が早くなります。
似たようなサイトで[Searchfox](https://searchfox.org/)というのもあります。

1. `"redeclaration of "`で検索
2. `js.msg`というファイルに以下のエラーメッセージの定義が見つかる。`JSMSG_REDECLARED_VAR`という識別子でこのエラーを呼んでいそうと分かる。

    ```
    MSG_DEF(JSMSG_REDECLARED_VAR, 2, JSEXN_SYNTAXERR, "redeclaration of {0} {1}")
    ```
1. `"JSMSG_REDECLARED_VAR"`で検索
2. `Parser.cpp`というファイルの1149行目あたりにエラーを呼んでそうな以下の関数が見つかる

    ```cpp
errorWithNotesAt(Move(notes), pos.begin, JSMSG_REDECLARED_VAR,
                     DeclarationKindString(prevKind), bytes.ptr())
    ```
3. ここで、`Parser.cpp`の1149行目にブレークポイントを設定してデバッガを走らせてみる。手順は次のような感じ。lldbというデバッガを使った。詳しい使い方は別途調べてみてください。

    ```bash
$ lldb ./dist/bin/js
(lldb) b Parser.cpp:1149
# ブレークポイントが設定される
(lldb) r temp.js
# さっきのブレークポイントで処理が止まる
(lldb) p DeclarationKindString(prevKind)
# => "let" となっていた！ ここが"class"になればOK！
    ```
    ```js:temp.js
class Foo {};
class Foo {};
    ```
1. 関数`DeclarationKindString(...)`の定義を見ると、`enum class DeclarationKind`の種類によって適切な文字列を返していた

    ```cpp
static const char*
DeclarationKindString(DeclarationKind kind)
{
      switch (kind) {

        ...

        case DeclarationKind::Var:
          return "var";
        case DeclarationKind::Let:
          return "let";
        case DeclarationKind::Const:
          return "const";

        ...

      }
      MOZ_CRASH("Bad DeclarationKind");
}
    ```
1. よって、`enum class DeclarationKind`に`case Class`を追加して、`DeclarationKindString(DeclarationKind)`では`DeclarationKind::Class`に対して`"class"`を返すように変更すれば良さそうと分かる
2. パースされたあとは`DeclarationKind::Class`は従来通り`DeclarationKind::Let`として扱われるべきなので、様々なところの`switch`文での`DeclarationKind::Class`の扱われ方は`DeclarationKind::Let`と同じように変更する
3. ここまでで、`DeclarationKind::Class`で登録されている変数に対しては適切に`"class"`を返すようになったが、クラスを宣言したときにメモリ(キャッシュ？)に登録される際に`DeclarationKind::Let`で登録されていることは変わりないので、次はクラスを宣言してメモリ領域に登録している部分を探す必要がある。
4. `DeclarationKind::Let`がどのように使用されているかを探してみる。運の良い(?)ことに、10箇所くらいしか使われている部分がなく、探しやすかった。
4. `Parser.cpp`の8508行目に次のような記述があり、変数宣言を登録してそうな感じがする

    ```cpp
        if (!noteDeclaredName(name, DeclarationKind::Let, namePos))
    ```
1. `noteDeclaredName(...)`が使われている場所をDXRの機能である「Find references」を使って探す
2. `Parser.cpp`7236行目の`noteDeclaredName(...)`が呼ばれている関数の名前が`classDefinition(...)`、7234行目に次の1行があり、これは勝った、って感じですね。

    ```cpp
        if (classContext == ClassStatement) {
    ```
1. ということで、7236行目の`DeclarationKind::Let`を`DeclarationKind::Class`にして、晴れて変更完了！という感じです。
2. 今回はクラス定義に関する変更を行ったので、`js/src/tests/ecma_6/Class/`にあるテストにパスすることを確認しました。
3. 最後にパッチを作ってBugzillaに投げて、レビューをもらって無事取り込まれました！