[parsing] Be less pessimistic about maybe_assigned of parameters.

[parsing] Be less pessimistic about maybe_assigned of parameters.

Instead of unconditionally setting maybe_assigned for parameters, treat
parameters like other variables except that at the end we set maybe_assigned if
the function has a sloppy arguments object.

R=adamk@chromium.org, mstarzinger@chromium.org
BUG=v8:5636
Review-Url: https://codereview.chromium.org/2578103002
Cr-Commit-Position: refs/heads/master@{#41731}
Committed: https://chromium.googlesource.com/v8/v8/+/7ca72292834361e2ebcff5da44e7657d6f53d388

[neis, 2016-12-15 11:54:55]

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

[commit-bot: I haz the power, 2016-12-15 11:54:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[adamk, 2016-12-15 11:58:36]

lgtm with the additional tests

https://codereview.chromium.org/2578103002/diff/1/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2578103002/diff/1/test/cctest/test-parsing.cc...
test/cctest/test-parsing.cc:3451: {false, "(function f(arg=1) {g(arg)})"},
Can you add a few tests with an argument called "arguments"?

f(arguments)
f(arguments=1)
f(...arguments)

[commit-bot: I haz the power, 2016-12-15 12:00:10]

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

[commit-bot: I haz the power, 2016-12-15 12:00:10]

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

[neis, 2016-12-15 12:13:15]

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

[commit-bot: I haz the power, 2016-12-15 12:13:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 12:26:24]

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

[commit-bot: I haz the power, 2016-12-15 12:26:25]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...)

[neis, 2016-12-15 12:56:29]

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

[commit-bot: I haz the power, 2016-12-15 12:56:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[marja, 2016-12-15 13:09:53]

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

[marja, 2016-12-15 13:09:54]

lgtm %

https://codereview.chromium.org/2578103002/diff/40001/test/cctest/test-parsin...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2578103002/diff/40001/test/cctest/test-parsin...
test/cctest/test-parsing.cc:3484: {true, "(function f(arg) {g(arg);
eval('arguments[0] = 42'); g(arg)})"},
Can you add a test where arg is the rest parameter (and then either does or
doesn't get assigned w/ the various constructs)?

[commit-bot: I haz the power, 2016-12-15 13:15:58]

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

[commit-bot: I haz the power, 2016-12-15 13:15:59]

Dry run: This issue passed the CQ dry run.

[marja, 2016-12-15 13:35:29]

https://codereview.chromium.org/2578103002/diff/40001/test/cctest/test-parsin...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2578103002/diff/40001/test/cctest/test-parsin...
test/cctest/test-parsing.cc:3445: {false, "(function f(arg) {})"},
Routing offline discussion here: The two cases you want to test are 1) top level
eager funcs and 2) inner lazy funcs

Here you always force the function to be eager so the lazy-nolazy distinguishion
doesn't do want you want it to do.

[Michael Starzinger, 2016-12-15 13:59:53]

LGTM from my end.

[gsathya, 2016-12-15 14:05:30]

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

[gsathya, 2016-12-15 14:05:31]

https://codereview.chromium.org/2578103002/diff/40001/test/cctest/test-parsin...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2578103002/diff/40001/test/cctest/test-parsin...
test/cctest/test-parsing.cc:3449: {false, "(function f(arg) {function h() {
g(arg) }; return h})"},
isn't this same as the previous one?

[neis, 2016-12-15 14:56:53]

Description was changed from

==========
[parsing] Be less pessimistic about maybe_assigned of parameters.

Instead of unconditionally setting maybe_assigned for parameters, treat
parameters like other variables except that at the end we set maybe_assigned if
the function has a sloppy argument object.

R=adamk@chromium.org, mstarzinger@chromium.org
BUG=v8:5636
==========

to

==========
[parsing] Be less pessimistic about maybe_assigned of parameters.

Instead of unconditionally setting maybe_assigned for parameters, treat
parameters like other variables except that at the end we set maybe_assigned if
the function has a sloppy arguments object.

R=adamk@chromium.org, mstarzinger@chromium.org
BUG=v8:5636
==========

[neis, 2016-12-15 15:16:51]

The CQ bit was checked by neis@chromium.org

[neis, 2016-12-15 15:16:52]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org,
marja@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2578103002/#ps80001
(title: "Simplify.")

[commit-bot: I haz the power, 2016-12-15 15:17:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 15:22:53]

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

[commit-bot: I haz the power, 2016-12-15 15:22:54]

Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[neis, 2016-12-15 15:36:34]

The CQ bit was checked by neis@chromium.org

[neis, 2016-12-15 15:36:34]

The patchset sent to the CQ was uploaded after l-g-t-m from marja@chromium.org,
adamk@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2578103002/#ps100001
(title: "Remove unused var.")

[commit-bot: I haz the power, 2016-12-15 15:36:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 16:04:45]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481816194140000,
"parent_rev": "b4aadaec1e46a98f2c2ee273c99aa808e19c770f", "commit_rev":
"7ca72292834361e2ebcff5da44e7657d6f53d388"}

[commit-bot: I haz the power, 2016-12-15 16:04:50]

Description was changed from

==========
[parsing] Be less pessimistic about maybe_assigned of parameters.

Instead of unconditionally setting maybe_assigned for parameters, treat
parameters like other variables except that at the end we set maybe_assigned if
the function has a sloppy arguments object.

R=adamk@chromium.org, mstarzinger@chromium.org
BUG=v8:5636
==========

to

==========
[parsing] Be less pessimistic about maybe_assigned of parameters.

Instead of unconditionally setting maybe_assigned for parameters, treat
parameters like other variables except that at the end we set maybe_assigned if
the function has a sloppy arguments object.

R=adamk@chromium.org, mstarzinger@chromium.org
BUG=v8:5636

Review-Url: https://codereview.chromium.org/2578103002
Cr-Commit-Position: refs/heads/master@{#41731}
Committed:
https://chromium.googlesource.com/v8/v8/+/7ca72292834361e2ebcff5da44e7657d6f5...
==========

[commit-bot: I haz the power, 2016-12-15 16:04:52]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/v8/v8/+/7ca72292834361e2ebcff5da44e7657d6f5...