LGTM.

http://codereview.chromium.org/21509/diff/1/2
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/21509/diff/1/2#newcode1334
Line 1334: (left_side.is_constant() && left_side.handle()->IsNull() ||
This condition should be aligned with the previous one, cc == equal.

http://codereview.chromium.org/21509/diff/1/2#newcode1341
Line 1341: (left_side.is_constant() && left_side.handle()->IsNull()) ?
I don't like breaking the ternary operator like this.  I'd rather see
the ? and : at the beginning of the following line.

You might consider just checking up front if left and right are null, at
the same time you check for smi constants, ie:

bool left_is_constant_smi = false;
bool left_is_constant_null = false;
if (left_side.is_constant()) {
   left_is_constant_smi = left_side.handle()->IsSmi();
   left_is_constant_null = left_side.handle()->IsNull();
}

Then eliminating the repeated && expression.

http://codereview.chromium.org/21509

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

Reply via email to