Closed
Bug 676936
Opened 13 years ago
Closed 13 years ago
Refactor Iterator/Generator/StopIteration initialization
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(3 files)
3.41 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
1.53 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
1.17 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #551149 -
Flags: review?(luke)
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #551150 -
Flags: review?(luke)
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #551153 -
Flags: review?(luke)
Comment 3•13 years ago
|
||
Comment on attachment 551149 [details] [diff] [review] Initial rearrangement of js_InitIterator, clarification of Iterator startup Review of attachment 551149 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsiter.cpp @@ +1453,5 @@ > + GlobalObject *global = obj->asGlobal(); > + > + /* > + * Bail if Iterator has already been initialized. We test for Iterator > + * rather than for StopIteration because if js_InitIteratorClasses recurs, s/js_//
Attachment #551149 -
Flags: review?(luke) → review+
Updated•13 years ago
|
Attachment #551150 -
Flags: review?(luke) → review+
Comment 4•13 years ago
|
||
Comment on attachment 551153 [details] [diff] [review] Simplification of StopIteration startup Review of attachment 551153 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsiter.cpp @@ +1445,4 @@ > static JSObject * > InitStopIterationClass(JSContext *cx, GlobalObject *global) > { > + JSObject *proto = global->createBlankPrototype(cx, &js_StopIterationClass); Here, I have the opposite comment as below: createBlankPrototype seems like it would be a free function, akin to NewNonFunction. @@ +1452,5 @@ > + /* This should use a non-JSProtoKey'd slot, but this is easier for now. */ > + if (!DefineConstructorAndPrototype(cx, global, JSProto_StopIteration, proto, proto)) > + return NULL; > + > + MarkStandardClassInitializedNoProto(global, &js_StopIterationClass); From IRL: some time in the future it would be nice if DefineConstructAndPrototype and MarkStandardClassInitializedNoProto were members of the GlobalObject class.
Attachment #551153 -
Flags: review?(luke) → review+
Assignee | ||
Comment 5•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/a399a694bfad http://hg.mozilla.org/integration/mozilla-inbound/rev/aade388e6c62 http://hg.mozilla.org/integration/mozilla-inbound/rev/f25d125362a8 Re s/js_//: note that that function name remains as it was, so I think that's correct for now. Re createBlankPrototype: we'll have to ask people. No one else who's reviewed patches such as these has said anything, and it's not clear to me why homing the method in GlobalObject is not right. Re IRL: yeah. There is more work to do even after the patches I have in my tree have landed, and that can be part of it.
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Comment 6•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a399a694bfad http://hg.mozilla.org/mozilla-central/rev/aade388e6c62 http://hg.mozilla.org/mozilla-central/rev/f25d125362a8
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in
before you can comment on or make changes to this bug.
Description
•