492150 - Unexpected error occurred when javascript.options.strict is true

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This is reported to bugzilla-jp and contributors checks this bug.

See the testcase: http://bugzilla.mozilla.gr.jp/attachment.cgi?id=4029

> <script type="application/x-javascript">
> 
> try {
> 	throw 'foo';
> }
> catch(e) {
> 	var e = 'bar';
> }
> document.write('OK');
> 
> </script>

If the catch block defines var with defined name, the line throw an error *only* when javascript.options.strict is true.

Sakai-san proposes a patch:
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=4030&action=diff

He guesses that this is a mistake of bug 487209.

Comment 1

[Atsushi Sakai]

Attachment: Patch v1.0

It's same as the patch already attached to bugzilla.mozilla.gr.jp, Nakano-san mentioned.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 376517 [details] [diff] [review]
Patch v1.0

Oops, sorry about this -- should have seen it. But let's get mrbkap's less biased review here. Thanks for the patch!

/be

Comment 3

[Brendan Eich [:brendan]]

Easy fix, should take if not a blocker (this bug is a regression from 3.0.x).

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 376517 [details] [diff] [review]
Patch v1.0

This is the right fix, but I wonder if it might be better to common out the now non-trivial condition like:

            bool error = op == JSOP_DEFCONST ||
                         dn_kind == JSDefinition::CONST ||
                         (dn_kind == JSDefinition::LET &&
                          (stmt->type != STMT_CATCH || OuterLet(tc, stmt, atom)));
            if (JS_HAS_STRICT_OPTION(cx)
                ? op != JSOP_DEFVAR || dn_kind != JSDefinition::VAR
                : error) {
                name = js_AtomToPrintableString(cx, atom);
                if (!name ||
                    !js_ReportCompileErrorNumber(cx, TS(tc->compiler), pn,
                                                 (!error)
                                                 ? JSREPORT_WARNING | JSREPORT_STRICT
                                                 : JSREPORT_ERROR,
                                                 JSMSG_REDECLARED_VAR,
                                                 JSDefinition::kindString(dn_kind),
                                                 name)) {
                    return JS_FALSE;
                }
            }

I'll leave it up to brendan to have the final word on that.

Comment 5

[Brendan Eich [:brendan]]

Attachment: fix just committed

Comment 6

[Brendan Eich [:brendan]]

Fixed:

http://hg.mozilla.org/tracemonkey/rev/c8a11e7cde90
http://hg.mozilla.org/mozilla-central/rev/6ea54bfd7c28

/be

Comment 7

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/821944c65a19