Closed Bug 641436 Opened 9 years ago Closed 9 years ago

yield in new

Categories

(Core :: JavaScript Engine, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dmitry.soshnikov, Assigned: luke)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.15) Gecko/20110303 Firefox/3.6.15
Build Identifier: SpiderMonkey 1.8.5

An empty `yield` in a generator's body created via the `new` call results an object instead of `undefined`.

// infinite objects generator
let g = new function () {
   this.x = 10;
   while (true) {
     yield;
   }
};

// generate an array of 3 objects
let objects = [1, 2, 3].map(function(i) g.next());

console.dir(objects);

Results:

[
     [[Class]]: "Array",
     length: 3,
     0: {
         [[Class]]: "Object",
         x: 10
     },
     1: {
         [[Class]]: "Object"
     },
     2: {
         [[Class]]: "Object"
     }
]

Only first object has `x` property. Also:

console.log(objects[0] == objects[1]); // false
console.log(objects[1] == objects[2]); // false

See also details: https://mail.mozilla.org/pipermail/es-discuss/2011-March/013049.html

Reproducible: Always
Status: UNCONFIRMED → NEW
Ever confirmed: true
Ah, ScriptPrologue/Epilogue is called for each activation/deactivation of the generator.  This gives it a fresh 'this' on each activation and converts the 'undefined' "return" value to 'this' on each yield.

So the simple fix would be to only call ScriptPrologue/Epilogue when the generator is first entered/closed.  This means that debug hooks for the generator will not be called in a LIFO order which may or may not break assumptions of jsd/Firebug.  Steve, JJB, timeless: any of you know if this is bad?

If it does matter, we can hack it so that debug hooks are called LIFO, but it would less hacky to not do this.
Assignee: general → luke
Firebug can't debug generators currently. I'll have to investigate why that is before looking at this issue.
yeah, i have no idea how generators interact w/ the debugger, they were added after i did my deepest jsd dive.
(In reply to comment #1)
> Ah, ScriptPrologue/Epilogue is called for each activation/deactivation of the
> generator.  This gives it a fresh 'this' on each activation and converts the
> 'undefined' "return" value to 'this' on each yield.
> 
> So the simple fix would be to only call ScriptPrologue/Epilogue when the
> generator is first entered/closed.  This means that debug hooks for the
> generator will not be called in a LIFO order which may or may not break
> assumptions of jsd/Firebug.  Steve, JJB, timeless: any of you know if this is
> bad?
Because of bug 641742 we have no assumptions in our code.

I personally think the example is a language bug or type error. The anonymous function following 'new' is an implicit constructor of a |Generator|. Consequently the code should be equivalent to
var generator = new Generator(function () {
   this.x = 10;
   while (true) {
     yield;
   }
}); // under the covers
let g = new generator;  // TypeError: generator is not a constructor

Therefore it doesn't make sense to ponder the debugger isues, we should not get that far.
Depends on: 641742
(In reply to comment #4)
> Therefore it doesn't make sense to ponder the debugger isues, we should not get
> that far.

My question had to do with generators in general and their non-LIFO activation records.
Ah, ok, then I'll need a bit of translation. I don't know your terminology.

As far as the developer is concerned the constructor call is 'off the books'. So I guess you are talking about the 'next()' calls.

The debugger will want the next() call to act like a function call / return, where the first line of execution in the function is below the previous yield.  For example, to support step-into, we need the next() call to fire jsdICallHook.TYPE_FUNCTION_CALL and jsdICallHook.TYPE_FUNCTION_RETURN on yield. I suppose we could invent two new constants.

I hope that is related to your question.
That does answer my question, thanks.  I'll make a patch on top of bug 636296, since it cleans this whole area up and should make the patch simple.
Depends on: 636296
(In reply to comment #6)
> As far as the developer is concerned the constructor call is 'off the books'.

There might be value in blocking on the opening brace (if that token can be picked from the debugger's UI) to catch the constructor when it makes a fresh generator iterator.

> So I guess you are talking about the 'next()' calls.

Definitely these are the juicy debuggables.

> The debugger will want the next() call to act like a function call / return,
> where the first line of execution in the function is below the previous yield. 
> For example, to support step-into, we need the next() call to fire
> jsdICallHook.TYPE_FUNCTION_CALL and jsdICallHook.TYPE_FUNCTION_RETURN on yield.
> I suppose we could invent two new constants.

+1 on new constants, yield is not return and .next is not exactly a call either.

/be
Attached patch fix and testSplinter Review
This patch keeps calling debug hooks when a generator resumes/yields but only calls the prologue/epilogue when the generator first begins/finally returns.  The test for comment 0 is included (modified) and another unrelated test that makes sure generators get put.
Attachment #521433 - Flags: review?(dvander)
Attachment #521433 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/e6c5a67da7ae
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/e6c5a67da7ae
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.