Introduce LanguageMode, drop StrictMode.

Introduce LanguageMode, drop StrictMode.

This enables adding more language modes in the future.

For maximum flexibility, LanguageMode is a bitmask, so we're not restricted to
use a sequence of language modes which are progressively stricter, but we can
express the language mode as combination of features.

For now, LanguageMode can only be "sloppy" or "strict", and there are
STATIC_ASSERTS in places which need to change when more modes are added.

LanguageMode is a bit like the old LanguageMode when "extended" mode was still
around (see https://codereview.chromium.org/8417035 and
https://codereview.chromium.org/181543002 ) except that it's transmitted through
all the layers (there's no StrictModeFlag).

BUG=

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

[marja, 2015-02-03 10:06:27]

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

[marja, 2015-02-03 10:06:28]

rossberg, ptal

I'd kinda like to rename SLOPPY in the enum to something else, since "sloppy"
means the opposite of "strict", and this will no longer be true after more modes
are added. "BASIC", "NORMAL", "NONE", "DEFAULT"?

This is why I also didn't add a is_sloppy() helper, since that might get
confused with comparing against that particular enum value, whereas the wanted
semantics is "is not strict".

[rossberg, 2015-02-03 12:26:21]

Nice, much cleaner than the language mode we had before.

> I'd kinda like to rename SLOPPY in the enum to something else, since "sloppy"
> means the opposite of "strict", and this will no longer be true after more
modes
> are added. "BASIC", "NORMAL", "NONE", "DEFAULT"?

Sloppy will continue to be the opposite of strict -- think of the new modes as
submodes of strict. I'd much prefer to stick to the established terminology.

> This is why I also didn't add a is_sloppy() helper, since that might get
> confused with comparing against that particular enum value, whereas the wanted
> semantics is "is not strict".

See above, I think is_sloppy is perfectly adequate (and easier to read than
!is_strict).

https://codereview.chromium.org/894683003/diff/40001/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/globals.h#newcode234
src/globals.h:234: #define is_strict(language_mode) ((language_mode) & STRICT)
Why a macro instead of an inline funciton?

https://codereview.chromium.org/894683003/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/objects-inl.h#newcod...
src/objects-inl.h:5818: DCHECK(!is_strict(this->language_mode()) ||
is_strict(language_mode));
Maybe abstract that into an is_stricter_than predicate.

https://codereview.chromium.org/894683003/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/894683003/diff/40001/src/objects.cc#newcode14003
src/objects.cc:14003: DCHECK(language_unchecked == SLOPPY || language_unchecked
== STRICT);
I suppose we could have a predicate is_valid_language_mode to avoid repeating
and maintaining these conditions.

https://codereview.chromium.org/894683003/diff/40001/src/parser.h
File src/parser.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/parser.h#newcode49
src/parser.h:49: DCHECK(backing_[kLanguageModeIndex] == SLOPPY ||
Same here, could use is_valid_l_m

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-comp...
File src/runtime/runtime-compiler.cc (right):

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-comp...
src/runtime/runtime-compiler.cc:446: DCHECK(args.smi_at(4) == STRICT ||
args.smi_at(4) == SLOPPY);
Could use predicate here as well.

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-utils.h
File src/runtime/runtime-utils.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-util...
src/runtime/runtime-utils.h:78: RUNTIME_ASSERT(args.smi_at(index) == STRICT ||  
    \
same here

https://codereview.chromium.org/894683003/diff/40001/test/unittests/compiler/...
File test/unittests/compiler/js-typed-lowering-unittest.cc (right):

https://codereview.chromium.org/894683003/diff/40001/test/unittests/compiler/...
test/unittests/compiler/js-typed-lowering-unittest.cc:65: const LanguageMode
kLanguageModes[] = {SLOPPY, STRICT};
Maybe add a static assert here as well, so that we remember to extend this?

[marja, 2015-02-03 14:45:02]

(intended to do everything except that one maybe is_stricter_than comment)

https://codereview.chromium.org/894683003/diff/40001/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/globals.h#newcode234
src/globals.h:234: #define is_strict(language_mode) ((language_mode) & STRICT)
On 2015/02/03 12:26:20, rossberg wrote:
> Why a macro instead of an inline funciton?

Made this inline; added is_sloppy and is_valid_language_mode.

https://codereview.chromium.org/894683003/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/objects-inl.h#newcod...
src/objects-inl.h:5818: DCHECK(!is_strict(this->language_mode()) ||
is_strict(language_mode));
On 2015/02/03 12:26:20, rossberg wrote:
> Maybe abstract that into an is_stricter_than predicate.

That seems overkill; this is now easier to read after adding is_sloppy:

is_sloppy(this->language_mode()) || is_strict(language_mode)

(And this occurs in... 2 places, I think)

https://codereview.chromium.org/894683003/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/894683003/diff/40001/src/objects.cc#newcode14003
src/objects.cc:14003: DCHECK(language_unchecked == SLOPPY || language_unchecked
== STRICT);
On 2015/02/03 12:26:20, rossberg wrote:
> I suppose we could have a predicate is_valid_language_mode to avoid repeating
> and maintaining these conditions.

Done.

https://codereview.chromium.org/894683003/diff/40001/src/parser.h
File src/parser.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/parser.h#newcode49
src/parser.h:49: DCHECK(backing_[kLanguageModeIndex] == SLOPPY ||
On 2015/02/03 12:26:20, rossberg wrote:
> Same here, could use is_valid_l_m

Done.

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-comp...
File src/runtime/runtime-compiler.cc (right):

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-comp...
src/runtime/runtime-compiler.cc:446: DCHECK(args.smi_at(4) == STRICT ||
args.smi_at(4) == SLOPPY);
On 2015/02/03 12:26:20, rossberg wrote:
> Could use predicate here as well.

Done.

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-utils.h
File src/runtime/runtime-utils.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/runtime/runtime-util...
src/runtime/runtime-utils.h:78: RUNTIME_ASSERT(args.smi_at(index) == STRICT ||  
    \
On 2015/02/03 12:26:20, rossberg wrote:
> same here

Done.

https://codereview.chromium.org/894683003/diff/40001/test/unittests/compiler/...
File test/unittests/compiler/js-typed-lowering-unittest.cc (right):

https://codereview.chromium.org/894683003/diff/40001/test/unittests/compiler/...
test/unittests/compiler/js-typed-lowering-unittest.cc:65: const LanguageMode
kLanguageModes[] = {SLOPPY, STRICT};
On 2015/02/03 12:26:20, rossberg wrote:
> Maybe add a static assert here as well, so that we remember to extend this?

Done.

[marja, 2015-02-03 14:50:15]

Oh and I also wanted to note, that I left the check as is_strict() in places
where we have options both for strict and sloppy, e.g.,

if (is_strict(...)) {
  do_strict_stuff
} else {
  do_sloppy_stuff
}

instead of

if (is_sloppy(...)) {
  do_sloppy_stuff
} else {
  do_strict_stuff
}

So that these places are now (still) consistently checking is_strict and listing
the strict option first.

[rossberg, 2015-02-03 15:14:29]

lgtm

https://codereview.chromium.org/894683003/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/894683003/diff/40001/src/objects-inl.h#newcod...
src/objects-inl.h:5818: DCHECK(!is_strict(this->language_mode()) ||
is_strict(language_mode));
On 2015/02/03 14:45:02, marja wrote:
> On 2015/02/03 12:26:20, rossberg wrote:
> > Maybe abstract that into an is_stricter_than predicate.
> 
> That seems overkill; this is now easier to read after adding is_sloppy:
> 
> is_sloppy(this->language_mode()) || is_strict(language_mode)
> 
> (And this occurs in... 2 places, I think)

The reason I thought this would be useful is that the condition might get more
complicated once we add more modes, and should hence be next to the mode
definition itself. There might also arise additional places where it comes up.

But I'm fine with deferring this.

[marja, 2015-02-03 17:24:09]

Thanks for review. The latest patch set adds the rest of the backends (they were
trivial - not necessarily needed to have a look).

[rossberg, 2015-02-03 17:40:15]

Still LGTM

[marja, 2015-02-04 08:33:58]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2015-02-04 08:34:21]

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

[commit-bot: I haz the power, 2015-02-04 08:36:32]

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

[commit-bot: I haz the power, 2015-02-04 08:36:34]

Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux_arm_rel on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/1150)
  v8_linux_rel on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/2604)
  v8_presubmit on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/345)

[marja, 2015-02-04 08:52:04]

Patchset #6 (id:100001) has been deleted

[marja, 2015-02-04 08:52:57]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2015-02-04 08:53:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/894683003/120001

[commit-bot: I haz the power, 2015-02-04 09:34:23]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-02-04 09:34:38]

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