I've addressed the "impossible cases" parts of the review, the misplacement
of
PropertyType and its related predicate will be tackled in a separate patch
for
this CL.
PTAL
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:
On 2011/11/07 14:13:39, Kevin Millikin wrote:
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.
Done.
http://codereview.chromium.org/8491016/diff/1/src/ic.cc#newcode1727
src/ic.cc:1727: case CONSTANT_FUNCTION:
On 2011/11/07 14:13:39, Kevin Millikin wrote:
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.
Done.
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) {
On 2011/11/07 14:13:39, Kevin Millikin wrote:
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.
I'm looking into this, but this is a bit unrelated to the other changes,
so I'll tackle this in another patch to the CL to make the review
easier.
http://codereview.chromium.org/8491016/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
