Using |yield| and |arguments| in a generator expression breaks the syntactic abstraction. I have an experimental patch (post FF4, of course) for syntactically rejecting them in the body of a generator expression.
See also: bug 632028.
Dave

Created attachment 512666[details][diff][review]
rough draft (no tests yet)
This isn't quite right for |arguments| outside of a function (it allows references but not assignments), but it's a start.
Dave

I should also mention: the quality of error reporting is less than ideal, because it reports the error only upon reaching the "for" or ")" token. And in the current patch it actually points to the last token *before* the "for" or ")" token, which is really confusing for the user.
So at the very least, it should advance the token pointer by one before reporting. But if there's some way to actually rewind to the yield/arguments token, that would be ideal. Not sure that's possible or worth the trouble.
Dave

Created attachment 522047[details][diff][review]
catches 'arguments' in top-level genexp
This now catches the case of a plain reference to 'arguments' in a generator expression at top-level (i.e., not nested inside any function). Also switched to uint32 per Brendan's suggestion. Merged the two error message types to just one type.
Still need to create test cases and improve the error reporting.
Dave

Comment on attachment 524523[details][diff][review]
ready for review
Sorry for the delay! I was finishing up the review, but I think this case is still not producing a syntax error when it should:
print((yield) for (i in [0,1,2,3]));
Makes:
main:
00001: callgname "print"
00004: lambda (function () {for (i in [1, 2, 3, 4]) {yield (yield);}})
It's tricky because a) the genexp doesn't have to be parenthesized in call position and b) the error data gets reset when the paren count dips back down to zero. Since the invoke-arguments production doesn't increase the paren count, it goes back to zero before we encounter the |for|, we reset it in the comprehensionTail production. Most of the other parens-are-implicit productions in the parser use the parenExpr production to avoid this problem.
It'd also be sweet if we could factor out the similar code (parenDepth/token-count accounting) from comprehensionTail and parenExpr into a stacky guard class on the tree context with a start/finish kind of thing.
I hope the fix is as easy as bumping the paren count in the arguments production, but I'm not totally sure it will be, so please r? me with that fixed. Thanks!

Created attachment 537712[details][diff][review]
correct even in unparenthesized sole function argument
Addresses Brendan's nits and the bug cdleary pointed out (thank you!). Abstracted out the bookkeeping in a helper class called GenexpGuard. Works in all contexts where genexps can appear, including in the unparenthesized case of a genexp as a sole function argument. Added this case to every unit test in the test file.
Brendan: the one nit I didn't address was abstracting out the == argumentsAtom check. Would you want that as an inline method in JSAtom?
Dave