[Interpreter] Adds support for with statement to interpreter.

[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280, v8:4684
LOG=N

Committed: https://crrev.com/8d3e1ca35735bf7c85976e1e0ee3cf4229e194ea
Cr-Commit-Position: refs/heads/master@{#33705}

[mythria, 2016-02-01 15:28:36]

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

[commit-bot: I haz the power, 2016-02-01 15:28:44]

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

[commit-bot: I haz the power, 2016-02-01 15:31:04]

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

[commit-bot: I haz the power, 2016-02-01 15:31:05]

Dry run: Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/10374)

[mythria, 2016-02-01 15:38:49]

Description was changed from

==========
[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280,v8:4684
==========

to

==========
[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280,v8:4684
LOG=N
==========

[mythria, 2016-02-01 16:14:42]

mythria@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[mythria, 2016-02-01 16:14:42]

PTAL,

Thanks,
Mythri

https://codereview.chromium.org/1656863002/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1656863002/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:2477: } else if
(closure_scope->is_eval_scope()) {
Turbofan never hits this. I implemented this from full-codegen:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/full-codege...

I think I understood the comment, but could you please check if I am doing it
correct. I am never entirely sure about contexts. All tests in test262 pass, so
it might be correct :) We hit this case when we have with within eval.

https://codereview.chromium.org/1656863002/diff/1/test/mjsunit/mjsunit.status
File test/mjsunit/mjsunit.status (left):

https://codereview.chromium.org/1656863002/diff/1/test/mjsunit/mjsunit.status...
test/mjsunit/mjsunit.status:757: 'regress/regress-debug*': [SKIP],
few more tests pass in es6/ harmony/ but I dint enable them for now.

[rmcilroy, 2016-02-02 17:05:23]

LGTM once comments are addressed, thanks.

https://codereview.chromium.org/1656863002/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1656863002/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:2477: } else if
(closure_scope->is_eval_scope()) {
On 2016/02/01 16:14:42, mythria wrote:
> Turbofan never hits this. I implemented this from full-codegen:
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/full-codege...
> 
> I think I understood the comment, but could you please check if I am doing it
> correct. I am never entirely sure about contexts. All tests in test262 pass,
so
> it might be correct :) We hit this case when we have with within eval. 

This looks right to me, thanks!

https://codereview.chromium.org/1656863002/diff/1/test/mjsunit/mjsunit.status
File test/mjsunit/mjsunit.status (left):

https://codereview.chromium.org/1656863002/diff/1/test/mjsunit/mjsunit.status...
test/mjsunit/mjsunit.status:757: 'regress/regress-debug*': [SKIP],
On 2016/02/01 16:14:42, mythria wrote:
> few more tests pass in es6/ harmony/ but I dint enable them for now.

Acknowledged.

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/cctest.status
File test/cctest/cctest.status (left):

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/cctest.stat...
test/cctest/cctest.status:588: 'test-unscopables-hidden-prototype/Unscopables':
[SKIP],
You'll need to rebase for this file (sorry.... ;).

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-interpreter.cc (right):

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:3847: "    with(y) return x;"
nit - remove extra space

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:3854: "    with(o) return x;"
ditto

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:3868: }
Could you add these tests to test-run-bytecode-graph-builder too please.

https://codereview.chromium.org/1656863002/diff/20001/test/mjsunit/mjsunit.st...
File test/mjsunit/mjsunit.status (left):

https://codereview.chromium.org/1656863002/diff/20001/test/mjsunit/mjsunit.st...
test/mjsunit/mjsunit.status:965: 'with-value': [SKIP],
Nice large number of newly passing tests! :)

[mythria, 2016-02-03 12:21:35]

Thanks for the review. Addressed all comments.

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/cctest.status
File test/cctest/cctest.status (left):

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/cctest.stat...
test/cctest/cctest.status:588: 'test-unscopables-hidden-prototype/Unscopables':
[SKIP],
On 2016/02/02 17:05:23, rmcilroy wrote:
> You'll need to rebase for this file (sorry.... ;).

Done.

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/test-interpreter.cc (right):

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:3847: "    with(y) return x;"
On 2016/02/02 17:05:23, rmcilroy wrote:
> nit - remove extra space

Done.

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:3854: "    with(o) return x;"
On 2016/02/02 17:05:23, rmcilroy wrote:
> ditto

Done.

https://codereview.chromium.org/1656863002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/test-interpreter.cc:3868: }
On 2016/02/02 17:05:23, rmcilroy wrote:
> Could you add these tests to test-run-bytecode-graph-builder too please.

Done.

[mythria, 2016-02-03 12:21:40]

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

[commit-bot: I haz the power, 2016-02-03 12:21:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1656863002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1656863002/60001

[commit-bot: I haz the power, 2016-02-03 12:36:18]

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

[commit-bot: I haz the power, 2016-02-03 12:36:19]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/13209)

[rmcilroy, 2016-02-03 12:38:30]

still lgtm

[mythria, 2016-02-03 13:36:56]

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

[commit-bot: I haz the power, 2016-02-03 13:37:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1656863002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1656863002/80001

[commit-bot: I haz the power, 2016-02-03 14:06:54]

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

[commit-bot: I haz the power, 2016-02-03 14:06:55]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-02-03 14:33:25]

The CQ bit was checked by mythria@chromium.org

[mythria, 2016-02-03 14:33:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1656863002/#ps80001
(title: "Fixed cctest.status.")

[commit-bot: I haz the power, 2016-02-03 14:33:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1656863002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1656863002/80001

[commit-bot: I haz the power, 2016-02-03 14:41:25]

Description was changed from

==========
[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280,v8:4684
LOG=N
==========

to

==========
[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280,v8:4684
LOG=N
==========

[commit-bot: I haz the power, 2016-02-03 14:41:26]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-03 14:41:46]

Description was changed from

==========
[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280,v8:4684
LOG=N
==========

to

==========
[Interpreter] Adds support for with statement to interpreter.

Adds implementation and tests for with statement to interprter.

BUG=v8:4280,v8:4684
LOG=N

Committed: https://crrev.com/8d3e1ca35735bf7c85976e1e0ee3cf4229e194ea
Cr-Commit-Position: refs/heads/master@{#33705}
==========

[commit-bot: I haz the power, 2016-02-03 14:41:47]

Patchset 5 (id:??) landed as
https://crrev.com/8d3e1ca35735bf7c85976e1e0ee3cf4229e194ea
Cr-Commit-Position: refs/heads/master@{#33705}

[Michael Achenbach, 2016-02-03 15:05:01]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2016-02-03 15:05:01]

Please add skips for these failures:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20...