All comments adressed.

http://codereview.chromium.org/113524/diff/1/3
File src/frame-element.h (right):

http://codereview.chromium.org/113524/diff/1/3#newcode155
Line 155: // If the elements agree exactly aside from the Copied bit,
they are equal.
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> 'Copied' should not be capitalized.

Done.

http://codereview.chromium.org/113524/diff/1/3#newcode156
Line 156: if (!((value_ ^ other.value_) & ~CopiedField::mask()) ) return
true;
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> There's an extra space between the last two parentheses on this line.

> Don't forget to follow the style guide.  "Short conditional statements
may be
> written on one line if this enhances readability. You may use this
only when the
> line is brief and the statement does not use the else clause."

> This whole function is missing line breaks and braces.

Done.

http://codereview.chromium.org/113524/diff/1/3#newcode157
Line 157: else if (is_constant() &&
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> If xor'ing and masking is a win in the first test (saving an
instruction over
> masking, masking, and comparing) then it's probably also a win to use
it in the
> second test (especially if the common subexpression is lifted out).

> What you have might be more readable here, but I'm always suspicious
about
> if (...) {
>    return true;
> } else {
>    return false;
> }

Done.

http://codereview.chromium.org/113524/diff/1/3#newcode173
Line 173: bool not_same_location =
This test is now factored out to a separate (inline) function.

http://codereview.chromium.org/113524/diff/1/4
File src/virtual-frame.cc (right):

http://codereview.chromium.org/113524/diff/1/4#newcode271
Line 271: // We don't need to recompute exact copied flags.
On 2009/05/19 07:34:25, Kevin Millikin wrote:
> No need to comment the code that used to be here.

> Just get rid of this comment.  After this change, we *never* compute
exact
> copied flags, so there's probably no need to note that we specifically
don't do
> it here.

Done.

http://codereview.chromium.org/113524

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

Reply via email to