clang/win: Build chromium code without -Wno-incompatible-pointer-types.

clang/win: Build without -Wno-incompatible-pointer-types.

It defines YY_USE_CONST when building mesa to fix this warning, disables
-Wincompatible-pointer-types for the other third-party libraries that
trigger it and fixes some underspecified dependencies in
third_party/libwebp.

BUG=504696
Committed: https://crrev.com/c98d446e81bcc18cf4241325c8719ccdc0b7abf3
Cr-Commit-Position: refs/heads/master@{#338227}

[Sam McNally, 2015-07-03 04:15:05]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2015-07-03 05:45:17]

sammc@chromium.org changed reviewers:
+ aboxhall@chromium.org, lambroslambrou@chromium.org, thakis@chromium.org

[Sam McNally, 2015-07-03 05:45:17]

aboxhall@chromium.org: Please review changes in third_party/iaccessible2

lambroslambrou@chromium.org: Please review changes in remoting

thakis@chromium.org: Please review changes in build

[Lambros, 2015-07-06 17:16:19]

remoting lgtm

[Nico, 2015-07-06 17:20:20]

Sorry, didn't see this. How many is "several"? In general, suppressing warnings
close to where they happen seems like a better long-term strategy, unless the
warning fires for many third-party projects.

https://codereview.chromium.org/1226583002/diff/20001/third_party/iaccessible...
File third_party/iaccessible2/BUILD.gn (right):

https://codereview.chromium.org/1226583002/diff/20001/third_party/iaccessible...
third_party/iaccessible2/BUILD.gn:32: configs += [
"//build/config/compiler:no_chromium_code" ]
This is already landing in https://codereview.chromium.org/1214163009/

[Sam McNally, 2015-07-07 00:36:35]

On 2015/07/06 17:20:20, Nico wrote:
> Sorry, didn't see this. How many is "several"? In general, suppressing
warnings
> close to where they happen seems like a better long-term strategy, unless the
> warning fires for many third-party projects.

It looks like it's just libwebp, zlib, mesa, libxml and libvpx:
FAILED: ninja -t msvc -e environment.x64 --
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo
/showIncludes /FC @obj/third_party/libwebp/dec/libwebp_dec.io.obj.rsp
/c ../../third_party/libwebp/dec/io.c
/Foobj/third_party/libwebp/dec/libwebp_dec.io.obj
/Fdobj/third_party/libwebp/libwebp_dec_c.pdb
In file included from ../../third_party/libwebp/dec/io.c:20:
../../third_party/libwebp/dec/../utils/utils.h(94,19) :  error: incompatible
pointer types passing 'uint32_t *' (aka 'unsigned int *') to parameter of type
'unsigned long *' [-Werror,-Wincompatible-pointer-types]
  _BitScanReverse(&first_set_bit, n);
                  ^~~~~~~~~~~~~~
d:\sammc\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.7.0\include\intrin.h(510,32)
:  note: passing argument to parameter '_Index' here
_BitScanReverse(unsigned long *_Index, unsigned long _Mask) {

FAILED: ninja -t msvc -e environment.x64 --
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo
/showIncludes /FC @obj/third_party/zlib/zlib.x86.obj.rsp /c
../../third_party/zlib/x86.c /Foobj/third_party/zlib/zlib.x86.obj
/Fdobj/third_party/zlib/zlib_c.pdb
../../third_party/zlib/x86.c(78,48) :  error: incompatible pointer types passing
'volatile int32_t *' (aka 'volatile int *') to parameter of type 'volatile long
*' [-Werror,-Wincompatible-pointer-types]
            switch (InterlockedCompareExchange(once_control, 2, 0)) {
                                               ^~~~~~~~~~~~
d:\depot_tools\win_toolchain\vs2013_files\win8sdk\Include\um\winnt.h(2739,50) : 
note: passing argument to parameter 'Destination' here
    _Inout_ _Interlocked_operand_ LONG volatile *Destination,
                                                 ^
../../third_party/zlib/x86.c(81,41) :  error: incompatible pointer types passing
'volatile int32_t *' (aka 'volatile int *') to parameter of type 'volatile long
*' [-Werror,-Wincompatible-pointer-types]
                    InterlockedExchange(once_control, 1);
                                        ^~~~~~~~~~~~
d:\depot_tools\win_toolchain\vs2013_files\win8sdk\Include\um\winnt.h(2712,50) : 
note: passing argument to parameter 'Target' here
    _Inout_ _Interlocked_operand_ LONG volatile *Target,
                                                 ^
2 errors generated.

FAILED: ninja -t msvc -e environment.x64 --
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo
/showIncludes /FC
@obj/third_party/mesa/src/chromium_gensrc/mesa/mesa.lex.yy.obj.rsp /c
../../third_party/mesa/src/chromium_gensrc/mesa/lex.yy.c
/Foobj/third_party/mesa/src/chromium_gensrc/mesa/mesa.lex.yy.obj
/Fdobj/third_party/mesa/mesa_c.pdb
src/src/mesa/program/program_lexer.l(502,30) :  error: passing 'const char *' to
parameter of type 'char *' discards qualifiers
[-Werror,-Wincompatible-pointer-types-discards-qualifiers]
   _mesa_program__scan_bytes(string,len,*scanner);
                             ^~~~~~
src/chromium_gensrc/mesa/lex.yy.c(3263,60) :  note: passing argument to
parameter 'yybytes'
here
YY_BUFFER_STATE _mesa_program__scan_bytes  (yyconst char * yybytes, int 
_yybytes_len , yyscan_t yyscanner)
                                                           ^
1 error generated.

FAILED: ninja -t msvc -e environment.x64 --
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo
/showIncludes /FC @obj/third_party/libxml/src/libxml2.threads.obj.rsp
/c ../../third_party/libxml/src/threads.c
/Foobj/third_party/libxml/src/libxml2.threads.obj
/Fdobj/third_party/libxml/libxml2_c.pdb
../../third_party/libxml/src/threads.c(461,43) :  error: incompatible pointer
types passing
'volatile LPCRITICAL_SECTION *' (aka 'struct _RTL_CRITICAL_SECTION *volatile *')
to parameter of type 'void *volatile *' [-Werror,-Wincompatible-pointer-types]
        InterlockedCompareExchangePointer(&global_init_lock, cs, NULL);
                                          ^~~~~~~~~~~~~~~~~
d:\depot_tools\win_toolchain\vs2013_files\win8sdk\Include\um\winnt.h(2801,43) : 
note: passing argument to parameter 'Destination' here
    _Interlocked_operand_ PVOID volatile *Destination,
                                          ^
1 error generated.

FAILED: ninja -t msvc -e environment.x64 --
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo
/showIncludes /FC
@obj/third_party/libvpx/source/libvpx/vp8/encoder/x86/libvpx_intrinsics_ssse3.quantize_ssse3.obj.rsp
/c ../../third_party/libvpx/source/libvpx/vp8/encoder/x86/quantize_ssse3.c
/Foobj/third_party/libvpx/source/libvpx/vp8/encoder/x86/libvpx_intrinsics_ssse3.quantize_ssse3.obj
/Fdobj/third_party/libvpx/libvpx_intrinsics_ssse3_c.pdb
../../third_party/libvpx/source/libvpx/vp8/encoder/x86/quantize_ssse3.c(21,19) :
 error: incompatible pointer types passing 'int *' to parameter of type
'unsigned long *' [-Werror,-Wincompatible-pointer-types]
  _BitScanReverse(&eob, mask);
                  ^~~~
d:\sammc\chrome\src\third_party\llvm-build\Release+Asserts\bin\..\lib\clang\3.7.0\include\intrin.h(510,32)
:  note: passing argument to parameter '_Index' here
_BitScanReverse(unsigned long *_Index, unsigned long _Mask) {
                               ^
1 error generated.

[Sam McNally, 2015-07-07 08:18:58]

sammc@chromium.org changed reviewers:
+ agl@chromium.org, cpu@chromium.org, fbarchard@chromium.org, kbr@chromium.org

[Sam McNally, 2015-07-07 08:18:59]

cpu@chromium.org: Please review changes in third_party/libxml

agl@chromium.org: Please review changes in third_party/zlib

fbarchard@chromium.org: Please review changes in third_party/libwebp

kbr@chromium.org: Please review changes in third_party/mesa

[agl, 2015-07-07 17:29:21]

lgtm

[Ken Russell (switch to Gerrit), 2015-07-07 17:44:32]

third_party/mesa LGTM

[cpu_(ooo_6.6-7.5), 2015-07-07 18:01:20]

libxml lgtm

[Nico, 2015-07-07 18:25:11]

build/ and all of third_party/ lgtm. Bonus points for adding short descriptions
to the flags like you did in in libxml.

https://codereview.chromium.org/1226583002/diff/40001/third_party/libwebp/BUI...
File third_party/libwebp/BUILD.gn (right):

https://codereview.chromium.org/1226583002/diff/40001/third_party/libwebp/BUI...
third_party/libwebp/BUILD.gn:189: }
Can you add a short comment about this? Also, since webp is "kind of" chromium
code, maybe file an upstream issue and link to it (in the gyp file too)

https://codereview.chromium.org/1226583002/diff/40001/third_party/mesa/mesa.gyp
File third_party/mesa/mesa.gyp (right):

https://codereview.chromium.org/1226583002/diff/40001/third_party/mesa/mesa.g...
third_party/mesa/mesa.gyp:271: '-Wno-incompatible-pointer-types',
(mesa is usually very good about fixing warnings we file with them)

[Sam McNally, 2015-07-08 00:52:24]

https://codereview.chromium.org/1226583002/diff/40001/third_party/libwebp/BUI...
File third_party/libwebp/BUILD.gn (right):

https://codereview.chromium.org/1226583002/diff/40001/third_party/libwebp/BUI...
third_party/libwebp/BUILD.gn:189: }
On 2015/07/07 18:25:11, Nico wrote:
> Can you add a short comment about this? Also, since webp is "kind of" chromium
> code, maybe file an upstream issue and link to it (in the gyp file too)

Done.

https://codereview.chromium.org/1226583002/diff/40001/third_party/mesa/mesa.gyp
File third_party/mesa/mesa.gyp (right):

https://codereview.chromium.org/1226583002/diff/40001/third_party/mesa/mesa.g...
third_party/mesa/mesa.gyp:271: '-Wno-incompatible-pointer-types',
On 2015/07/07 18:25:11, Nico wrote:
> (mesa is usually very good about fixing warnings we file with them)

Looking into this a bit further, this warning is due to yyconst being defined as
const only if being compiled as C++ or C99. Defining YY_USE_CONST also fixes
this. See also
http://cgit.freedesktop.org/mesa/mesa/commit/?id=015e8b6384bbdba8421c5dafd478...

[Nico, 2015-07-08 03:18:50]

lgtm++

[Nico, 2015-07-08 03:19:00]

The CQ bit was checked by thakis@chromium.org

[Nico, 2015-07-08 03:19:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
lambroslambrou@chromium.org, kbr@chromium.org, cpu@chromium.org,
agl@chromium.org
Link to the patchset: https://codereview.chromium.org/1226583002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-07-08 03:19:52]

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

[Sam McNally, 2015-07-08 03:20:36]

The CQ bit was unchecked by sammc@chromium.org

[Sam McNally, 2015-07-08 03:29:22]

On 2015/07/08 03:19:52, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1226583002/60001

This still needs a libvpx change:
https://chromium-review.googlesource.com/#/c/283979/.

[Sam McNally, 2015-07-10 00:23:41]

Patchset #4 (id:80001) has been deleted

[Sam McNally, 2015-07-10 02:47:16]

The CQ bit was checked by sammc@chromium.org

[Sam McNally, 2015-07-10 02:47:17]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
cpu@chromium.org, lambroslambrou@chromium.org, thakis@chromium.org,
agl@chromium.org
Link to the patchset: https://codereview.chromium.org/1226583002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2015-07-10 02:47:42]

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

[commit-bot: I haz the power, 2015-07-10 02:51:42]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-07-10 02:52:32]

Patchset 4 (id:??) landed as
https://crrev.com/c98d446e81bcc18cf4241325c8719ccdc0b7abf3
Cr-Commit-Position: refs/heads/master@{#338227}

[fbarchard, 2015-07-10 19:26:42]

fyi I rolled libyuv against this revision and it worked ok.

https://codereview.chromium.org/1226583002/diff/100001/third_party/libwebp/li...
File third_party/libwebp/libwebp.gyp (right):

https://codereview.chromium.org/1226583002/diff/100001/third_party/libwebp/li...
third_party/libwebp/libwebp.gyp:37: 'libwebp_utils',
pulling in utilities as a dependency seems like overkill.  would be better to
apply the option to the target, or rely on common.gpyi.

https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib.gyp
File third_party/zlib/zlib.gyp (right):

https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib....
third_party/zlib/zlib.gyp:86: '-Wno-incompatible-pointer-types',
Does this option not work on arm/mips?

[Nico, 2015-07-10 19:28:38]

https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib.gyp
File third_party/zlib/zlib.gyp (right):

https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib....
third_party/zlib/zlib.gyp:86: '-Wno-incompatible-pointer-types',
On 2015/07/10 19:26:42, fbarchard wrote:
> Does this option not work on arm/mips?

It should work everywhere where we use chromium's clang to compile.

[fbarchard, 2015-07-10 19:50:30]

On 2015/07/10 19:28:38, Nico wrote:
>
https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib.gyp
> File third_party/zlib/zlib.gyp (right):
> 
>
https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib....
> third_party/zlib/zlib.gyp:86: '-Wno-incompatible-pointer-types',
> On 2015/07/10 19:26:42, fbarchard wrote:
> > Does this option not work on arm/mips?
> 
> It should work everywhere where we use chromium's clang to compile.

So it fails on xcode?
Shouldnt this be enabled for other configs?  e.g. chromeos?
I'd prefer just have it enabled for clang everywhere, so warnings dont trickle
in.

[Nico, 2015-07-10 19:53:41]

On Fri, Jul 10, 2015 at 12:50 PM, <fbarchard@chromium.org> wrote:

> On 2015/07/10 19:28:38, Nico wrote:
>
>
>
https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib.gyp
>
>> File third_party/zlib/zlib.gyp (right):
>>
>
>
>
>
https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib....
>
>> third_party/zlib/zlib.gyp:86: '-Wno-incompatible-pointer-types',
>> On 2015/07/10 19:26:42, fbarchard wrote:
>> > Does this option not work on arm/mips?
>>
>
>  It should work everywhere where we use chromium's clang to compile.
>>
>
> So it fails on xcode?
>

Not if xcode uses chromium's clang to build, which it can. But the iOS
build has toggles for using either xcode's clang or chromium's clang, and
it'd fail there.

Actually, this flag is probably old enough that it works in xcode's clang
too.


> Shouldnt this be enabled for other configs?  e.g. chromeos? I'd prefer
> just have it enabled for clang everywhere, so warnings dont trickle

in.
>

Yes, that's a good suggestion.


>
>
>
> https://codereview.chromium.org/1226583002/
>

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

[Sam McNally, 2015-07-13 08:00:26]

https://codereview.chromium.org/1226583002/diff/100001/third_party/libwebp/li...
File third_party/libwebp/libwebp.gyp (right):

https://codereview.chromium.org/1226583002/diff/100001/third_party/libwebp/li...
third_party/libwebp/libwebp.gyp:37: 'libwebp_utils',
On 2015/07/10 19:26:42, fbarchard wrote:
> pulling in utilities as a dependency seems like overkill.  would be better to
> apply the option to the target, or rely on common.gpyi.

demux.c includes utils/utils.h so it should depend on the target containing it.
libwebp_utils probably should list its headers in sources.

https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib.gyp
File third_party/zlib/zlib.gyp (right):

https://codereview.chromium.org/1226583002/diff/100001/third_party/zlib/zlib....
third_party/zlib/zlib.gyp:86: '-Wno-incompatible-pointer-types',
On 2015/07/10 19:26:42, fbarchard wrote:
> Does this option not work on arm/mips?

This is disabling a warning that is only triggered by x86.c in Windows-specific
code.

[dmazzoni, 2015-07-13 16:15:03]

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

[dmazzoni, 2015-07-13 16:15:04]

lgtm for third_party/iaccessible2