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() ||
On 2009/02/19 13:00:45, Kevin Millikin wrote:
> This condition should be aligned with the previous one, cc == equal.

Done.

http://codereview.chromium.org/21509/diff/1/2#newcode1341
Line 1341: (left_side.is_constant() && left_side.handle()->IsNull()) ?
On 2009/02/19 13:00:45, Kevin Millikin wrote:
> 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.

Done.

http://codereview.chromium.org/21509

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

Reply via email to