LGTM.
http://codereview.chromium.org/6625048/diff/1/src/arm/codegen-arm.cc File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/6625048/diff/1/src/arm/codegen-arm.cc#newcode620 src/arm/codegen-arm.cc:620: ASSERT(shadow != NULL && shadow->AsSlot() != NULL || Parens around the && please. http://codereview.chromium.org/6625048/diff/1/src/arm/full-codegen-arm.cc File src/arm/full-codegen-arm.cc (right): http://codereview.chromium.org/6625048/diff/1/src/arm/full-codegen-arm.cc#newcode217 src/arm/full-codegen-arm.cc:217: if (arguments_shadow != NULL) { A bit simpler (also on the other platforms): if (arguments_shadow != NULL) { // Duplicate the value; move-to-slot operation might clobber registers. __ mov(r3, r0); Move(arguments_shadow->AsSlot(), r3, r1, r2); } Move(arguments->AsSlot(), r0, r1, r2); http://codereview.chromium.org/6625048/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/6625048/diff/1/src/hydrogen.cc#newcode2238 src/hydrogen.cc:2238: scope->arguments_shadow() != NULL && Parens around the && please. http://codereview.chromium.org/6625048/diff/1/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/6625048/diff/1/src/parser.cc#newcode656 src/parser.cc:656: TemporaryScope temp_scope(&this->temp_scope_); Hmmm. TemporaryScope seems like a bad name---it's not really a "scope" at all. It seems like a bundle of function-specific bits (literals and a loop flag). We already have a stack-allocated "LexicalScope" everywhere we have a TemporaryScope, so we should probably just combine them. Maybe roll it into the next change you make to the parser. http://codereview.chromium.org/6625048/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
