LGTM. On Wed, Oct 28, 2009 at 6:43 PM, <[email protected]> wrote:
> > http://codereview.chromium.org/347001/diff/1/3 > File src/compiler.cc (right): > > http://codereview.chromium.org/347001/diff/1/3#newcode671 > Line 671: // global variables and and properties. > On 2009/10/28 14:17:11, Kevin Millikin wrote: > >> "We support plain non-compound assignments to properties, parameters >> > and > >> non-context (stack-allocated) locals, and global variables." >> > > Otherwise it sounds like they might be only global properties. >> > > Done. > > > http://codereview.chromium.org/347001/diff/1/3#newcode685 > Line 685: Visit(expr->target()); > On 2009/10/28 14:17:11, Kevin Millikin wrote: > >> Hmmm. Here it's probably OK to visit the property as if it were an >> > rvalue, but > >> I think we should avoid it. Imagine if the selector had to do >> > something > >> different for lvalue and rvalue properties.... >> > > Better: >> > > if (var == NULL) { >> Property* prop = expr->target()->AsProperty(); >> if (prop == NULL) BAILOUT(...); >> Visit(prop->obj()); >> Visit(prop->key()); >> } else ... >> > > Done. Also I noticed that I don't need an else{} if we bailout on the > then-part :) > > > http://codereview.chromium.org/347001/diff/1/3#newcode688 > Line 688: ASSERT(var->slot() != NULL); > On 2009/10/28 14:17:11, Kevin Millikin wrote: > >> It occurs to me that we could hit the var->slot() == NULL case. (Due >> > to a > >> left-hand side that is not a property but is a proxy that rewrites to >> > a > >> property.) We should fix that by adding a bailout for var->slot() == >> > NULL. > > Done. > > > http://codereview.chromium.org/347001/diff/1/2 > File src/ia32/fast-codegen-ia32.cc (right): > > http://codereview.chromium.org/347001/diff/1/2#newcode470 > Line 470: __ add(Operand(esp), Immediate(kPointerSize)); > On 2009/10/28 14:17:11, Kevin Millikin wrote: > >> Indentation. >> > > Done. > > > http://codereview.chromium.org/347001/diff/1/4 > File src/x64/fast-codegen-x64.cc (right): > > http://codereview.chromium.org/347001/diff/1/4#newcode482 > Line 482: __ addq(rsp, Immediate(kPointerSize)); > On 2009/10/28 14:17:11, Kevin Millikin wrote: > >> Indentation. >> > > Done. > > > http://codereview.chromium.org/347001 > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
