LGTM. Added Toon just FYI about the change in the objects.cc file. And
Kudos for
tackling all of the conformance issues in the Array builtins, you are my
hero.
:)
https://codereview.chromium.org/726773002/diff/1/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/726773002/diff/1/src/objects.cc#newcode13100
src/objects.cc:13100: bool JSArray::HasReadOnlyLength(Handle<JSArray>
array) {
On 2014/11/13 23:43:29, adamk wrote:
Should this just delegate to IsReadOnlyLengthDescriptor above? It's
not clear to
me why the code in the two methods looks different...
Acknowledged. I chatted with Toon about this, semantically these two
methods should do the same. But since the LookupIterator is the "better
implementation", we should keep this method and instead deprecate the
other one. So I would just keep it as it is for now (i.e. keep both
methods) and maybe even add a TODO to IsReadOnlyLengthDescriptor that it
is deprecated.
https://codereview.chromium.org/726773002/diff/1/test/mjsunit/array-methods-read-only-length.js
File test/mjsunit/array-methods-read-only-length.js (right):
https://codereview.chromium.org/726773002/diff/1/test/mjsunit/array-methods-read-only-length.js#newcode154
test/mjsunit/array-methods-read-only-length.js:154: assertFalse(2 in b);
On 2014/11/13 23:43:29, adamk wrote:
These have since been fixed.
Acknowledged. Awesome.
https://codereview.chromium.org/726773002/
--
--
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.
