So much better! I have a few naming quibbles, though.
https://codereview.chromium.org/314953006/diff/180001/src/lookup.cc
File src/lookup.cc (right):
https://codereview.chromium.org/314953006/diff/180001/src/lookup.cc#newcode72
src/lookup.cc:72: state_ = NOT_FOUND;
Might be clearer to not pre-default this. There are only two places it
is used anyway.
https://codereview.chromium.org/314953006/diff/180001/src/lookup.cc#newcode82
src/lookup.cc:82: }
Put a fall-through comment here and below.
https://codereview.chromium.org/314953006/diff/180001/src/lookup.h
File src/lookup.h (right):
https://codereview.chromium.org/314953006/diff/180001/src/lookup.h#newcode17
src/lookup.h:17: enum Type {
Can we not call this Type?
https://codereview.chromium.org/314953006/diff/180001/src/lookup.h#newcode34
src/lookup.h:34: enum PropertyType {
PropertyKind, perhaps?
https://codereview.chromium.org/314953006/diff/180001/src/lookup.h#newcode36
src/lookup.h:36: ACCESSORS
Spec says "accessor property", so I'd drop the plural.
https://codereview.chromium.org/314953006/diff/180001/src/lookup.h#newcode96
src/lookup.h:96: Handle<JSReceiver> GetOrigin() const;
Hm, origin is a bit confusing as well, as it already has a specific
meaning in the JS/web context. How about naming this GetInitial?
https://codereview.chromium.org/314953006/diff/180001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/314953006/diff/180001/src/objects-inl.h#newcode668
src/objects-inl.h:668: return
HeapObject::cast(this)->map()->IsJSProxyMap();
Why not make the same change to IsJSObject? (And perhaps IsJSReceiver.)
https://codereview.chromium.org/314953006/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
