Reviewers: Kasper Lund, Mads Ager, iposva, Message: This is a fix for issue 220. Discarding the value of SetValue on a reference and discarding the reference itself (which preserves top-of-stack) only commute when the reference is zero-sized.
We should be able to construct a regression test for this, but it's tricky. We should also double check that our behavior in this case is what we want---it is only triggered by constant declarations and function declarations in statement position, neither of which is ECMA-262. Description: Fix for off-by-one when initializing a constant or function declaration that was not a slot. Please review this at http://codereview.chromium.org/19745 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/codegen-arm.cc M src/codegen-ia32.cc Index: src/codegen-arm.cc =================================================================== --- src/codegen-arm.cc (revision 1198) +++ src/codegen-arm.cc (working copy) @@ -1144,15 +1144,13 @@ } if (val != NULL) { - // Set initial value. - Reference target(this, node->proxy()); - ASSERT(target.is_slot()); - Load(val); - target.SetValue(NOT_CONST_INIT); - // Get rid of the assigned value (declarations are statements). It's - // safe to pop the value lying on top of the reference before unloading - // the reference itself (which preserves the top of stack) because we - // know it is a zero-sized reference. + { + // Set initial value. + Reference target(this, node->proxy()); + Load(val); + target.SetValue(NOT_CONST_INIT); + } + // Get rid of the assigned value (declarations are statements). frame_->Pop(); } } Index: src/codegen-ia32.cc =================================================================== --- src/codegen-ia32.cc (revision 1198) +++ src/codegen-ia32.cc (working copy) @@ -1431,15 +1431,13 @@ } if (val != NULL) { - // Set initial value. - Reference target(this, node->proxy()); - ASSERT(target.is_slot()); - Load(val); - target.SetValue(NOT_CONST_INIT); - // Get rid of the assigned value (declarations are statements). It's - // safe to pop the value lying on top of the reference before unloading - // the reference itself (which preserves the top of stack) because we - // know that it is a zero-sized reference. + { + // Set initial value. + Reference target(this, node->proxy()); + Load(val); + target.SetValue(NOT_CONST_INIT); + } + // Get rid of the assigned value (declarations are statements). frame_->Pop(); } } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
