The default bug view has changed. See this FAQ.

Function can contain both a return statement with an argument and a yield expression

RESOLVED FIXED in mozilla9

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: cdleary)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
mozilla9
x86_64
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
js> (function() { if (x) {return this;} else {(yield 3);}})
(function () {if (x) {return this;} else {yield 3;}})

js> (function () {if (x) {return this;} else {yield 3;}})
typein:3: TypeError: anonymous generator function returns a value:
typein:3: (function () {if (x) {return this;} else {yield 3;}})
typein:3: ..........................................^

I think the bug is that the first function is *not* rejected. Before rev 81c343a150a4, it was rejected.

jsfunfuzz found this because yield *statements* are rejected, and the expression happened to turn into a statement in this case.

The first bad revision is:
changeset:   81c343a150a4
user:        Dave Herman
date:        Thu Jun 16 08:20:18 2011 -0700
summary:     disallow yield and arguments in generator expressions (bug 634472, r=cdleary)
Whiteboard: js-triage-needed
This is definitely bad, and definitely my fault, and I'm going to try to fix it tonight.

But it might not be *too* bad. I believe generator functions are always run in the interpreter, and I suspect the interpreter just pushes the return value onto the stack harmlessly before throwing StopIteration. Nevertheless, this is definitely supposed to be statically rejected.

I'll report back tonight after I dig into this.

Dave
Created attachment 557435 [details] [diff] [review]
recheck TCF_RETURN_EXPR after lazily setting TCF_FUN_IS_GENERATOR

Pretty simple mistake: since we lazily set TCF_FUN_IS_GENERATOR, the eager check whenever it encounters a `yield' token is insufficient; we need a check at the point where we actually set TCF_FUN_IS_GENERATOR to see if TCF_RETURN_EXPR is set.

I also went through to see where else jsparse.cpp ever checks TCF_FUN_IS_GENERATOR, in case there might be other subtle bugs caused by the lazy checking, but all of them happen in the context of Parser::analyzeFunctions(), which gets called after the containing function body has been completely parsed (so all lazy `yield'-checking is completely finished).

The patch also includes a new regression test file.

Dave
Assignee: general → dherman
Attachment #557435 - Flags: review?(cdleary)
PS All this stuff should be simpler with the new function* syntax in ES6: the * token makes it clear up front that you're in a generator, so you don't have to infer your context lazily.

Unfortunately, we probably won't be able to remove this at least in the short term, since existing Firefox-specific code uses generators with the current syntax.

Dave

Updated

6 years ago
Summary: Function can contain both a return statement and a yield expression → Function can contain both a return statement with an argument and a yield expression
Update: I'm out for the next two weeks. Chris has graciously agreed to shepherd this bug while I'm out. If it is deemed unimportant I can take it over again when I get back, but otherwise Chris can take over the patch and land it as he sees fit.

Thanks,
Dave
Assignee: dherman → cdleary
Attachment #557435 - Flags: review?(cdleary) → review+
Whiteboard: js-triage-needed
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/01751bc07cca
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.