gn: Enforce braces after 'else'

gn: Enforce braces after 'else'

R=brettw@chromium.org
BUG=434508
Committed: https://crrev.com/3d5823f1b9b8220cacd112aaa9f97793e0b8317e
Cr-Commit-Position: refs/heads/master@{#319975}

[Dirk Pranke, 2015-03-10 19:49:15]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2015-03-10 19:49:15]

lgtm

[tfarina, 2015-03-10 20:37:24]

tfarina@chromium.org changed reviewers:
+ tfarina@chromium.org

[tfarina, 2015-03-10 20:37:25]

https://codereview.chromium.org/996893002/diff/1/tools/gn/parser.cc
File tools/gn/parser.cc (right):

https://codereview.chromium.org/996893002/diff/1/tools/gn/parser.cc#newcode503
tools/gn/parser.cc:503: if (!LookAhead(Token::LEFT_BRACE) &&
!LookAhead(Token::IF)) {
|| here does the wrong thing?

Anyway, thanks for figuring out this!

https://codereview.chromium.org/996893002/diff/1/tools/gn/parser.cc#newcode505
tools/gn/parser.cc:505: return scoped_ptr<ParseNode>();
This can be return nullptr;

[scottmg, 2015-03-10 21:21:00]

https://codereview.chromium.org/996893002/diff/1/tools/gn/parser.cc
File tools/gn/parser.cc (right):

https://codereview.chromium.org/996893002/diff/1/tools/gn/parser.cc#newcode503
tools/gn/parser.cc:503: if (!LookAhead(Token::LEFT_BRACE) &&
!LookAhead(Token::IF)) {
On 2015/03/10 20:37:24, tfarina wrote:
> || here does the wrong thing?

I'm not sure what you mean. Right now it says "if it's not a left brace, and
it's not an if" then error out. An "or" there would always go into the error
condition because it can only be one of those two things.
 
> Anyway, thanks for figuring out this!

https://codereview.chromium.org/996893002/diff/1/tools/gn/parser.cc#newcode505
tools/gn/parser.cc:505: return scoped_ptr<ParseNode>();
On 2015/03/10 20:37:24, tfarina wrote:
> This can be return nullptr;

There's a whole bunch that predate nullptr in this file. I don't want to change
them all in this CL, so I'm going to leave it like this and they can all be
changed together separately.

[scottmg, 2015-03-10 21:21:03]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2015-03-10 21:22:35]

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

[commit-bot: I haz the power, 2015-03-10 21:28:47]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-03-10 21:30:05]

Patchset 1 (id:??) landed as
https://crrev.com/3d5823f1b9b8220cacd112aaa9f97793e0b8317e
Cr-Commit-Position: refs/heads/master@{#319975}