Overall looks good, just some comments.
https://codereview.chromium.org/429053005/diff/1/src/field-index.cc
File src/field-index.cc (right):
https://codereview.chromium.org/429053005/diff/1/src/field-index.cc#newcode24
src/field-index.cc:24: FieldIndex FieldIndex::ForLookupIterator(const
LookupIterator* lookup) {
Let the LookupIterator calculate the FieldIndex instead, perhaps by
calling a lower-level interface on FieldIndex.
https://codereview.chromium.org/429053005/diff/1/src/ic.cc
File src/ic.cc (right):
https://codereview.chromium.org/429053005/diff/1/src/ic.cc#newcode208
src/ic.cc:208: if
(!holder->GetNamedInterceptor()->getter()->IsUndefined()) {
In the future we should probably get the property from the interceptor
to see if it's there. If not, we should continue the lookup behind the
interceptor and compile this into the IC as well.
Since this matches what we did before, it's fine for now.
https://codereview.chromium.org/429053005/diff/1/src/ic.cc#newcode855
src/ic.cc:855: bool receiver_is_holder = lookup->HolderIsReceiver();
These 2 things are different.
Rename HolderIsReceiver to HolderIsReceiverOrHiddenPrototype() along the
way to make it explicit?
https://codereview.chromium.org/429053005/diff/1/src/ic.cc#newcode862
src/ic.cc:862: lookup->has_fast_properties() ? Code::FAST :
Code::NORMAL);
What about just doing lookup->holder_map()->is_dictionary_map() and
removing this method again from the LookupIterator?
The difference between Code::FAST and Code::NORMAL should go away any
time soon now anyway ;)
https://codereview.chromium.org/429053005/diff/1/src/ic.cc#newcode967
src/ic.cc:967: Handle<JSObject> holder(lookup->GetHolder<JSObject>());
Handle<JSObject> holder = lookup->GetHolder<JSObject>();
https://codereview.chromium.org/429053005/diff/1/src/ic.cc#newcode977
src/ic.cc:977: switch (lookup->property_details().type()) {
What about doing the same as GetProperty; using property_kind() to
switch between callbacks and others, then using property_encoding for
dict vs field/constant; and only at the end using property_details?
I'd like to get rid of the current form of PropertyType at some point.
https://codereview.chromium.org/429053005/diff/1/src/ic.cc#newcode997
src/ic.cc:997: type, global, cell, name, !lookup->IsConfigurable());
What about changing the interface of CompileLoadGlobal? Otherwise you
negate the flag 3 times :)
!is_dont_delete -> !configurable -> !is_dont_delete
https://codereview.chromium.org/429053005/diff/1/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/429053005/diff/1/src/objects.cc#newcode15169
src/objects.cc:15169: PropertyCell*
GlobalObject::GetPropertyCell(LookupIterator* lookup) {
Let the LookupIterator return the cell instead.
https://codereview.chromium.org/429053005/
--
--
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.
