522158 - ES5 strict mode: extra warning for duplicated prop names in object initializer

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

This is a followup bug for bug 514567:

One ugliness with this patch is that with JSOPTION_STRICT enabled, you
get two warnings:

js> ({x:1, x:2})
({x:1, x:2})
typein:5: warning: duplicated property name x in object literal:
typein:5: warning: ({x:1, x:2})
typein:5: warning: .........^
typein:5: strict warning: redeclaration of property x
({x:2})
js>

The first three lines are the warning from the compiler.  They reflect
a check added by this patch, on the principle that (almost) anything
strict mode forbids shall also become a JSOPTION_STRICT warning.

The fourth line is from the interpreter: the JSOP_INITPROP call in the
interpreter calls js_CheckRedeclaration, which produces that warning.

Since 1) we now check for duplicate property names at compile time, 2)
JSOP_INITPROP is only used for object literals, which start with an
object with no own properties, and 4) we don't warn about shadowing
properties from the prototype, it seems to me that we don't really
need to call js_CheckRedeclaration any more for JSOP_INITPROP.  I
haven't looked into JSOP_INITMETHOD.

Comment 1

[Brendan Eich [:brendan]]

JSOP_INITMETHOD is just like JSOP_INITPROP but enables lookahead from JSOP_LAMBDA. But don't forget that a duplicate property name can be a number:

            /* Annotate JSOP_INITELEM so we decompile 2:c and not just c. */
            if (pn3->pn_type == TOK_NUMBER) {
                if (js_NewSrcNote(cx, cg, SRC_INITPROP) < 0)
                    return JS_FALSE;
                if (js_Emit1(cx, cg, JSOP_INITELEM) < 0)
                    return JS_FALSE;
            } else {
                JS_ASSERT(pn3->pn_type == TOK_NAME ||
                          pn3->pn_type == TOK_STRING);
 ...

/be

Comment 2

[Jim Blandy :jimb]

(In reply to comment #1)
> JSOP_INITMETHOD is just like JSOP_INITPROP but enables lookahead from
> JSOP_LAMBDA.

Okay --- so treating them similarly in this case should be okay.

> But don't forget that a duplicate property name can be a number:

The patch in bug 514567 also handles numeric property names.  Please give that a look if it sounds like I've missed something.

Comment 3

[Jim Blandy :jimb]

Attachment: Avoid extra warning for duplicated prop names in object initializers.

One wrinkle here is that I've changed the tests to require JSOPTION_STRICT to be enabled when the code is *compiled*, not merely when it is executed; that's necessary if we're to delete the run-time checks altogether.

Comment 4

[Jim Blandy :jimb]

http://hg.mozilla.org/tracemonkey/rev/e4499666512b

Comment 5

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/e4499666512b