Great in theory, but some bugs in the execution ;)
https://codereview.chromium.org/233293005/diff/100001/src/hydrogen.cc
File src/hydrogen.cc (left):
https://codereview.chromium.org/233293005/diff/100001/src/hydrogen.cc#oldcode7663
src/hydrogen.cc:7663: if (receiver_map->instance_type() !=
JS_ARRAY_TYPE) return false;
This is invalid, since .push is spec'ed as reading the length, writing
the element, updating the length.
1) regular objects don't have a HObjectAccess::ForArrayLength(...) and
2) the length may be observable through accessors.
Additionally, I presume UncheckedMono... doesn't actually do anything to
a "length" field for non-JSArrays, so the fact that the length is
updated by .push is relevant.
Please add tests to this effect.
https://codereview.chromium.org/233293005/diff/100001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/233293005/diff/100001/src/hydrogen.cc#newcode7666
src/hydrogen.cc:7666: if (!receiver_map->is_extensible()) return false;
I think is_extensible isn't necessary cause we guarantee we have
dictionary elements for those cases. We should definitely ASSERT though.
Both these conditions should also hold for ArrayPop. In fact, they are
supposed to hold for all BuildUncheckMonomorphicElementAccess users, so
we should ASSERT them in that method.
https://codereview.chromium.org/233293005/
--
--
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.
