http://codereview.chromium.org/57052/diff/2001/3005
File src/jump-target.cc (right):

http://codereview.chromium.org/57052/diff/2001/3005#newcode286
Line 286: current.set_static_type(StaticType::unknown());
Changed "current" to be a pointer to the element in the frame, instead
of a copy.

http://codereview.chromium.org/57052/diff/2001/3005#newcode295
Line 295: current.set_static_type(merged_type);
And ditto.

http://codereview.chromium.org/57052/diff/2001/3010
File src/register-allocator.h (right):

http://codereview.chromium.org/57052/diff/2001/3010#newcode58
Line 58: bool is_jsstring() { return static_type_ == STRING_TYPE; }
So do I, but then the static factory method should be called string(),
and that makes the lint check barf because I use "string" without
including "string.h".
Maybe I should capitalize the factory methods.

http://codereview.chromium.org/57052/diff/2001/3010#newcode63
Line 63: // if (object->IsSmi()) return smi();                 // Unused
for now.
On 2009/04/03 09:49:22, Kevin Millikin wrote:
> Go ahead and put the commented code in?  In any case, this probably
deserves a
> comment reminding that the order of comparisons is important (most
specific
> first).

Done.

http://codereview.chromium.org/57052/diff/2001/3018
File src/virtual-frame.cc (right):

http://codereview.chromium.org/57052/diff/2001/3018#newcode446
Line 446: StaticType merge_type =
static_type.merge(element.static_type());
I'll assert that there can't. If it turns out we are wrong, we'll have
to figure out whether the bug is in the assert or in the compiler.

http://codereview.chromium.org/57052/diff/2001/3019
File src/virtual-frame.h (right):

http://codereview.chromium.org/57052/diff/2001/3019#newcode47
Line 47: class FrameElement  {
Indeed it does.

http://codereview.chromium.org/57052/diff/2001/3019#newcode76
Line 76: return FrameElement(REGISTER, reg, is_synced != NOT_SYNCED,
static_type);
On 2009/04/03 09:49:22, Kevin Millikin wrote:
> ... so could you move the conversion from enum to bool all the way
down into the
> private constructor?

Done.

http://codereview.chromium.org/57052

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

Reply via email to