Add the missing flags for clang's chromium-style plugin.

Add the missing flags for clang's chromium-style plugin.

Confirmed that when we enable them it correctly throw the following
error:

../tools/gn/group_target_generator.h:18:34: error: [chromium-style]
Overriding method must be marked with 'override' or 'final'.
  virtual ~GroupTargetGenerator();
                                   ^
                                                                     override

This patch was tested with the following command lines (after changing
BUILD.gn):

$ gn gen out-gn
$ ninja -v -C out-gn gn

And manually checked that the new flags were passed to clang.

See for example:
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_compile_dbg_ng/builds/27949

This came while doing https://codereview.chromium.org/964203002/, which
without this patch passed fine but hit on the GYP trybots.

BUG=462972
TEST=see above
R=dpranke@chromium.org

[Nico, 2015-03-02 03:35:47]

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

[Nico, 2015-03-02 03:35:47]

not lgtm, we shouldn't duplicate these flags. that makes things needlessly hard
for us.

if it helps gn to make that script a python script instead of a shell script, we
can do that

[tfarina, 2015-03-02 13:41:14]

Yes, changing to a Python script helps GN.

[Nico, 2015-03-02 18:44:29]

On 2015/03/02 13:41:14, tfarina wrote:
> Yes, changing to a Python script helps GN.

Feel free to do that, then :-)

[tfarina, 2015-03-02 20:41:07]

Done. https://codereview.chromium.org/966723003 ;)

[tfarina, 2015-03-04 20:37:03]

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

[tfarina, 2015-03-04 20:37:17]

Brett, ping? Could you approve this?

[Nico, 2015-03-04 20:39:57]

lgtm, given that the gyp build does this too soon

[brettw, 2015-03-04 20:58:40]

lgtm

[tfarina, 2015-03-04 21:19:39]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2015-03-04 21:20:58]

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

[Nico, 2015-03-04 21:42:52]

The CQ bit was unchecked by thakis@chromium.org

[Nico, 2015-03-04 21:43:15]

I'm unchecking cq for a bit since this collides with
https://codereview.chromium.org/980523002/ (should be easy to rebase though)

[tfarina, 2015-03-06 15:47:19]

After
https://codereview.chromium.org/980523002/diff/20001/tools/clang/scripts/plug...,
it made this patch not necessary anymore.

Closing this.

[Nico, 2015-03-06 15:53:41]

heh, so I guess this was incorrect because these flags are duplicated and
someone forgot to update the gn files.

How's the thing that automatically compares compile arguments coming along? :-/

[scottmg, 2015-03-06 17:38:21]

The tool's there (tools/gn/bin/gyp_flag_compare.py) but I think we have to
get the builds closer before it could be automatic? Maybe linux is close
enough now though that we could actually whitelist some things and then
make it a bot step. I'll run it again today and see how it looks.

On Fri, Mar 6, 2015 at 7:53 AM, <thakis@chromium.org> wrote:

> heh, so I guess this was incorrect because these flags are duplicated and
> someone forgot to update the gn files.
>
> How's the thing that automatically compares compile arguments coming
> along? :-/
>
>
> https://codereview.chromium.org/971593003/
>
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Dirk Pranke, 2015-03-06 18:46:36]

Yeah, I think we should start seriously looking at the output of that tool
now; it's been
on my to-do list for a week or so, along with too many other things.

-- Dirk

On Fri, Mar 6, 2015 at 9:38 AM, Scott Graham <scottmg@chromium.org> wrote:

> The tool's there (tools/gn/bin/gyp_flag_compare.py) but I think we have to
> get the builds closer before it could be automatic? Maybe linux is close
> enough now though that we could actually whitelist some things and then
> make it a bot step. I'll run it again today and see how it looks.
>
> On Fri, Mar 6, 2015 at 7:53 AM, <thakis@chromium.org> wrote:
>
>> heh, so I guess this was incorrect because these flags are duplicated and
>> someone forgot to update the gn files.
>>
>> How's the thing that automatically compares compile arguments coming
>> along? :-/
>>
>>
>> https://codereview.chromium.org/971593003/
>>
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to chromium-reviews+unsubscribe@chromium.org.
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[scottmg, 2015-03-06 21:37:33]

My gnat brain forgot that I already added it to the Linux GN bots, see e.g.

http://build.chromium.org/p/chromium.linux/builders/Linux%20GN/builds/24403/s...

"""
Total differences: 293832
"""

I'm not sure if they're close enough yet to switch that step to be
actionable. Some don't look too important, but they'd need to be audited.

I didn't do this yet, but we could pull that number out and track it like a
perf number so that the step will be red if it increases. We still have to
put in the work to decrease the number though, of course.


On Fri, Mar 6, 2015 at 10:46 AM, Dirk Pranke <dpranke@chromium.org> wrote:

> Yeah, I think we should start seriously looking at the output of that tool
> now; it's been
> on my to-do list for a week or so, along with too many other things.
>
> -- Dirk
>
>
> On Fri, Mar 6, 2015 at 9:38 AM, Scott Graham <scottmg@chromium.org> wrote:
>
>> The tool's there (tools/gn/bin/gyp_flag_compare.py) but I think we have
>> to get the builds closer before it could be automatic? Maybe linux is close
>> enough now though that we could actually whitelist some things and then
>> make it a bot step. I'll run it again today and see how it looks.
>>
>> On Fri, Mar 6, 2015 at 7:53 AM, <thakis@chromium.org> wrote:
>>
>>> heh, so I guess this was incorrect because these flags are duplicated and
>>> someone forgot to update the gn files.
>>>
>>> How's the thing that automatically compares compile arguments coming
>>> along? :-/
>>>
>>>
>>> https://codereview.chromium.org/971593003/
>>>
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to chromium-reviews+unsubscribe@chromium.org.
>>>
>>
>>
>  To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.