Fix misleading names and comments in mute local variables debugger helper

Fix misleading names and comments in mute local variables debugger helper

R=yangguo@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15342

[Peter Rybin, 2013-06-25 14:36:36]

Hi Yang

This is a follow-up change for updating local scope algorithm. Unfortunately, I
named my variables very misleading and stealthily swapped them.

This change does not alter behavior.

Peter

[Yang, 2013-06-25 14:49:03]

On 2013/06/25 14:36:36, Peter Rybin wrote:
> Hi Yang
> 
> This is a follow-up change for updating local scope algorithm. Unfortunately,
I
> named my variables very misleading and stealthily swapped them.
> 
> This change does not alter behavior.
> 
> Peter

I'm confused.

In runtime.cc, array element 1 is set i to be local_scope_after, and array
element 2 is set to be local_scope.
In mirror-debugger, array element 1 is used as local_scope_on_stack, and
array_element 2 is used as local_scope_modified.

Wouldn't that be wrong?

[Peter Rybin, 2013-06-25 16:22:17]

On 2013/06/25 14:49:03, Yang wrote:
> On 2013/06/25 14:36:36, Peter Rybin wrote:
> > Hi Yang
> > 
> > This is a follow-up change for updating local scope algorithm.
Unfortunately,
> I
> > named my variables very misleading and stealthily swapped them.
> > 
> > This change does not alter behavior.
> > 
> > Peter
> 
> I'm confused.
> 
> In runtime.cc, array element 1 is set i to be local_scope_after, and array
> element 2 is set to be local_scope.
> In mirror-debugger, array element 1 is used as local_scope_on_stack, and
> array_element 2 is used as local_scope_modified.
> 
> Wouldn't that be wrong?

That's exactly where it's misleading. "After" doesn't mean "working values after
evaluation". "After" means "control copy of stack locals, made after
evaluation".
So I materialize stack variables before execution, then materialize them after
execution (call "control copy" now), and this way I notice that stack locals
haven't change and decide how to manually update them.

I introduced new names now.

[Yang, 2013-06-26 09:49:49]

On 2013/06/25 16:22:17, Peter Rybin wrote:
> On 2013/06/25 14:49:03, Yang wrote:
> > On 2013/06/25 14:36:36, Peter Rybin wrote:
> > > Hi Yang
> > > 
> > > This is a follow-up change for updating local scope algorithm.
> Unfortunately,
> > I
> > > named my variables very misleading and stealthily swapped them.
> > > 
> > > This change does not alter behavior.
> > > 
> > > Peter
> > 
> > I'm confused.
> > 
> > In runtime.cc, array element 1 is set i to be local_scope_after, and array
> > element 2 is set to be local_scope.
> > In mirror-debugger, array element 1 is used as local_scope_on_stack, and
> > array_element 2 is used as local_scope_modified.
> > 
> > Wouldn't that be wrong?
> 
> That's exactly where it's misleading. "After" doesn't mean "working values
after
> evaluation". "After" means "control copy of stack locals, made after
> evaluation".
> So I materialize stack variables before execution, then materialize them after
> execution (call "control copy" now), and this way I notice that stack locals
> haven't change and decide how to manually update them.
> 
> I introduced new names now.

I see now. I think I misunderstood the previous CL then.

How about not using MaterializeLocalScopeWithFrameInspector re-materialize a
local scope object for the control copy, but use Copy (handles.cc) instead?

Other than that, LGTM!

[Peter Rybin, 2013-06-26 15:21:20]

> How about not using MaterializeLocalScopeWithFrameInspector re-materialize a
> local scope object for the control copy, but use Copy (handles.cc) instead?
> 
> Other than that, LGTM!

You choose. The way it is done now is unexpectedly more natural: we probe actual
state of locals immediately before our compare-and-update routine. This is more
accurate and allows for magically changed locals :)

[Yang, 2013-06-26 15:24:21]

On 2013/06/26 15:21:20, Peter Rybin wrote:
> > How about not using MaterializeLocalScopeWithFrameInspector re-materialize a
> > local scope object for the control copy, but use Copy (handles.cc) instead?
> > 
> > Other than that, LGTM!
> 
> You choose. The way it is done now is unexpectedly more natural: we probe
actual
> state of locals immediately before our compare-and-update routine. This is
more
> accurate and allows for magically changed locals :)

Hm. I guess you are right. I was thinking using Copy should give the same
results, but is cheaper than MaterializeLocalScopeWithFrameInspector. Then again
we don't care about performance in this case anyways.

[Peter Rybin, 2013-06-26 16:18:09]

Committed patchset #3 manually as r15342 (presubmit successful).