add UseCounters for NonOctalDecimalIntegerLiteral in strict mode

In parallel to the strict octal check that would reject `012` in strict mode, this patch collects UseCounters for `089` in strict mode. The spec says this should be an error, but this patch does not report it as such.

BUG=v8:4973
LOG=y

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

[caitp (gmail), 2016-05-05 18:21:07]

caitpotter88@gmail.com changed reviewers:
+ caitpotter88@gmail.com

[caitp (gmail), 2016-05-05 18:21:08]

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

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc#newc...
src/parsing/scanner.cc:1034: decimal_that_looks_like_octal = true;
'8' and '9' are decidedly not octal digits --- why are we saying this looks
octal? Just confused about what problem this solves

[caitp (gmail), 2016-05-05 18:25:02]

Sorry, I think I get it, you want to prevent reporting the error if a series of
octal digits are followed by a decimal digit, right?

Anyways, there are some nits here

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc
File src/parsing/scanner.cc (left):

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc#oldc...
src/parsing/scanner.cc:990: int start_pos = source_pos();  // For reporting
octal positions.
This change seems not to be needed, since it's irrelevant when the number
doesn't begin with 0

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

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc#newc...
src/parsing/scanner.cc:1067: if (decimal_that_looks_like_octal)
`decimal_that_looks_like_octal` is always false unless the `c0_ == '0'` branch
is taken on line 991 --- these should be safe to get rid of, AFAIK?

[jwolfe, 2016-05-05 18:29:56]

The variable "decimal_that_looks_like_octal" could definitely use a better name.
I just really don't like the name "NonOctalDecimalIntegerLiteral", because that
sounds like it would apply to every decimal integer literal that is not octal
(e.g. "1").

[jwolfe, 2016-05-05 18:42:02]

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc
File src/parsing/scanner.cc (left):

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc#oldc...
src/parsing/scanner.cc:990: int start_pos = source_pos();  // For reporting
octal positions.
On 2016/05/05 18:25:02, caitp wrote:
> This change seems not to be needed, since it's irrelevant when the number
> doesn't begin with 0

I can leave the local variable uninitialized until this line. I just need to
declare it out where the later code can see it.

I would have loved to have dealt with `octal_pos_` inside the `c0_ == '0'`
block, but I didn't know where the end of the token was going to be yet.

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

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc#newc...
src/parsing/scanner.cc:1067: if (decimal_that_looks_like_octal)
On 2016/05/05 18:25:02, caitp wrote:
> `decimal_that_looks_like_octal` is always false unless the `c0_ == '0'` branch
> is taken on line 991 --- these should be safe to get rid of, AFAIK?

I was surprised by needing two different checks in this function. This check is
for `09`, and the one on line 1107 is for `019`. I don't know why `019` isn't
reported as a SMI token by this function. Perhaps it's a mistake?

[caitp (gmail), 2016-05-06 21:01:34]

caitpotter88@gmail.com changed reviewers:
+ littledan@chromium.org

[caitp (gmail), 2016-05-06 21:01:34]

+littledan for frontend review

[caitp (gmail), 2016-05-06 21:08:03]

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

https://codereview.chromium.org/1948403002/diff/1/src/parsing/scanner.cc#newc...
src/parsing/scanner.cc:1108: octal_pos_ = Location(start_pos, source_pos());
So, if we want to have a different error message for this, we could save it as
`decimal_leading_zero_pos_` or something, and  pick which one to report in
CheckOctalLiteral()

[Dan Ehrenberg, 2016-05-06 21:30:46]

As noted at https://bugs.chromium.org/p/v8/issues/detail?id=4973 , I'm concerned
about the web compatibility of this patch. Let's get some data before landing
this error checking.

[jwolfe, 2016-05-06 22:37:38]

Description was changed from

==========
report error for NonOctalDecimalIntegerLiteral in strict mode

In short, if it's an error to write 012, it's also an error to write 018.

This code reuses the error detecting and reporting framework for octal
literals, and even reports the same message, which isn't quite accourate:

  print(019);
        ^^^
  SyntaxError: Octal literals are not allowed in strict mode.

In order to get a different error message, we'll need to add a field to
Scanner to either distinguish which kind of octal literal we encountered
last, or simply store the last known octal position in addition to the
last known decimal_that_looks_like_octal position.

BUG=v8:4973
==========

to

==========
report error for NonOctalDecimalIntegerLiteral in strict mode

In short, if it's an error to write 012, it's also an error to write 018.

This code reuses the error detecting and reporting framework for octal
literals, and even reports the same message, which isn't quite accourate:

  print(019);
        ^^^
  SyntaxError: Octal literals are not allowed in strict mode.

In order to get a different error message, we'll need to add a field to
Scanner to either distinguish which kind of octal literal we encountered
last, or simply store the last known octal position in addition to the
last known decimal_that_looks_like_octal position.

BUG=v8:4973
LOG=y
==========

[jwolfe, 2016-05-07 21:21:44]

According to v8.h, I still need to update V8Initializer.cpp in Chromium. I'm
planning on getting a Chromium dev environment set up next week so I can address
this.

[jwolfe, 2016-05-07 21:25:02]

Description was changed from

==========
report error for NonOctalDecimalIntegerLiteral in strict mode

In short, if it's an error to write 012, it's also an error to write 018.

This code reuses the error detecting and reporting framework for octal
literals, and even reports the same message, which isn't quite accourate:

  print(019);
        ^^^
  SyntaxError: Octal literals are not allowed in strict mode.

In order to get a different error message, we'll need to add a field to
Scanner to either distinguish which kind of octal literal we encountered
last, or simply store the last known octal position in addition to the
last known decimal_that_looks_like_octal position.

BUG=v8:4973
LOG=y
==========

to

==========
In parallel to the strict octal check that would reject `012` in strict mode,
this patch collects UseCounters for `089` in strict mode. The spec says this
should be an error, but this patch does not report it as such.

BUG=v8:4973
LOG=y
==========

[Dan Ehrenberg, 2016-05-11 00:36:57]

lgtm

https://codereview.chromium.org/1948403002/diff/60001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1948403002/diff/60001/include/v8.h#newcode5615
include/v8.h:5615: // and V8PerIsolateData.cpp in Chromium.
...and histogram.xml (see https://codereview.chromium.org/1578143002 for a full
example).

[jwolfe, 2016-05-11 01:00:41]

The CQ bit was checked by jwolfe@igalia.com

[jwolfe, 2016-05-11 01:00:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org
Link to the patchset: https://codereview.chromium.org/1948403002/#ps80001
(title: "don't forget histograms.xml")

[commit-bot: I haz the power, 2016-05-11 01:00:49]

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

[commit-bot: I haz the power, 2016-05-11 01:17:40]

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

[commit-bot: I haz the power, 2016-05-11 01:17:41]

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

[jwolfe, 2016-05-12 20:18:13]

The CQ bit was checked by jwolfe@igalia.com

[jwolfe, 2016-05-12 20:18:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org
Link to the patchset: https://codereview.chromium.org/1948403002/#ps100001
(title: "fix uninitialized field. guard against null")

[commit-bot: I haz the power, 2016-05-12 20:18:27]

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

[commit-bot: I haz the power, 2016-05-12 20:51:55]

Description was changed from

==========
In parallel to the strict octal check that would reject `012` in strict mode,
this patch collects UseCounters for `089` in strict mode. The spec says this
should be an error, but this patch does not report it as such.

BUG=v8:4973
LOG=y
==========

to

==========
In parallel to the strict octal check that would reject `012` in strict mode,
this patch collects UseCounters for `089` in strict mode. The spec says this
should be an error, but this patch does not report it as such.

BUG=v8:4973
LOG=y
==========

[commit-bot: I haz the power, 2016-05-12 20:51:56]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-12 20:54:25]

Description was changed from

==========
In parallel to the strict octal check that would reject `012` in strict mode,
this patch collects UseCounters for `089` in strict mode. The spec says this
should be an error, but this patch does not report it as such.

BUG=v8:4973
LOG=y
==========

to

==========
In parallel to the strict octal check that would reject `012` in strict mode,
this patch collects UseCounters for `089` in strict mode. The spec says this
should be an error, but this patch does not report it as such.

BUG=v8:4973
LOG=y

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

[commit-bot: I haz the power, 2016-05-12 20:54:26]

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

[Michael Achenbach, 2016-05-13 06:52:40]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1970333004/ by machenbach@chromium.org.

The reason for reverting is: [Sheriff] Breaks
https://build.chromium.org/p/client.v8.ports/builders/V8%20Arm%20-%20debug%20....

[caitp (gmail), 2016-05-13 12:58:31]

https://codereview.chromium.org/1948403002/diff/100001/src/parsing/scanner.cc
File src/parsing/scanner.cc (right):

https://codereview.chromium.org/1948403002/diff/100001/src/parsing/scanner.cc...
src/parsing/scanner.cc:990: int start_pos;  // For reporting octal positions.
Maybe rename this to `octal_start_pos` and initialize it here --- though I think
you should be just fine to initialize this within the `c0_ == '0'` branch
(provided you don't use it outside of the branch).