More than 5 years have passed since last update.

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

Last updated at 2017-11-14Posted at 2017-11-12

はじめに

大学の実験で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なんとか(...)みたいな関数でエラーメッセージが出力される

どう変更したか

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

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

エラーメッセージを変更したので、js/src/jit-test/tests/debug/bug1385843.jsで期待する文字列も変更
Before

assertEq(ex.message.includes("uninitialized"), true);
js:After
assertEq(ex.message.includes("call super constructor"), true);
```

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";
js: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とかを利用すると快適にコードサーチできます。

"|this| used uninitialized in"で検索
以下の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も通って無事本体に取り込まれました！

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up