clang/plugin: Make auto raw check default in FindBadConstructs plugin.

clang/plugin: Make auto raw check default in FindBadConstructs plugin.

This patch makes the auto raw deduction check default. It still keeps
the parsing value in there, since it's currently being used in
Chromium, but the intent is to remove it once this version rolls.

R=dcheng@chromium.org, thakis@chromium.org
BUG=554600
Review-Url: https://codereview.chromium.org/2732973004
Cr-Commit-Position: refs/heads/master@{#454982}
Committed: https://chromium.googlesource.com/chromium/src/+/7e93859e576b0675f2fa3a4b29d0122a1ba91ef8

[vmpstr, 2017-03-06 21:09:58]

Please take a look.

[dcheng, 2017-03-06 21:15:25]

lgtm

[Nico, 2017-03-06 21:19:24]

lgtm

I assume someone at some point checked that this check doesn't increase build
time.

[dcheng, 2017-03-06 21:20:57]

On 2017/03/06 21:19:24, Nico wrote:
> lgtm
> 
> I assume someone at some point checked that this check doesn't increase build
> time.

I did some basic measurements when converting the plugin to use
RecursiveASTVisitor but haven't done much benchmarking since then.
Btw, are there dashboards with compile times somewhere? I assume they're
internal only, but let's throw a link to them somewhere in the docs?

[dcheng, 2017-03-06 21:21:08]

On 2017/03/06 21:20:57, dcheng wrote:
> On 2017/03/06 21:19:24, Nico wrote:
> > lgtm
> > 
> > I assume someone at some point checked that this check doesn't increase
build
> > time.
> 
> I did some basic measurements when converting the plugin to use
> RecursiveASTVisitor but haven't done much benchmarking since then.
> Btw, are there dashboards with compile times somewhere? I assume they're
> internal only, but let's throw a link to them somewhere in the docs?

(RecursiveASTVisitor itself didn't add to the build time)

[Nico, 2017-03-06 21:23:05]

I don't know of any dashboards with compile time.

The clang trunk bots all do local clober builds, so clicking any builder on
https://build.chromium.org/p/chromium.fyi/console?category=clang%20tot and
then "stats" gives you something tracking compile time over the last 200
builds: https://build.chromium.org/p/chromium.fyi/stats/ClangToTLinux
Several (most?) of the tot bots build without plugin though.

On Mon, Mar 6, 2017 at 4:20 PM, <dcheng@chromium.org> wrote:

> On 2017/03/06 21:19:24, Nico wrote:
> > lgtm
> >
> > I assume someone at some point checked that this check doesn't increase
> build
> > time.
>
> I did some basic measurements when converting the plugin to use
> RecursiveASTVisitor but haven't done much benchmarking since then.
> Btw, are there dashboards with compile times somewhere? I assume they're
> internal only, but let's throw a link to them somewhere in the docs?
>
> https://codereview.chromium.org/2732973004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vmpstr, 2017-03-06 21:56:39]

On 2017/03/06 21:23:05, Nico wrote:
> I don't know of any dashboards with compile time.
> 
> The clang trunk bots all do local clober builds, so clicking any builder on
> https://build.chromium.org/p/chromium.fyi/console?category=clang%20tot and
> then "stats" gives you something tracking compile time over the last 200
> builds: https://build.chromium.org/p/chromium.fyi/stats/ClangToTLinux
> Several (most?) of the tot bots build without plugin though.
> 
> On Mon, Mar 6, 2017 at 4:20 PM, <mailto:dcheng@chromium.org> wrote:
> 
> > On 2017/03/06 21:19:24, Nico wrote:
> > > lgtm
> > >
> > > I assume someone at some point checked that this check doesn't increase
> > build
> > > time.
> >
> > I did some basic measurements when converting the plugin to use
> > RecursiveASTVisitor but haven't done much benchmarking since then.
> > Btw, are there dashboards with compile times somewhere? I assume they're
> > internal only, but let's throw a link to them somewhere in the docs?
> >
> > https://codereview.chromium.org/2732973004/
> >
> 

I remember us doing profiling when switching to the RecursiveASTVisitor which
didn't have an impact (as dcheng@ mentioned), but I don't think we profiled this
check specifically. I'll see if any of the build bots you've linked compile with
the plugin and then keep an eye out for any build time changes.

[Nico, 2017-03-06 22:20:58]

I don't think this cl will have an effect since we already pass the Flag
that turns on the check...

On Mar 6, 2017 4:56 PM, <vmpstr@chromium.org> wrote:

> On 2017/03/06 21:23:05, Nico wrote:
> > I don't know of any dashboards with compile time.
> >
> > The clang trunk bots all do local clober builds, so clicking any builder
> on
> > https://build.chromium.org/p/chromium.fyi/console?category=clang%20tot
> and
> > then "stats" gives you something tracking compile time over the last 200
> > builds: https://build.chromium.org/p/chromium.fyi/stats/ClangToTLinux
> > Several (most?) of the tot bots build without plugin though.
> >
> > On Mon, Mar 6, 2017 at 4:20 PM, <mailto:dcheng@chromium.org> wrote:
> >
> > > On 2017/03/06 21:19:24, Nico wrote:
> > > > lgtm
> > > >
> > > > I assume someone at some point checked that this check doesn't
> increase
> > > build
> > > > time.
> > >
> > > I did some basic measurements when converting the plugin to use
> > > RecursiveASTVisitor but haven't done much benchmarking since then.
> > > Btw, are there dashboards with compile times somewhere? I assume
> they're
> > > internal only, but let's throw a link to them somewhere in the docs?
> > >
> > > https://codereview.chromium.org/2732973004/
> > >
> >
>
> I remember us doing profiling when switching to the RecursiveASTVisitor
> which
> didn't have an impact (as dcheng@ mentioned), but I don't think we
> profiled this
> check specifically. I'll see if any of the build bots you've linked
> compile with
> the plugin and then keep an eye out for any build time changes.
>
> https://codereview.chromium.org/2732973004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vmpstr, 2017-03-06 22:30:26]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-03-06 22:31:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 22:42:19]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488839426237520, "parent_rev":
"7ee15a00fe953c13a83ef4faeddb9ec4a03dc1c1", "commit_rev":
"7e93859e576b0675f2fa3a4b29d0122a1ba91ef8"}

[commit-bot: I haz the power, 2017-03-06 22:43:18]

Description was changed from

==========
clang/plugin: Make auto raw check default in FindBadConstructs plugin.

This patch makes the auto raw deduction check default. It still keeps
the parsing value in there, since it's currently being used in
Chromium, but the intent is to remove it once this version rolls.

R=dcheng@chromium.org, thakis@chromium.org
BUG=554600
==========

to

==========
clang/plugin: Make auto raw check default in FindBadConstructs plugin.

This patch makes the auto raw deduction check default. It still keeps
the parsing value in there, since it's currently being used in
Chromium, but the intent is to remove it once this version rolls.

R=dcheng@chromium.org, thakis@chromium.org
BUG=554600

Review-Url: https://codereview.chromium.org/2732973004
Cr-Commit-Position: refs/heads/master@{#454982}
Committed:
https://chromium.googlesource.com/chromium/src/+/7e93859e576b0675f2fa3a4b29d0...
==========

[commit-bot: I haz the power, 2017-03-06 22:43:23]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/7e93859e576b0675f2fa3a4b29d0...