346902 - Incorrect uneval involving an expression that optimized to an object literal

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Jesse Ruderman]

http://www.squarefree.com/shell/shell.html, Mac trunk nightly

> function() { 1 ? {} : 0, alert(3) }
function () { {}, alert(3); }

> function () { {}, alert(3); }
syntax error

I'd expect uneval to either put parentheses around the object literal, or optimize it away altogether, leaving only alert(3) in the function.

Comment 1

[Jesse Ruderman]

Is this the same bug or something I should file separately?

> function() { 1 ? {}[alert(5)] : 0 }
function () { {}[alert(5)]; }

> function () { {}[alert(5)]; }
function () { [alert(5)]; }

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Is this the same bug or something I should file separately?
> 
> > function() { 1 ? {}[alert(5)] : 0 }
> function () { {}[alert(5)]; }

This is the bug.

> > function () { {}[alert(5)]; }
> function () { [alert(5)]; }

This is fine -- the empty block has been culled.

/be

Comment 3

[Brendan Eich [:brendan]]

This one seems worth fixing for 1.8.1.

/be

Comment 4

[Brendan Eich [:brendan]]

This one seems worth fixing for 1.8.1.

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: one possible fix

mrbkap had a neat alterna-patch that used the operator precedence magic coded in jsopcode.tbl, but I think it ran into a contradiction: it wanted

prec(ENDINIT) < prec(POP) && prec(POP) < prec(ASSIGN)

but to avoid extra parenthesization for x = {p: 2}, we want

prec(ASSIGN) < prec(ENDINIT)

which makes a cycle in the order chain.

So my patch uses brute force in the constant folder.

/be

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 234282 [details] [diff] [review]
one possible fix

>+        if (pn->pn_type == TOK_RP)
>+            return pn->pn_type == TOK_LP;

This can't be what you wanted.

Comment 7

[Brendan Eich [:brendan]]

Attachment: oops, corrected copy/paste bug

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 234292 [details] [diff] [review]
oops, corrected copy/paste bug

Another safe fix for uneval | eval round-tripping.

/be

Comment 9

[Brendan Eich [:brendan]]

Fixed on the trunk.

/be

Comment 10

[Bob Clary [:bc] (inactive)]

Checking in regress-346902.js;
/cvsroot/mozilla/js/tests/js1_5/Expressions/regress-346902.js,v  <--  regress-346902.js
initial revision: 1.1

Comment 11

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060818 windows/mac(ppc|tel)/linux

Comment 12

[Mike Schroepfer]

Comment on attachment 234292 [details] [diff] [review]
oops, corrected copy/paste bug

a=schrep for drivers - approving all [181approval pending] bugs now that the tree is open.

Comment 13

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 14

[Bob Clary [:bc] (inactive)]

verified fixed 1.8, 1.9 20060824 windows/mac*/linux

Comment 15

[Bob Clary [:bc] (inactive)]

Checking in regress-346902.js;
/cvsroot/mozilla/js/tests/js1_5/Expressions/regress-346902.js,v  <--  regress-346902.js
new revision: 1.2; previous revision: 1.1
done