The style is fishy. It will look good if you fix the issues noted below.
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. 'Copied' should not be capitalized. http://codereview.chromium.org/113524/diff/1/3#newcode156 Line 156: if (!((value_ ^ other.value_) & ~CopiedField::mask()) ) return true; 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. http://codereview.chromium.org/113524/diff/1/3#newcode157 Line 157: else if (is_constant() && 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; } 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. 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. http://codereview.chromium.org/113524 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
