345855 - Syntax error with 'blank' yield expressions

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Aiko]

(Split from bug 326466 comment 143)

The following expressions give syntax errors but should be allowed (note that yield is not followed by an expression here)

  x = 12 + (yield)
  foo(yield)

I falsely claimed that the following expressions have the same problem but they're fine:

  x = 12 + (yield 42)
  foo(yield 42)

I don't know why Python doesn't allow "yield yield", maybe it's one of those "edge cases" mentioned in PEP 342?

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix

Fairly straightforward fix.  I chose to disallow yield yield ... to match Python.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: better fix

The previous patch degraded error messages for cases such as return) gratuitously.

This patch also outlaws |yield yield|, to match Python.

/be

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 231362 [details] [diff] [review]
better fix

Grr, jetlag.

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 231362 [details] [diff] [review]
better fix

No, this is fine.  Not sure what I was seeing, but this works in my Minefield build, and it makes sense. I chose to test (tt != TOK_YIELD || ...) in the #if'd line in order to make clearer what is going on with the (tt2 != tt && ...) right-hand-side of || there.

/be

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 231362 [details] [diff] [review]
better fix

Fixed on trunk.  This is another required js1.7 fix.

/be

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 231362 [details] [diff] [review]
better fix

a=drivers, please land this on the MOZILLA_1_8_BRANCH.

Comment 7

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch too.

/be

Comment 8

[Bob Clary [:bc] (inactive)]

Checking in regress-345855.js;
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-345855.js,v  <--  regress-345855.js
initial revision: 1.1

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed 1.8, 1.9 windows/mac(ppc|tel)/linux 20060803

Comment 10

[Bob Clary [:bc] (inactive)]

note, bug 384991 changed the behavior of js1_7/geniter/regress-345855.js on the trunk to no longer require yield be parenthesized which currently causes 

  expect = /SyntaxError: yield expression must be parenthesized/;
  try
  {
    eval('(function (){foo(yield 42)})');
    actual = 'No Error';
  }
  catch(ex)
  {
    actual = ex + '';
  }
  reportMatch(expect, actual, summary + ': function (){foo(yield 42)}');

to fail on spidermonkey trunk. I can either remove this test or change it to require no error which will show up on 1.8.1 as a known failure. 

Boyd, fyi for rhino.

Comment 11

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_7/geniter/regress-345855.js,v  <--  regress-345855.js
new revision: 1.6; previous revision: 1.5