Closed
Bug 885463
Opened 11 years ago
Closed 11 years ago
Warn about 'yield' without operand
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(1 file)
6.67 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
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.)
Assignee | ||
Comment 1•11 years ago
|
||
Steal this bug if you have time. I'm going to try and get this into FF24 though.
Assignee: general → jorendorff
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #765663 -
Flags: review?(jwalden+bmo)
Comment 3•11 years ago
|
||
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.
Attachment #765663 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 4•11 years ago
|
||
No kidding. Filed bug 885766.
Comment 5•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 6•11 years ago
|
||
(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?
Assignee | ||
Comment 7•11 years ago
|
||
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.
Comment 8•11 years ago
|
||
Keywords: dev-doc-complete,
site-compat
You need to log in
before you can comment on or make changes to this bug.
Description
•