Fix scopes for body of sloppy-mode for-in/of loop

Fix scopes for body of sloppy-mode for-in/of loop

This patch fixes an obscure edge case for functions defined as the
direct body of a for-of/for-in loop, such as the following:

 for (foo in []) function foo() { return foo; }

Here, the first occurrence of foo should point to the outer scope;
however, before this patch, it pointed to the inner foo in an
invalid way which caused an assertion about the scope chain to fail.
This patch fixes the scope chain by inserting an extra scope for
the body of the loop, not including the header.

BUG=chromium:542099
LOG=N
R=rossberg

Committed: https://crrev.com/d0618585a747e1ba51e84a2d30526d8b21ff7191
Cr-Commit-Position: refs/heads/master@{#31268}

[Dan Ehrenberg, 2015-10-13 09:40:38]

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

[commit-bot: I haz the power, 2015-10-13 09:40:46]

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

[commit-bot: I haz the power, 2015-10-13 10:09:06]

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

[commit-bot: I haz the power, 2015-10-13 10:09:07]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2015-10-13 10:09:39]

littledan@chromium.org changed reviewers:
+ rossberg@chromium.org
- adamk@chromium.org

[rossberg, 2015-10-13 10:52:04]

https://codereview.chromium.org/1396663004/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1396663004/diff/1/src/parser.cc#newcode3797
src/parser.cc:3797: // This block must not be based on for_scope because if a
lexical
What does "be based on" mean?

https://codereview.chromium.org/1396663004/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-542099.js (right):

https://codereview.chromium.org/1396663004/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-542099.js:8: for (foo in []) function foo() {
return foo; }
Can you extend this test such that it also tests that variables are resolved as
expected?

[Dan Ehrenberg, 2015-10-13 14:58:40]

PTAL

https://codereview.chromium.org/1396663004/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1396663004/diff/1/src/parser.cc#newcode3797
src/parser.cc:3797: // This block must not be based on for_scope because if a
lexical
On 2015/10/13 at 10:52:04, rossberg wrote:
> What does "be based on" mean?

Clarified wording

https://codereview.chromium.org/1396663004/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-542099.js (right):

https://codereview.chromium.org/1396663004/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-542099.js:8: for (foo in []) function foo() {
return foo; }
On 2015/10/13 at 10:52:04, rossberg wrote:
> Can you extend this test such that it also tests that variables are resolved
as expected?

Done

[Dan Ehrenberg, 2015-10-13 14:58:44]

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

[commit-bot: I haz the power, 2015-10-13 14:58:49]

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

[commit-bot: I haz the power, 2015-10-13 15:20:44]

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

[commit-bot: I haz the power, 2015-10-13 15:20:45]

Dry run: This issue passed the CQ dry run.

[rossberg, 2015-10-13 15:21:18]

LGTM module comments (online & offline)

https://codereview.chromium.org/1396663004/diff/20001/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-542099.js (right):

https://codereview.chromium.org/1396663004/diff/20001/test/mjsunit/regress/re...
test/mjsunit/regress/regress-542099.js:16: assertEquals("function", typeof
foo());
How about adding

assertSame(foo, foo());

[Dan Ehrenberg, 2015-10-13 16:02:41]

On 2015/10/13 at 14:58:49, commit-bot wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1396663004/20001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1396663004/20001

I tried to improve the comment wording further. PTAL.

[rossberg, 2015-10-13 16:05:27]

Excellent, thanks!

[Dan Ehrenberg, 2015-10-14 15:49:59]

The CQ bit was checked by littledan@chromium.org

[Dan Ehrenberg, 2015-10-14 15:49:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1396663004/#ps60001
(title: "Improve test")

[commit-bot: I haz the power, 2015-10-14 15:50:10]

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

[commit-bot: I haz the power, 2015-10-14 17:36:11]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-14 17:36:26]

Patchset 4 (id:??) landed as
https://crrev.com/d0618585a747e1ba51e84a2d30526d8b21ff7191
Cr-Commit-Position: refs/heads/master@{#31268}