Fix DebugEvaluate for generators.

Fix DebugEvaluate for generators.


R=mstarzinger@chromium.org
BUG=v8:3225
LOG=N

Committed: https://code.google.com/p/v8/source/detail?r=20195

[Michael Starzinger, 2014-03-24 10:24:28]

https://codereview.chromium.org/207153004/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/207153004/diff/1/src/runtime.cc#newcode11427
src/runtime.cc:11427: if (scope_info->ContextSlotIndex(*name, &mode, &init_flag)
!= -1) {
I am little bit confused here. For generators we should already context-allocate
all parameter slots, which in turn means this check should hit for all of them.
So I don't see how the assert below would ever hit unless the scoping
information is off.

https://codereview.chromium.org/207153004/diff/1/src/runtime.cc#newcode11479
src/runtime.cc:11479: ASSERT(!frame->GetParameter(i)->IsTheHole());
Likewise, instead of baking in arbitrary special handling of generators here, I
would argue that we should instead use the provided scoping information and just
skip shadowed parameters in this loop.

[Yang, 2014-03-24 10:40:31]

On 2014/03/24 10:24:28, Michael Starzinger wrote:
> https://codereview.chromium.org/207153004/diff/1/src/runtime.cc
> File src/runtime.cc (right):
> 
> https://codereview.chromium.org/207153004/diff/1/src/runtime.cc#newcode11427
> src/runtime.cc:11427: if (scope_info->ContextSlotIndex(*name, &mode,
&init_flag)
> != -1) {
> I am little bit confused here. For generators we should already
context-allocate
> all parameter slots, which in turn means this check should hit for all of
them.
> So I don't see how the assert below would ever hit unless the scoping
> information is off.
> 
> https://codereview.chromium.org/207153004/diff/1/src/runtime.cc#newcode11479
> src/runtime.cc:11479: ASSERT(!frame->GetParameter(i)->IsTheHole());
> Likewise, instead of baking in arbitrary special handling of generators here,
I
> would argue that we should instead use the provided scoping information and
just
> skip shadowed parameters in this loop.

updated

[Michael Starzinger, 2014-03-24 10:57:07]

LGTM with a nit.

https://codereview.chromium.org/207153004/diff/30001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/207153004/diff/30001/src/runtime.cc#newcode11443
src/runtime.cc:11443: Runtime::SetObjectProperty(
nit: Should fit into one line again now.

[Yang, 2014-03-24 14:11:03]

Committed patchset #4 manually as r20195 (presubmit successful).