I don't immediately see why some of the types are unreachable in
KeyedStoreIC::UpdateCaches.


http://codereview.chromium.org/8491016/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/8491016/diff/1/src/ic.cc#newcode1396
src/ic.cc:1396: case HANDLER:
Is HANDLER unreachable because the caller takes care of proxies?  In
that case, then !receiver->IsJSProxy() is also one of the preconditions
and should be asserted on entry to this function.

I find it much more clear to get a precondition assertion violation on
entry rather than as a switch match failure in the middle of the
function.

http://codereview.chromium.org/8491016/diff/1/src/ic.cc#newcode1727
src/ic.cc:1727: case CONSTANT_FUNCTION:
I guess we should not get NULL_DESCRIPTOR here, and HANDLER is covered
because the caller ensures it (?).

I don't immediately see why CONSTANT_FUNCTION, INTERCEPTOR, and
ELEMENTS_TRANSITION is impossible.  It definitely needs a short comment.

http://codereview.chromium.org/8491016/diff/1/src/v8globals.h
File src/v8globals.h (right):

http://codereview.chromium.org/8491016/diff/1/src/v8globals.h#newcode351
src/v8globals.h:351: inline bool IsTransitionType(PropertyType type) {
I'm a bit nervous about this.  I honestly don't know if compilers do a
good job of inlining the switch.  I wonder if it's best just to put this
in property.cc (and then you don't need checks.h in this header)?

I'm also nervous about the near-circularity of putting checks.h into
v8globals.h.  v8.h contains the headers that need a fixed include order
for one reason or another, and it has v8globals.h followed by v8checks.h
(which includes checks.h).

So, not exactly a circularity but already getting tangled.

http://codereview.chromium.org/8491016/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to