Allow yield expressions without a RHS.

Allow yield expressions without a RHS.

R=marja@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=22163

[marja, 2014-06-26 10:41:52]

Could you add test-parsing tests for all kinds of legal and illegal cases?
(That's the most streamlined way of testing illegal syntax, and testing
PreParser.)

test-parsing.cc and mjsunit have some minor amount of duplication... If you're
just testing that stuff is getting parsed without an error, I'd say test-parsing
is the way to go. In the mjsunit tests, it would be good to assert that the
right thing happens when the code is executed (something we cannot do in
test-parsing.cc). Wdyt?

Could you also add a test where a comment (single or multiline) starts on the
same line as yield?

https://codereview.chromium.org/348893007/diff/1/test/mjsunit/harmony/generat...
File test/mjsunit/harmony/generators-parsing.js (right):

https://codereview.chromium.org/348893007/diff/1/test/mjsunit/harmony/generat...
test/mjsunit/harmony/generators-parsing.js:39: function* g() { yield; }
Could you add a test that calls this kind of function (which has yield without a
value) and assert that the right thing is yielded? (What is the right thing in
this case?)

[wingo, 2014-06-26 11:04:19]

On 2014/06/26 10:41:52, marja wrote:
> Could you add test-parsing tests for all kinds of legal and illegal cases?
> (That's the most streamlined way of testing illegal syntax, and testing
> PreParser.)

To clarify, are you suggesting that I move the entirety of generators-parsing.js
to test-parsing?  With the exception of the newly added case on line 63, none of
these generators are actually executed; this file is just for parsing tests.

> Could you also add a test where a comment (single or multiline) starts on the
> same line as yield?

Sure.

[wingo, 2014-06-26 11:04:27]

https://codereview.chromium.org/348893007/diff/1/test/mjsunit/harmony/generat...
File test/mjsunit/harmony/generators-parsing.js (right):

https://codereview.chromium.org/348893007/diff/1/test/mjsunit/harmony/generat...
test/mjsunit/harmony/generators-parsing.js:39: function* g() { yield; }
On 2014/06/26 10:41:52, marja wrote:
> Could you add a test that calls this kind of function (which has yield without
a
> value) and assert that the right thing is yielded? (What is the right thing in
> this case?)

You can see an example of this on line 63.  (The right thing is
{value:undefined, done:false}.)

[marja, 2014-06-27 09:51:56]

On 2014/06/27 09:51:19, marja wrote:

Hey, not fun! I wrote a reply but codereview eated it. I will write it again.

[marja, 2014-06-27 09:55:36]

The reply was:

I don't have super strong opinions on this. We'd need to find a good balance wrt
which tests to put to mjsunit and which to test-parsing.cc.

mjsunit:
Pros: can run code, easy to understand, self contained
Cons: doesn't invoke PreParser at all, testing erroneous constructs is
cumbersome

test-parsing.cc:
Pros: can easily test erroneous constructs, can test combinations easily (like,
test all combinations of function signature and function body), can test
PreParser (and that Parser and PreParser give the same errors), can test the
same code with all combinations of flags
Cons: not mjsunit

So I was suggesting that maybe some of the tests you have for yielding are a
good fit for test-parsing.cc. Wdyt?

[wingo, 2014-06-27 10:06:31]

On 2014/06/27 09:55:36, marja wrote:
> The reply was:
> 
> I don't have super strong opinions on this. We'd need to find a good balance
wrt
> which tests to put to mjsunit and which to test-parsing.cc.
> 
> mjsunit:
> Pros: can run code, easy to understand, self contained
> Cons: doesn't invoke PreParser at all, testing erroneous constructs is
> cumbersome
> 
> test-parsing.cc:
> Pros: can easily test erroneous constructs, can test combinations easily
(like,
> test all combinations of function signature and function body), can test
> PreParser (and that Parser and PreParser give the same errors), can test the
> same code with all combinations of flags
> Cons: not mjsunit
> 
> So I was suggesting that maybe some of the tests you have for yielding are a
> good fit for test-parsing.cc. Wdyt?

Cool, sounds good to me.  I'll just copy most of them over I guess -- I suppose
it can't hurt to test something twice.  Tx for the feedback :)

[wingo, 2014-07-02 12:53:29]

I believe I've addressed the feedback about tests; PTAL when you can.  Happily
adding more tests fixed a bug in the first patch which we were parsing
function*() { yield* } as function*() { yield* undefined }.

[wingo, 2014-07-02 13:00:16]

Forgot to mention that I rebased the patch :/, in order to have the test-parsing
changes that just landed.  Sorry about that; I guess just look at the overall
diffs and not the interdiffs.

[marja, 2014-07-02 13:04:06]

lgtm

(No problem with the patches, I don't look at the interdiffs anyway. And this
was really small. To make it easier for people who *do* look at them, you could
upload the rebase as a separate patch set.)

[wingo, 2014-07-02 13:48:39]

Committed patchset #2 manually as r22163 (presubmit successful).