tools/gn: disallow non-canonical integer literals

tools/gn: disallow non-canonical integer literals

In most languages familiar to Chromium developers (e.g., C++,
JavaScript, Python, Java, Go, Bash, Perl, Ruby), a leading 0 in an
integer literal indicates an octal number.  For example, "010" and
"-010" represent 8 and -8, respectively.

GN doesn't have any use for octal integer literals, so reject integers
with leading zeros to avoid confusion rather than interpreting them as
decimal values.

While here, "-0" isn't useful either since it's equivalent to "0", so
reject that too.

No Chromium BUILD.gn files are affected by this change.

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

[mdempsky, 2015-03-26 23:52:37]

mdempsky@chromium.org changed reviewers:
+ brettw@chromium.org

[tfarina, 2015-03-27 00:02:54]

https://codereview.chromium.org/1038233002/diff/20001/tools/gn/parse_tree.cc
File tools/gn/parse_tree.cc (right):

https://codereview.chromium.org/1038233002/diff/20001/tools/gn/parse_tree.cc#...
tools/gn/parse_tree.cc:637: const auto& s = value_.value();
could you not use auto here?

[mdempsky, 2015-03-27 00:12:17]

https://codereview.chromium.org/1038233002/diff/20001/tools/gn/parse_tree.cc
File tools/gn/parse_tree.cc (right):

https://codereview.chromium.org/1038233002/diff/20001/tools/gn/parse_tree.cc#...
tools/gn/parse_tree.cc:637: const auto& s = value_.value();
On 2015/03/27 00:02:54, tfarina wrote:
> could you not use auto here?

Done.

[mdempsky, 2015-03-27 00:14:18]

https://codereview.chromium.org/1038233002/diff/20001/tools/gn/parse_tree.cc
File tools/gn/parse_tree.cc (right):

https://codereview.chromium.org/1038233002/diff/20001/tools/gn/parse_tree.cc#...
tools/gn/parse_tree.cc:637: const auto& s = value_.value();
On 2015/03/27 00:12:17, mdempsky wrote:
> On 2015/03/27 00:02:54, tfarina wrote:
> > could you not use auto here?
> 
> Done.

Hm, was that a question about why I didn't just use "auto", or a suggestion to
not use it at all?  (I interpreted as the former.)

[tfarina, 2015-03-27 00:17:17]

https://codereview.chromium.org/1038233002/diff/30001/tools/gn/parse_tree.cc
File tools/gn/parse_tree.cc (right):

https://codereview.chromium.org/1038233002/diff/30001/tools/gn/parse_tree.cc#...
tools/gn/parse_tree.cc:637: auto s = value_.value();
to not use it at all. I don't like to have to go to the header (parse_tree.h) to
see what value_ is, then to go to header of value_, to figure out what value()
is.

[mdempsky, 2015-03-27 00:18:54]

https://codereview.chromium.org/1038233002/diff/30001/tools/gn/parse_tree.cc
File tools/gn/parse_tree.cc (right):

https://codereview.chromium.org/1038233002/diff/30001/tools/gn/parse_tree.cc#...
tools/gn/parse_tree.cc:637: auto s = value_.value();
On 2015/03/27 00:17:17, tfarina wrote:
> to not use it at all. I don't like to have to go to the header (parse_tree.h)
to
> see what value_ is, then to go to header of value_, to figure out what value()
> is.

Done.

[tfarina, 2015-03-27 00:25:27]

Thanks for the auto change.

https://codereview.chromium.org/1038233002/diff/50001/tools/gn/parse_tree.cc
File tools/gn/parse_tree.cc (right):

https://codereview.chromium.org/1038233002/diff/50001/tools/gn/parse_tree.cc#...
tools/gn/parse_tree.cc:638: if ((s.starts_with("0") && s.size() > 1) ||
s.starts_with("-0")) {
The logic seems correct. I don't see problems with it, seems pretty
straightforward.

But I will let Brett sign this off. Maybe he can spot something that I can't.

[mdempsky, 2015-03-30 20:09:44]

Ping?

[brettw, 2015-03-31 23:05:34]

lgtm

https://codereview.chromium.org/1038233002/diff/50001/tools/gn/parse_tree_uni...
File tools/gn/parse_tree_unittest.cc (right):

https://codereview.chromium.org/1038233002/diff/50001/tools/gn/parse_tree_uni...
tools/gn/parse_tree_unittest.cc:225: const char *bad[] = {
I think it would be better to move this to right above the loop (style guide
says to keep var declarations as close as possible to use).

[mdempsky, 2015-03-31 23:19:20]

https://codereview.chromium.org/1038233002/diff/50001/tools/gn/parse_tree_uni...
File tools/gn/parse_tree_unittest.cc (right):

https://codereview.chromium.org/1038233002/diff/50001/tools/gn/parse_tree_uni...
tools/gn/parse_tree_unittest.cc:225: const char *bad[] = {
On 2015/03/31 23:05:33, brettw wrote:
> I think it would be better to move this to right above the loop (style guide
> says to keep var declarations as close as possible to use).

Done.

[mdempsky, 2015-03-31 23:19:24]

The CQ bit was checked by mdempsky@chromium.org

[mdempsky, 2015-03-31 23:19:24]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1038233002/#ps70001
(title: "Move kBad tests down by usage")

[commit-bot: I haz the power, 2015-03-31 23:19:43]

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

[commit-bot: I haz the power, 2015-03-31 23:58:32]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2015-03-31 23:59:07]

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