Closed Bug 1384299 Opened 3 years ago Closed 3 years ago

Error messages for using 'yield' outside generators is confusing

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jimb, Assigned: arai)

Details

Attachments

(1 file)

This behavior was surprising to me:

    js> async function f() { yield 1; }            
    typein:4:27 SyntaxError: missing ; before statement:
    typein:4:27 async function f() { yield 1; }
    typein:4:27 ...........................^
    js> async function f() { yield ; }  
    js> 

I had to have it explained to me that yield is not being interpreted as a keyword in this context, so the second definition is, essentially:

    async function f() { global; }

It would be nice if SpiderMonkey could produce a better error message, as we do for the await keyword: https://dxr.mozilla.org/mozilla-central/rev/7d2e89fb92331d7e4296391213c1e63db628e046/js/src/frontend/Parser.cpp#2928-2931
To avoid red herrings: the 'async' keyword in the example above is extraneous:

js> function f() { yield 1; }
typein:9:21 SyntaxError: missing ; before statement:
typein:9:21 function f() { yield 1; }
typein:9:21 .....................^
js>
Comment on attachment 8890114 [details] [diff] [review]
Provide better error message when errornous syntax possibly match "yield SOMETHING" outside generators.

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

Thank you, much better. r=me with or without the change I ask about below applied.

::: js/src/js.msg
@@ +352,5 @@
>  MSG_DEF(JSMSG_WHILE_AFTER_DO,          0, JSEXN_SYNTAXERR, "missing while after do-loop body")
>  MSG_DEF(JSMSG_YIELD_IN_ARROW,          0, JSEXN_SYNTAXERR, "arrow function may not contain yield")
>  MSG_DEF(JSMSG_YIELD_IN_DEFAULT,        0, JSEXN_SYNTAXERR, "yield in default expression")
>  MSG_DEF(JSMSG_YIELD_IN_METHOD,         0, JSEXN_SYNTAXERR, "non-generator method definitions may not contain yield")
> +MSG_DEF(JSMSG_YIELD_OUTSIDE_GENERATOR, 0, JSEXN_SYNTAXERR, "yield expression is only valid in generators")

Would it make sense to use this message for the non-generator method case right above it, too?
Attachment #8890114 - Flags: review?(till) → review+
thank you for reviewing :)

(In reply to Till Schneidereit [:till] from comment #4)
> ::: js/src/js.msg
> @@ +352,5 @@
> >  MSG_DEF(JSMSG_WHILE_AFTER_DO,          0, JSEXN_SYNTAXERR, "missing while after do-loop body")
> >  MSG_DEF(JSMSG_YIELD_IN_ARROW,          0, JSEXN_SYNTAXERR, "arrow function may not contain yield")
> >  MSG_DEF(JSMSG_YIELD_IN_DEFAULT,        0, JSEXN_SYNTAXERR, "yield in default expression")
> >  MSG_DEF(JSMSG_YIELD_IN_METHOD,         0, JSEXN_SYNTAXERR, "non-generator method definitions may not contain yield")
> > +MSG_DEF(JSMSG_YIELD_OUTSIDE_GENERATOR, 0, JSEXN_SYNTAXERR, "yield expression is only valid in generators")
> 
> Would it make sense to use this message for the non-generator method case
> right above it, too?

I'll leave it to other bug, since there are also JSMSG_YIELD_IN_ARROW etc, and it might be nice to align them.
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc186bad8bd537919a15722a1a79dd46660f95f1
Bug 1384299 - Provide better error message when errornous syntax possibly match "yield SOMETHING" outside generators. r=till
https://hg.mozilla.org/mozilla-central/rev/dc186bad8bd5
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.