https://codereview.chromium.org/236143002/diff/1/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/236143002/diff/1/src/objects.cc#newcode16090
src/objects.cc:16090: return factory->CreateIteratorResultObject(value,
false);
On 2014/04/14 19:51:49, arv wrote:
On 2014/04/14 19:15:42, adamk wrote:
> I notice you pass false here unconditionally, even if the table is
already
> Closed() at this point. Is that per spec, or is it up to the
implementation?
>
> Also makes me wonder, what if the forEach function adds an entry to
the table
> during its final callback? It looks like that might cause the
forEach to
> terminate without hitting that last entry, and yet I think the spec
would say
> that the newly-added entry should also be iterated over...
I think you are right. I'll review the spec and add tests.
Done.
https://codereview.chromium.org/236143002/diff/1/src/objects.cc#newcode16121
src/objects.cc:16121: Handle<FixedArray> array =
factory->NewFixedArray(2);
On 2014/04/14 19:15:42, adamk wrote:
This method should be static since it does allocation.
Done.
https://codereview.chromium.org/236143002/diff/1/src/objects.cc#newcode16160
src/objects.cc:16160: return factory->NewJSArrayWithElements(array);
On 2014/04/14 19:15:42, adamk wrote:
Same as above, should be static.
Done.
https://codereview.chromium.org/236143002/diff/60001/src/factory.cc
File src/factory.cc (right):
https://codereview.chromium.org/236143002/diff/60001/src/factory.cc#newcode2003
src/factory.cc:2003: JSObject::SetProperty(result, value_string(),
value, NONE, SLOPPY).Assert();
On 2014/04/14 21:33:01, adamk wrote:
I think you can skip all the JSObject machinery here and set these
properties
directly into the object. I believe the syntax is:
result->InObjectPropertyAtPut(
JSGeneratorObject::kResultValuePropertyIndex, value);
result->InObjectPropertyAtPut(
JSGeneratorObject::kResultDonePropertyIndex, ToBoolean(done));
Seems like we might want to move those constants out of
JSGeneratorObject and
into another class...not sure what the v8 folks would want.
Done.
https://codereview.chromium.org/236143002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
