Last Comment Bug 885463 - Warn about 'yield' without operand
: Warn about 'yield' without operand
Status: RESOLVED FIXED
: dev-doc-complete, site-compat
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla25
Assigned To: Jason Orendorff [:jorendorff]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 891806
Blocks: 889104 889527
  Show dependency treegraph
 
Reported: 2013-06-20 12:10 PDT by Jason Orendorff [:jorendorff]
Modified: 2013-07-14 19:03 PDT (History)
4 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (6.67 KB, patch)
2013-06-20 16:10 PDT, Jason Orendorff [:jorendorff]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description Jason Orendorff [:jorendorff] 2013-06-20 12:10:20 PDT
In a generator, we parse

    yield
        f();

as two statements, meaning

    yield undefined;
    f();

But in ES6, this will parse as a single statement:

    yield f();

Users should be warned.

(We may not break their code, at least not immediately, since ES6 generators are distinguished from JS1.7+ generators by the "*". But people will naturally want to port up to ES6, and we don't want them to hit silent breakage. So a warning is in order.)
Comment 1 Jason Orendorff [:jorendorff] 2013-06-20 12:11:18 PDT
Steal this bug if you have time. I'm going to try and get this into FF24 though.
Comment 2 Jason Orendorff [:jorendorff] 2013-06-20 16:10:48 PDT
Created attachment 765663 [details] [diff] [review]
v1
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2013-06-20 18:30:15 PDT
Comment on attachment 765663 [details] [diff] [review]
v1

Review of attachment 765663 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome.  Land it!

::: js/src/jit-test/lib/asserts.js
@@ +72,5 @@
> +                print("assertWarning: " + msg);
> +            throw exc;
> +        } finally {
> +            if (!hadWerror)
> +                options("werror");

Guh, options() is not a good interface.
Comment 4 Jason Orendorff [:jorendorff] 2013-06-21 09:29:44 PDT
No kidding. Filed bug 885766.
Comment 5 Ryan VanderMeulen [:RyanVM] 2013-06-26 11:41:51 PDT
https://hg.mozilla.org/mozilla-central/rev/818298a5a183
Comment 6 Olli Pettay [:smaug] 2013-07-10 06:40:36 PDT
(Got here from Bug 891806)

Why do we need yield undefined; and not just yield; ?
I can understand 
yield 
  f(); needing some change, but will yield; mean something totally 
different than what it is currently?
Comment 7 Jason Orendorff [:jorendorff] 2013-07-10 10:59:54 PDT
Unless the proposal changes, `yield;` will be a SyntaxError. We plan to implement the standard (soon) and remove the SpiderMonkey-only extensions (eventually).

smaug, I encourage you to take this to es-discuss! If "yield undefined" looks ugly in Moz-only code, then it'll look just as ugly in Web code, which will follow the new standard syntax.

Since you have existing real-world code, you are in a position to make a uniquely convincing argument. You've seen the future, you've lived there, you're proposing a simple feature that makes the future prettier.

Individuals can make a difference on TC39. Please give it a shot if you really care about this syntax.

Note You need to log in before you can comment on or make changes to this bug.