【Firefox】super();呼ばなかったときのエラーメッセージを改善した【SpiderMonkey】


はじめに

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

あわせて読みたい

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

変更内容

バグレポート

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

つまり、

$ cd js/src/build_DBG.OBJ
$ ./dist/bin/js temp1.js
ReferenceError: |this| used uninitialized in A class constructor
$ ./dist/bin/js temp2.js
ReferenceError: |this| used uninitialized in arrow function in class constructor
temp1.js
class B {};
class A extends B {
    constructor() {
      this.someValue = 'value';
    }
}
new A();
temp2.js
class B {};
class A extends B {
    constructor() {
        (() => {
            this.someValue = 'value';
        })()
    }
}
new A();

となっていたものを

$ ./dist/bin/js temp1.js
ReferenceError: must call super constructor before using |this| in A class constructor
$ ./dist/bin/js temp2.js
ReferenceError: must call super constructor before using |this| in arrow function in derived class constructor

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

パッチ

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

エラーメッセージ出力の仕組み

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

  • js.msgというファイルにエラーメッセージ一覧が定義されている
  • Parser.cppでパースするときや、Interpreter.cppでバイトコードが解釈されるときに例外が起こったとき、ReportなんとかErrorなんとか(...)みたいな関数でエラーメッセージが出力される

どう変更したか

具体的にどのような変更を行ったのか、先に簡単に紹介します。
といっても今回は文字列を変えただけなので超簡単な変更でした。

  1. js.msgに定義されているエラーメッセージ2つを次のように変更

    Before
    MSG_DEF(JSMSG_UNINITIALIZED_THIS,       1, JSEXN_REFERENCEERR, "|this| used uninitialized in {0} class constructor")
    MSG_DEF(JSMSG_UNINITIALIZED_THIS_ARROW, 0, JSEXN_REFERENCEERR, "|this| used uninitialized in arrow function in class constructor")
    
    After
    MSG_DEF(JSMSG_UNINITIALIZED_THIS,       1, JSEXN_REFERENCEERR, "must call super constructor before using |this| in {0} class constructor")
    MSG_DEF(JSMSG_UNINITIALIZED_THIS_ARROW, 0, JSEXN_REFERENCEERR, "must call super constructor before using |this| in arrow function in derived class constructor")
    
  2. エラーメッセージを変更したので、js/src/jit-test/tests/debug/bug1385843.jsで期待する文字列も変更

    Before
    assertEq(ex.message.includes("uninitialized"), true);
    
    After
    assertEq(ex.message.includes("call super constructor"), true);
    
  3. js/src/tests/ecma_6/Class/uninitializedThisError.jsも同様に期待するエラーメッセージを変更

    Before
    if (className !== "")
        expected = "|this| used uninitialized in " + className + " class constructor";
    else
        expected = "|this| used uninitialized in arrow function in class constructor";
    
    After
    if (className !== "")
        expected = "must call super constructor before using |this| in " + className + " class constructor";
    else
        expected = "must call super constructor before using |this| in arrow function in derived class constructor";
    

どう手探ったか

それでは上記の変更箇所を具体的にどのように手探っていったかを残していきます。
例によってSearchfoxとかDXRとかを利用すると快適にコードサーチできます。

  1. "|this| used uninitialized in"で検索
  2. 以下の2つのエラーメッセージが見つかる(と同時にこのエラーメッセージをテストしているテストケースも見つかる)

    MSG_DEF(JSMSG_UNINITIALIZED_THIS,       1, JSEXN_REFERENCEERR, "|this| used uninitialized in {0} class constructor")
    MSG_DEF(JSMSG_UNINITIALIZED_THIS_ARROW, 0, JSEXN_REFERENCEERR, "|this| used uninitialized in arrow function in class constructor")
    
  3. "JSMSG_UNINITIALIZED_THIS"で検索すると、使用されているところは1箇所しかなかったので、文字列を変えるだけで問題なさそうということが分かる。

  4. 文字列を変更!

  5. 次に"JSMSG_UNINITIALIZED_THIS_ARROW"で検索すると、こちらも使用箇所が1箇所しかなかったので、文字列を変えるだけで問題なさそうと分かる。

  6. 文字列を変更!

  7. js/src/tests/ecma_6/Class/uninitializedThisError.jsでこのエラーメッセージに関するテストをしていることを最初に見つけていたので、今回の変更に合わせてテストを変更

  8. 文字列変えるだけのお仕事だったぜ!と意気揚々とパッチを作ってBugzillaに投げ、レビューもしてもらってtry-runを走らせてもらいました。

  9. しかし、try-runが落ちた! try-runのログから原因を探してみると、jit-testというもので落ちていることが分かった。

  10. js/src/jit-test/tests/debug/bug1385843.jsで同じくエラーメッセージのテストをしているところを発見したので、ここも今回の変更に沿うように修正

  11. 次はtry-runも通って無事本体に取り込まれました!