Closed Bug 676936 Opened 13 years ago Closed 13 years ago

Refactor Iterator/Generator/StopIteration initialization

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: Waldo, Assigned: Waldo)

References

Details

Attachments

(3 files)

      No description provided.
Attachment #551149 - Flags: review?(luke)
Attachment #551150 - Flags: review?(luke)
Attachment #551153 - Flags: review?(luke)
Blocks: 675520
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+
Attachment #551150 - Flags: review?(luke) → review+
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+
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: