Update the clang plugin to optionally enforce rules for Blink code.

Update the clang plugin to optionally enforce rules for Blink code.

BUG=535367
TEST=it all stays green

Committed: https://crrev.com/3d8f27691f52f96578f83fe0d3db01bd65e0df74
Cr-Commit-Position: refs/heads/master@{#353187}

[Avi (use Gerrit), 2015-10-01 18:06:27]

avi@chromium.org changed reviewers:
+ thakis@chromium.org

[Avi (use Gerrit), 2015-10-01 18:06:27]

Nico—

There's disagreement on blink-dev whether we want to turn this on, but Daniel
suggested we might want to land the flag anyway and leave it off to give us the
flexibility to try turning it on.

[dcheng, 2015-10-01 21:42:48]

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

[dcheng, 2015-10-01 21:42:49]

I leave it up to thakis@ whether we should land this or not: there's currently
~6.5k unique warnings in Blink, mostly about virtuals defined inline and inlined
constructors/destructors of various sorts.

https://codereview.chromium.org/1372833002/diff/1/tools/clang/plugins/ChromeC...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/1372833002/diff/1/tools/clang/plugins/ChromeC...
tools/clang/plugins/ChromeClassTester.cpp:204: banned_namespaces_.insert("std");
IMO, if you're going to change all these, you may as well use emplace.

[Avi (use Gerrit), 2015-10-01 21:52:07]

https://codereview.chromium.org/1372833002/diff/1/tools/clang/plugins/ChromeC...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/1372833002/diff/1/tools/clang/plugins/ChromeC...
tools/clang/plugins/ChromeClassTester.cpp:204: banned_namespaces_.insert("std");
On 2015/10/01 21:42:48, dcheng wrote:
> IMO, if you're going to change all these, you may as well use emplace.

Wouldn't the same rules about no C++11 library features apply here?

[dcheng, 2015-10-01 21:53:23]

https://codereview.chromium.org/1372833002/diff/1/tools/clang/plugins/ChromeC...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/1372833002/diff/1/tools/clang/plugins/ChromeC...
tools/clang/plugins/ChromeClassTester.cpp:204: banned_namespaces_.insert("std");
On 2015/10/01 at 21:52:06, Avi wrote:
> On 2015/10/01 21:42:48, dcheng wrote:
> > IMO, if you're going to change all these, you may as well use emplace.
> 
> Wouldn't the same rules about no C++11 library features apply here?

Nope =)

The plugin already uses std::unique_ptr and stuff like that.

[Avi (use Gerrit), 2015-10-01 22:02:43]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-01 22:03:36]

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

[commit-bot: I haz the power, 2015-10-01 22:09:27]

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

[commit-bot: I haz the power, 2015-10-01 22:09:27]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Nico, 2015-10-09 00:09:19]

lgtm, seems fine to me. easy enough to remove. can you mention the thread title
of the discussion thread?

[Avi (use Gerrit), 2015-10-09 00:13:54]

On 2015/10/09 00:09:19, Nico (offline until Fri Oct 9) wrote:
> lgtm, seems fine to me. easy enough to remove. can you mention the thread
title
> of the discussion thread?

I can link the discussion thread. Do you mean in the CL description or as a
comment in the code?

[Nico, 2015-10-09 00:15:14]

Cl description is enough. (Just a comment on the review is fine with me too)

[Avi (use Gerrit), 2015-10-09 00:18:52]

On 2015/10/09 00:15:14, Nico (offline until Fri Oct 9) wrote:
> Cl description is enough. (Just a comment on the review is fine with me too)

For reference, I brought up the topic of turning this on in
https://groups.google.com/a/chromium.org/d/msg/blink-dev/PUqNtuEsY_8/AdEUjDz0...
. I'm not putting this in with the idea of immediately turning it on, but it'll
be nice to have this ability in the plugin to play with.

[Avi (use Gerrit), 2015-10-09 00:19:22]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2015-10-09 00:20:53]

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

[commit-bot: I haz the power, 2015-10-09 00:38:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-09 00:39:34]

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