Refactor how clang warning flags are set.

Refactor how clang warning flags are set.

Previously, every gyp file that wanted to set clang warnings had to check
for clang==1 and then set cflags and xcode_settings.WARNING_CFLAGS. Factor
this out, so that targets only need to set clang_warning_flags for warnings
that apply to all platforms. (Per-platform flags still need to be set manually.)

This removes existing duplication from gyp files, and prevents adding more
duplication when trying to add the same warning flags for clang/win.

BUG=82385
R=hans@chromium.org, scottmg@chromium.org
TBR=various owners

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287092

[Nico, 2014-08-01 20:56:08]

Look at build/common.gypi and build/set_clang_warning_flags.gypi first, the rest
is mechanical.

There's a ton of this in 3rd-party repos that are depsed in for no good reason
as far as I can tell, but doing this there is blocked on getting this change in
first.

In addition to helping with Windows, it's also going to help with switching
chrome/android builds to always use clang for host (at least that's my plan).
For this to work, I only need to convert all host toolset targets.

[scottmg, 2014-08-01 21:00:14]

https://codereview.chromium.org/437543007/diff/110001/build/set_clang_warning...
File build/set_clang_warning_flags.gypi (right):

https://codereview.chromium.org/437543007/diff/110001/build/set_clang_warning...
build/set_clang_warning_flags.gypi:11: # warn instead!
Maybe consider calling this file set_clang_warnings_flags_third_party or
_non_chromium or something to encourage this.

[scottmg, 2014-08-01 21:00:38]

Oh, oops, it's already via common.gypi, ignore last comment.

lgtm

[Nico, 2014-08-01 21:08:50]

The CQ bit was checked by thakis@chromium.org

[Nico, 2014-08-01 21:11:42]

The CQ bit was unchecked by thakis@chromium.org

[Nico, 2014-08-01 21:11:53]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2014-08-01 21:12:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thakis@chromium.org/437543007/110001

[hans, 2014-08-01 21:23:29]

Nice! lgtm with comment

https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
File third_party/snappy/snappy.gyp (right):

https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
third_party/snappy/snappy.gyp:84: 'clang_warning_flags': [ 'Wno-return-type' ],
There's a dash missing here.

[Nico, 2014-08-01 21:25:06]

https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
File third_party/snappy/snappy.gyp (right):

https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
third_party/snappy/snappy.gyp:84: 'clang_warning_flags': [ 'Wno-return-type' ],
On 2014/08/01 21:23:29, hans wrote:
> There's a dash missing here.

Hm, so I guess if this still gets green runs, this flag isn't needed? I'll try
removing it.

[Nico, 2014-08-01 21:35:20]

https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
File third_party/snappy/snappy.gyp (right):

https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
third_party/snappy/snappy.gyp:84: 'clang_warning_flags': [ 'Wno-return-type' ],
On 2014/08/01 21:23:29, hans wrote:
> There's a dash missing here.

…looks like not bot builds this target. So I added the -, maybe someone uses
this target locally?

[Nico, 2014-08-01 21:44:36]

On 2014/08/01 21:35:20, Nico (away) wrote:
>
https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
> File third_party/snappy/snappy.gyp (right):
> 
>
https://codereview.chromium.org/437543007/diff/110001/third_party/snappy/snap...
> third_party/snappy/snappy.gyp:84: 'clang_warning_flags': [ 'Wno-return-type'
],
> On 2014/08/01 21:23:29, hans wrote:
> > There's a dash missing here.
> 
> …looks like not bot builds this target.

*no bot

[Nico, 2014-08-01 21:48:31]

Committed patchset #7 manually as r287092 (presubmit successful).

[hans, 2014-08-14 21:11:02]

https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi
File skia/skia_common.gypi (right):

https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi#n...
skia/skia_common.gypi:143: '-Wstring-conversion',
I just noticed this: it looks like this is adding a warning, not uppressing it.
How does that work?

[Nico, 2014-08-14 21:15:35]

https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi
File skia/skia_common.gypi (right):

https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi#n...
skia/skia_common.gypi:143: '-Wstring-conversion',
On 2014/08/14 21:11:02, hans wrote:
> I just noticed this: it looks like this is adding a warning, not uppressing
it.
> How does that work?

That looks wrong, it should've probably been clang_warning_flags_unset . Looks
like this isn't needed anymore, then?

[scottmg, 2014-08-14 21:18:10]

https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi
File skia/skia_common.gypi (right):

https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi#n...
skia/skia_common.gypi:143: '-Wstring-conversion',
On 2014/08/14 21:15:35, Nico (very away) wrote:
> On 2014/08/14 21:11:02, hans wrote:
> > I just noticed this: it looks like this is adding a warning, not uppressing
> it.
> > How does that work?
> 
> That looks wrong, it should've probably been clang_warning_flags_unset . Looks
> like this isn't needed anymore, then?

Looks like they're mostly gone
https://code.google.com/p/chromium/codesearch#search/&q=assert.*&&.*%...
but might depend on exact build config.

[hans, 2014-08-14 21:40:01]

On 2014/08/14 21:15:35, Nico (very away) wrote:
> https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi
> File skia/skia_common.gypi (right):
> 
>
https://codereview.chromium.org/437543007/diff/110001/skia/skia_common.gypi#n...
> skia/skia_common.gypi:143: '-Wstring-conversion',
> On 2014/08/14 21:11:02, hans wrote:
> > I just noticed this: it looks like this is adding a warning, not uppressing
> it.
> > How does that work?
> 
> That looks wrong, it should've probably been clang_warning_flags_unset . Looks
> like this isn't needed anymore, then?

I suspect something isn't right here (I'm trying to suppress another warning,
but it's not working).

I added -foo-bar here (and in skia/skia_chromium.gypi), re-gyp'ed and built the
"skia" target without any complaints. It seems like these flags aren't getting
used?