Treat a '!' preceding a function literal as eager-compile hint.

Treat a '!' preceding a function literal as eager-compile hint.

Some minifiers use the pattern !function ... () for JS code that should
be immediately executed. This change recognizes that pattern and treats
it equally to parenthesized functions.

A bit more background info is in the referenced bug.

R=verwaest@chromium.org
BUG=v8:5643
Committed: https://crrev.com/eefe11a1e6516f4ceda9d581b6c28ec792896e7d
Cr-Commit-Position: refs/heads/master@{#41114}

[vogelheim, 2016-11-17 12:06:14]

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

[commit-bot: I haz the power, 2016-11-17 12:06:20]

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

[vogelheim, 2016-11-17 12:09:49]

https://codereview.chromium.org/2509143003/diff/1/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2509143003/diff/1/src/parsing/parser-base.h#n...
src/parsing/parser-base.h:1491:
outer_function_state_->next_function_is_exclaimed_ = false;
self-nitpick: :)  All this flag handling is rather pedestrian. I considered
putting them in some sort of 'next_function_hints' bit field (or so), but that
didn't seem to be worth the effort.

[commit-bot: I haz the power, 2016-11-17 12:30:50]

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

[commit-bot: I haz the power, 2016-11-17 12:30:51]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-11-17 13:56:49]

lgtm

https://codereview.chromium.org/2509143003/diff/20001/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/2509143003/diff/20001/src/parsing/parser.cc#n...
src/parsing/parser.cc:2534: (function_state_->next_function_is_parenthesized()
||
What about just something like: next_function_is_likely_called?

[vogelheim, 2016-11-17 14:56:31]

https://codereview.chromium.org/2509143003/diff/20001/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/2509143003/diff/20001/src/parsing/parser.cc#n...
src/parsing/parser.cc:2534: (function_state_->next_function_is_parenthesized()
||
On 2016/11/17 13:56:48, Toon Verwaest wrote:
> What about just something like: next_function_is_likely_called?

I thought about it... the reason why I prefer it this way is that at least this
keeps the decision in one place.

We've had a bunch of difficulties with parse decisions being made all over the
place. If we give FunctionState a next_function_is_likely_called member, then
anyone with access to FunctionState can make this eager/lazy-parse decision.
This way, the FunctionState records only clearly defined "facts" (as in: was
there an opening parenthesis/an exclamation point?), and we make the decision
here.

That is, admittedly, semantic nitpicking and hair splitting, since... in either
case we're passing booleans around, and the only actual difference would be that
we execute the || operator sooner (rather than later).

I'm not really sure.... If you have a strong opinion, I'll gladly follow it.
Otherwise, I'd leave this as-is.

[Toon Verwaest, 2016-11-17 15:08:42]

I don't really see the problem with anyone being able to set it since it's
always about the next function. I mostly prefer it since it seems simpler. But
I'm ok with the current form too. You decide ... :)

[vogelheim, 2016-11-18 12:54:28]

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

[commit-bot: I haz the power, 2016-11-18 12:54:37]

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

[vogelheim, 2016-11-18 12:58:15]

On 2016/11/17 15:08:42, Toon Verwaest wrote:
> I don't really see the problem with anyone being able to set it since it's
> always about the next function. I mostly prefer it since it seems simpler. But
> I'm ok with the current form too. You decide ... :)

Done.

While doing so, I did notice the this_* members & accessor is actually dead
code, since the new FunctionState is only being constructed after next_* is
read, so I removed it, too. I also extended the test case to make sure this
'reset' logic workd.

[commit-bot: I haz the power, 2016-11-18 13:19:50]

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

[commit-bot: I haz the power, 2016-11-18 13:19:51]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-11-18 13:59:48]

lgtm

https://codereview.chromium.org/2509143003/diff/40001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2509143003/diff/40001/src/parsing/parser-base...
src/parsing/parser-base.h:1781: if (peek() == Token::FUNCTION)
use {} after if for statements that don't fit on the same line.

[vogelheim, 2016-11-18 14:10:13]

The CQ bit was checked by vogelheim@chromium.org

[vogelheim, 2016-11-18 14:10:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/2509143003/#ps60001
(title: "Review feedback.")

[commit-bot: I haz the power, 2016-11-18 14:10:15]

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

[vogelheim, 2016-11-18 14:10:30]

https://codereview.chromium.org/2509143003/diff/40001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2509143003/diff/40001/src/parsing/parser-base...
src/parsing/parser-base.h:1781: if (peek() == Token::FUNCTION)
On 2016/11/18 13:59:48, Toon Verwaest wrote:
> use {} after if for statements that don't fit on the same line.

Done.

[commit-bot: I haz the power, 2016-11-18 14:13:05]

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

[commit-bot: I haz the power, 2016-11-18 14:13:06]

Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/16472)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[vogelheim, 2016-11-18 14:28:22]

The CQ bit was checked by vogelheim@chromium.org

[vogelheim, 2016-11-18 14:28:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/2509143003/#ps80001
(title: "Rebase + drop now obsolete FLAG_min_preparse_length.")

[commit-bot: I haz the power, 2016-11-18 14:28:26]

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-11-18 14:56:39]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-18 14:57:07]

Description was changed from

==========
Treat a '!' preceding a function literal as eager-compile hint.

Some minifiers use the pattern !function ... () for JS code that should
be immediately executed. This change recognizes that pattern and treats
it equally to parenthesized functions.

A bit more background info is in the referenced bug.

R=verwaest@chromium.org
BUG=v8:5643
==========

to

==========
Treat a '!' preceding a function literal as eager-compile hint.

Some minifiers use the pattern !function ... () for JS code that should
be immediately executed. This change recognizes that pattern and treats
it equally to parenthesized functions.

A bit more background info is in the referenced bug.

R=verwaest@chromium.org
BUG=v8:5643

Committed: https://crrev.com/eefe11a1e6516f4ceda9d581b6c28ec792896e7d
Cr-Commit-Position: refs/heads/master@{#41114}
==========

[commit-bot: I haz the power, 2016-11-18 14:57:10]

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