[strong] Remove all remainders of strong mode

[strong] Remove all remainders of strong mode

R=mstarzinger@chromium.org,bmeurer@chromium.org,adamk@chromium.org
BUG=v8:3956
LOG=Y

Committed: https://crrev.com/4614c7caaf82879e6aa9e8b67d7a1ef606c4f6f3
Cr-Commit-Position: refs/heads/master@{#34669}

[rossberg, 2016-03-07 15:37:21]

Description was changed from

==========
[strong] Remove remainders of strong mode

BUG=
==========

to

==========
[strong] Remove all remainders of strong mode

BUG=v8:3956
LOG=Y
==========

[rossberg, 2016-03-08 10:16:27]

Description was changed from

==========
[strong] Remove all remainders of strong mode

BUG=v8:3956
LOG=Y
==========

to

==========
[strong] Remove all remainders of strong mode

R=mstarzinger@chromium.org,bmeurer@chromium.org,adamk@chromium.org
BUG=v8:3956
LOG=Y
==========

[rossberg, 2016-03-08 10:16:28]

rossberg@chromium.org changed reviewers:
+ adamk@chromium.org, bmeurer@chromium.org, mstarzinger@chromium.org

[rossberg, 2016-03-08 10:16:51]

PTAL

[Michael Starzinger, 2016-03-08 10:53:46]

LGTM. Nice! I like it.

https://codereview.chromium.org/1773653002/diff/1/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1773653002/diff/1/src/globals.h#newcode246
src/globals.h:246: enum LanguageMode { SLOPPY, STRICT, LANGUAGE_END = 3 };
As discussed offline: A nice follow-up to this CL would be to drop LANGUAGE_END
down to 1 (or even remove it completely) and adapt all the bit-fields that store
a language mode to only be 1 bit wide instead of 2 bits.

[Benedikt Meurer, 2016-03-08 13:14:40]

LGTM (rubber-stamped)

[adamk, 2016-03-08 17:53:22]

lgtm % parser nits

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

https://codereview.chromium.org/1773653002/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:5378: void Parser::RaiseLanguageMode(Scope* scope,
LanguageMode mode) {
Not clear why this needed to change at all in this patch; could have kept using
scope_.

https://codereview.chromium.org/1773653002/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:5380: SetLanguageMode(scope, old > mode ? old : mode);
Seems bad to rely on STRICT > SLOPPY (mathematically) here, this could just be:

SetLanguageMode(scope, is_strict(mode) ? mode : old);

[rossberg, 2016-03-10 10:41:20]

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

https://codereview.chromium.org/1773653002/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:5378: void Parser::RaiseLanguageMode(Scope* scope,
LanguageMode mode) {
On 2016/03/08 17:53:22, adamk wrote:
> Not clear why this needed to change at all in this patch; could have kept
using
> scope_.

Was supposed to be a clean-up to be consistent with SetLanguageMode. But I don't
care enough. Changed.

https://codereview.chromium.org/1773653002/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:5380: SetLanguageMode(scope, old > mode ? old : mode);
On 2016/03/08 17:53:22, adamk wrote:
> Seems bad to rely on STRICT > SLOPPY (mathematically) here, this could just
be:
> 
> SetLanguageMode(scope, is_strict(mode) ? mode : old);

That would be more brittle, and silently break if you add additional modes
again. Not sure I see the concern either: the notion of "Raise" only makes sense
if there is an ordering (and ordering on enums is well-defined). So I'd rather
keep this as is.

[rossberg, 2016-03-10 10:41:30]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-03-10 10:41:30]

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

[commit-bot: I haz the power, 2016-03-10 10:41:36]

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

[commit-bot: I haz the power, 2016-03-10 10:44:06]

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

[commit-bot: I haz the power, 2016-03-10 10:44:07]

Try jobs failed on following builders:
  v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/2681)
  v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

[rossberg, 2016-03-10 10:51:58]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-03-10 10:51:59]

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

[commit-bot: I haz the power, 2016-03-10 10:52:07]

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

[commit-bot: I haz the power, 2016-03-10 10:55:34]

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

[commit-bot: I haz the power, 2016-03-10 10:55:36]

Try jobs failed on following builders:
  v8_win_compile_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/1...)
  v8_win_nosnap_shared_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_compil...)

[rossberg, 2016-03-10 11:01:14]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-03-10 11:01:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, mstarzinger@chromium.org, adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/1773653002/#ps60001
(title: "Happiness for dumb compilers")

[commit-bot: I haz the power, 2016-03-10 11:01:30]

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

[commit-bot: I haz the power, 2016-03-10 11:08:49]

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

[commit-bot: I haz the power, 2016-03-10 11:08:51]

Try jobs failed on following builders:
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/16802)
  v8_win64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/4229)

[rossberg, 2016-03-10 12:15:18]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-03-10 12:15:19]

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

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

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

[commit-bot: I haz the power, 2016-03-10 12:21:58]

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

[commit-bot: I haz the power, 2016-03-10 12:21:59]

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

[rossberg, 2016-03-10 12:35:53]

The CQ bit was checked by rossberg@chromium.org

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

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

[commit-bot: I haz the power, 2016-03-10 12:43:57]

Description was changed from

==========
[strong] Remove all remainders of strong mode

R=mstarzinger@chromium.org,bmeurer@chromium.org,adamk@chromium.org
BUG=v8:3956
LOG=Y
==========

to

==========
[strong] Remove all remainders of strong mode

R=mstarzinger@chromium.org,bmeurer@chromium.org,adamk@chromium.org
BUG=v8:3956
LOG=Y
==========

[commit-bot: I haz the power, 2016-03-10 12:43:58]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-10 12:45:46]

Description was changed from

==========
[strong] Remove all remainders of strong mode

R=mstarzinger@chromium.org,bmeurer@chromium.org,adamk@chromium.org
BUG=v8:3956
LOG=Y
==========

to

==========
[strong] Remove all remainders of strong mode

R=mstarzinger@chromium.org,bmeurer@chromium.org,adamk@chromium.org
BUG=v8:3956
LOG=Y

Committed: https://crrev.com/4614c7caaf82879e6aa9e8b67d7a1ef606c4f6f3
Cr-Commit-Position: refs/heads/master@{#34669}
==========

[commit-bot: I haz the power, 2016-03-10 12:45:48]

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

[Michael Achenbach, 2016-03-10 13:12:01]

This seems to break compilation with gcc and code coverage:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20-%20gcov%20cov...

Could you take a look?

[adamk, 2016-03-10 18:46:49]

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

https://codereview.chromium.org/1773653002/diff/1/src/parsing/parser.cc#newco...
src/parsing/parser.cc:5378: void Parser::RaiseLanguageMode(Scope* scope,
LanguageMode mode) {
On 2016/03/10 10:41:19, rossberg wrote:
> On 2016/03/08 17:53:22, adamk wrote:
> > Not clear why this needed to change at all in this patch; could have kept
> using
> > scope_.
> 
> Was supposed to be a clean-up to be consistent with SetLanguageMode. But I
don't
> care enough. Changed.

Yeah, I can see that, just my inclination in general to keep separate concerns
in separate patches.