[interpreter] Fix debug stepping for generators.

[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yangguo@chromium.org
BUG=v8:4907
Committed: https://crrev.com/6e700b7f769e9f15a5193f9efcf43aa07fc3054d
Cr-Commit-Position: refs/heads/master@{#36896}

[neis, 2016-06-09 10:37:04]

Description was changed from

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yanguo@chromium.org
BUG=v8:4907
==========

to

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yanguo@chromium.org
BUG=v8:4907
==========

[neis, 2016-06-09 10:37:05]

neis@chromium.org changed reviewers:
+ adamk@chromium.org

[neis, 2016-06-09 10:37:26]

adamk: please take a look at the parser changes

[neis, 2016-06-09 10:37:49]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-09 10:38:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2051783002/1

[rmcilroy, 2016-06-09 10:50:18]

Interpreter changes LGTM, one suggestion on the tests.

https://codereview.chromium.org/2051783002/diff/1/test/mjsunit/es6/debug-step...
File test/mjsunit/es6/debug-stepin-generators.js (right):

https://codereview.chromium.org/2051783002/diff/1/test/mjsunit/es6/debug-step...
test/mjsunit/es6/debug-stepin-generators.js:5: // Flags: --expose-debug-as debug
--ignition-generators
Do we want to only test --ignition-generators with this test? Michi made the
good point yesterday that we should probably just remove the
--ignition-generators flag and use ignition generators if --ignition is set
(otherwise FCG currently) - this would mean that all generator mjsunit tests
would end up being run with ignition generators in the ignition testing variant.
WDYT (as a followup CL)?

[neis, 2016-06-09 10:58:28]

On 2016/06/09 10:50:18, rmcilroy wrote:
> Interpreter changes LGTM, one suggestion on the tests.
> 
>
https://codereview.chromium.org/2051783002/diff/1/test/mjsunit/es6/debug-step...
> File test/mjsunit/es6/debug-stepin-generators.js (right):
> 
>
https://codereview.chromium.org/2051783002/diff/1/test/mjsunit/es6/debug-step...
> test/mjsunit/es6/debug-stepin-generators.js:5: // Flags: --expose-debug-as
debug
> --ignition-generators
> Do we want to only test --ignition-generators with this test? Michi made the
> good point yesterday that we should probably just remove the
> --ignition-generators flag and use ignition generators if --ignition is set
> (otherwise FCG currently) - this would mean that all generator mjsunit tests
> would end up being run with ignition generators in the ignition testing
variant.
> WDYT (as a followup CL)?

Hi Ross, not sure if I get your question. We want to test both
generator implementations, that's why I'm adding --ignition-generators
but not --ignition (same as in the other generator test files).

I'm fine with making --ignition imply --ignition-generators.  Once we
do this, I will remove --ignition-generators from these tests again
(which is then a purely cosmetic change).

[rmcilroy, 2016-06-09 11:02:30]

rmcilroy@chromium.org changed reviewers:
+ yangguo@chromium.org
- yanguo@chromium.org

[rmcilroy, 2016-06-09 11:02:31]

Fix Yang's email address on reviewers.

[rmcilroy, 2016-06-09 11:02:58]

On 2016/06/09 10:58:28, neis wrote:
> On 2016/06/09 10:50:18, rmcilroy wrote:
> > Interpreter changes LGTM, one suggestion on the tests.
> > 
> >
>
https://codereview.chromium.org/2051783002/diff/1/test/mjsunit/es6/debug-step...
> > File test/mjsunit/es6/debug-stepin-generators.js (right):
> > 
> >
>
https://codereview.chromium.org/2051783002/diff/1/test/mjsunit/es6/debug-step...
> > test/mjsunit/es6/debug-stepin-generators.js:5: // Flags: --expose-debug-as
> debug
> > --ignition-generators
> > Do we want to only test --ignition-generators with this test? Michi made the
> > good point yesterday that we should probably just remove the
> > --ignition-generators flag and use ignition generators if --ignition is set
> > (otherwise FCG currently) - this would mean that all generator mjsunit tests
> > would end up being run with ignition generators in the ignition testing
> variant.
> > WDYT (as a followup CL)?
> 
> Hi Ross, not sure if I get your question. We want to test both
> generator implementations, that's why I'm adding --ignition-generators
> but not --ignition (same as in the other generator test files).
> 
> I'm fine with making --ignition imply --ignition-generators.  Once we
> do this, I will remove --ignition-generators from these tests again
> (which is then a purely cosmetic change).

Ahh, I forgot that --ignition-generators does nothing if --ignition is false. In
that case the change is fine as-is, and we can later make  --ignition imply
--ignition-generators and remove --ignition-generator from the tests.

[commit-bot: I haz the power, 2016-06-09 11:03:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-09 11:03:50]

Dry run: This issue passed the CQ dry run.

[neis, 2016-06-09 11:58:10]

Description was changed from

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yanguo@chromium.org
BUG=v8:4907
==========

to

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yangguo@chromium.org
BUG=v8:4907
==========

[Yang, 2016-06-09 12:40:27]

On 2016/06/09 11:03:50, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

lgtm.

[adamk, 2016-06-10 12:51:06]

lgtm for parsing, sorry for the delay

[neis, 2016-06-10 12:51:31]

The CQ bit was checked by neis@chromium.org

[neis, 2016-06-10 12:51:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2051783002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-10 12:51:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2051783002/20001

[commit-bot: I haz the power, 2016-06-10 13:28:39]

Description was changed from

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yangguo@chromium.org
BUG=v8:4907
==========

to

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yangguo@chromium.org
BUG=v8:4907
==========

[commit-bot: I haz the power, 2016-06-10 13:28:40]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-10 13:30:27]

Description was changed from

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yangguo@chromium.org
BUG=v8:4907
==========

to

==========
[interpreter] Fix debug stepping for generators.

In commit b3bfc0bd58b07232da08c1d6e7504c55b4e80710, I corrected the source
position of yield-exceptions by not setting the "return position" on returns
that correspond to yields. It turns out that this caused a bug with debug
stepping. The proper fix is to keep the return position on those returns but
additionally attach the yield's source position to the Throw emitted in
VisitYield.

R=rmcilroy@chromium.org, yangguo@chromium.org
BUG=v8:4907

Committed: https://crrev.com/6e700b7f769e9f15a5193f9efcf43aa07fc3054d
Cr-Commit-Position: refs/heads/master@{#36896}
==========

[commit-bot: I haz the power, 2016-06-10 13:30:28]

Patchset 2 (id:??) landed as
https://crrev.com/6e700b7f769e9f15a5193f9efcf43aa07fc3054d
Cr-Commit-Position: refs/heads/master@{#36896}