New issue
Advanced search Search tips

Issue 777363 link

Starred by 1 user
Status: Fixed
Owner:
h...@chromium.org
Closed: Nov 2017
Cc:
thakis@chromium.org
r...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

ClangToTWin bots failing with -Wsign-compare

Project Member Reported by h...@chromium.org, Oct 23 2017 
For example from https://build.chromium.org/p/chromium.clang/builders/ToTWin/builds/191:

./../third_party/expat/files/lib/xmlparse.c(2429,24):  error: comparison of integers of different signs: 'enum XML_Error' and 'unsigned int' [-Werror,-Wsign-compare]
  if (code > 0 && code < sizeof(message)/sizeof(message[0]))
                  ~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Not sure if that's isolated or if there are lots of them.

This seems to be a side-effect of Clang r316268.

Comment 1 by h...@chromium.org, Oct 23 2017 

I think the behaviour only changed for Windows (because enums are signed there) and C code. So maybe it's not much more than that expat warning.

If it's just that one, maybe we can just add a cast and be done with it.

Comment 2 by h...@chromium.org, Oct 23 2017

Owner: h...@chromium.org
Status: Started (was: Available)
https://chromium-review.googlesource.com/c/chromium/src/+/733225 for the expat warning

I'm always through a full debug build with that one.

Comment 3 by h...@chromium.org, Oct 23 2017 

s/always/almost/
And now the build finished, so hopefully we're good after that lands.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7378f29a80837745d43076cb5b02a13587a72f30

commit 7378f29a80837745d43076cb5b02a13587a72f30
Author: Hans Wennborg <hans@chromium.org>
Date: Mon Oct 23 17:58:44 2017

Fix Win/Clang -Wsign-compare warning

enums are backed by signed integers by default on Windows, and Clang
(after r316268) would warn about comparing 'code', which is signed, with
sizeof(..) which is unsigned.

Bug:  777363 
Change-Id: I4f69d92e9f770a1a9011dbd8b76ffaea1fadc469
Reviewed-on: https://chromium-review.googlesource.com/733225
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510842}
[add] https://crrev.com/7378f29a80837745d43076cb5b02a13587a72f30/third_party/expat/0005-Fix-Win-Clang-Wsign-compare-warning.patch
[modify] https://crrev.com/7378f29a80837745d43076cb5b02a13587a72f30/third_party/expat/README.chromium
[modify] https://crrev.com/7378f29a80837745d43076cb5b02a13587a72f30/third_party/expat/files/lib/xmlparse.c

Comment 5 by h...@chromium.org, Oct 24 2017 

Sent a better fix to Expat here: https://github.com/libexpat/libexpat/pull/162

Comment 6 by h...@chromium.org, Nov 3 2017

Status: Fixed (was: Started)
►

Sign in to add a comment