My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 151927: New Clang warning -Wtautological-constant-out-of-range-compare clean-up
1 person starred this issue and may be notified of changes. Back to list
 
Project Member Reported by h...@chromium.org, Sep 24, 2012
A new Clang warning fires in a couple of places. This was introduced in Clang r164143.

These are the warnings I get in a full debug build on Linux:

/usr/local/google/work/chrome/src/v8/src/utils.h:977:20: warning: comparison of constant 32 with expression of type 'v8::internal::AstPropertiesFlag' is always true [-Wtautological-constant-out-of-range-compare]
    ASSERT(element < static_cast<int>(sizeof(T) * CHAR_BIT));
    ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


/usr/local/google/work/chrome/src/third_party/libxml/src/xpath.c:12269:13: warning: comparison of constant 18 with expression of type 'xmlXPathTypeVal' is always false [-Wtautological-constant-out-of-range-compare]
                        if (type == XML_NAMESPACE_DECL)
                            ~~~~ ^  ~~~~~~~~~~~~~~~~~~


/usr/local/google/work/chrome/src/third_party/protobuf/src/google/protobuf/descriptor.cc:2361:20: warning: comparison of constant 18446744073709551615 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare]
  if (name_dot_pos == string::npos) {
      ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~


/usr/local/google/work/chrome/src/third_party/protobuf/src/google/protobuf/compiler/command_line_interface.cc:894:22: warning: comparison of constant 18446744073709551615 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare]
      if (equals_pos == string::npos) {
          ~~~~~~~~~~ ^  ~~~~~~~~~~~~


/usr/local/google/work/chrome/src/third_party/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:662:43: warning: comparison of constant 4 with expression of type 'WebCore::ImageFrame::FrameDisposalMethod' is always false [-Wtautological-constant-out-of-range-compare]
        if (frame_reader->disposal_method == 4)
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~
Sep 24, 2012
#1 thakis@chromium.org
(Current testing is at 164036, so we won't get this until the next testing release)
Sep 25, 2012
#2 h...@chromium.org
V8 CL is here: https://codereview.chromium.org/10987016/
Protobuf patch uploaded here: https://code.google.com/p/protobuf/issues/detail?id=421
Status: Started
Owner: h...@chromium.org
Cc: -h...@chromium.org
Sep 25, 2012
#3 thakis@chromium.org
Hm, the v8 patch kinda suggests that this warning shouldn't fire in template instantiations…maybe that should at least be discussed on the clang list?

pliard@ has contributed patches to protobuf before, he probably knows how to get traction there. (He's in your timezone, too.)
Sep 25, 2012
#4 h...@chromium.org
WebKit patch: https://bugs.webkit.org/show_bug.cgi?id=97563

> Hm, the v8 patch kinda suggests that this warning shouldn't fire in template instantiations…maybe that should at least be discussed on the clang list?

I'll ping the list.

> pliard@ has contributed patches to protobuf before, he probably knows how to get traction there. (He's in your timezone, too.)

Thanks, will ping him.
Sep 26, 2012
#6 bugdroid1@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=158800

------------------------------------------------------------------------
r158800 | hans@chromium.org | 2012-09-26T14:35:59.807614Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/src/xpath.c?r1=158800&r2=158799&pathrev=158800
   M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libxml/README.chromium?r1=158800&r2=158799&pathrev=158800

Merge a libxml clang warning fix

This fixes the following warning:

/usr/local/google/work/chrome/src/third_party/libxml/src/xpath.c:12269:13: warning: comparison of constant 18 with expression of type 'xmlXPathTypeVal' is always false [-Wtautological-constant-out-of-range-compare]
                        if (type == XML_NAMESPACE_DECL)
                            ~~~~ ^  ~~~~~~~~~~~~~~~~~~

The upstream fix is at http://git.gnome.org/browse/libxml2/commit/?id=713434d2309da469d64b35e163ea6556dadccada

TEST=none
BUG=151927

Review URL: https://codereview.chromium.org/10984039
------------------------------------------------------------------------
Sep 27, 2012
#7 thakis@chromium.org
Another one:

../../third_party/nss/mozilla/nsprpub/pr/src/misc/prnetdb.c:2081:16: error: comparison of constant 3 with expression of type 'PRStatus' is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (rv == EAI_BADFLAGS && (hints.ai_flags & AI_ADDRCONFIG)) {
            ~~ ^  ~~~~~~~~~~~~
1 error generated.
Sep 27, 2012
#8 thakis@chromium.org
This is the fix for comment 7:

Index: mozilla/nsprpub/pr/src/misc/prnetdb.c
===================================================================
--- mozilla/nsprpub/pr/src/misc/prnetdb.c	(revision 158748)
+++ mozilla/nsprpub/pr/src/misc/prnetdb.c	(working copy)
@@ -2030,7 +2030,7 @@
 #endif
     {
         PRADDRINFO *res, hints;
-        PRStatus rv;
+        int rv;
 
         /*
          * we assume a RFC 2553 compliant getaddrinfo.  this may at some


rv is only used as result of GETADDRINFO(), which is declared further up in the file and returns int, not PRStatus.
Oct 1, 2012
#11 h...@chromium.org
The WebKit patch was landed in http://trac.webkit.org/changeset/129523, which has rolled in by now.
The V8 patch was landed in https://code.google.com/p/v8/source/detail?r=12621,
should hopefully roll in this week.
Oct 1, 2012
#12 bugdroid1@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=159462

------------------------------------------------------------------------
r159462 | thakis@chromium.org | 2012-10-01T08:41:04.571661Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=159462&r2=159461&pathrev=159462

Roll nss 158748:159459

Only change in that range:
r159459: Merge https://bugzilla.mozilla.org/show_bug.cgi?id=795213 to fix a warning.

BUG=151927
TBR=hans

Review URL: https://codereview.chromium.org/11019008
------------------------------------------------------------------------
Oct 1, 2012
#13 bugdroid1@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=159466

------------------------------------------------------------------------
r159466 | hans@chromium.org | 2012-10-01T08:54:12.357348Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/protobuf/README.chromium?r1=159466&r2=159465&pathrev=159466
   M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/protobuf/src/google/protobuf/descriptor.cc?r1=159466&r2=159465&pathrev=159466
   M http://src.chromium.org/viewvc/chrome/trunk/src/third_party/protobuf/src/google/protobuf/compiler/command_line_interface.cc?r1=159466&r2=159465&pathrev=159466

Protobuf: Cherry-pick upstream r427.

Use string::size_type instead of int for results of
string::find_first_of().

BUG=151927

Review URL: https://codereview.chromium.org/11012009
------------------------------------------------------------------------
Oct 2, 2012
#14 h...@chromium.org
Just built with ToT Clang this morning on Linux; seems all warnings are fixed.
Status: Fixed
Oct 13, 2012
#15 thakis@chromium.org
wtc had to roll back his nss roll, which sadly reverted my changes too. It looks like his revert didn't help, so I suppose it'll reland soon?
Status: Started
Cc: wtc@chromium.org
Oct 15, 2012
#16 thakis@chromium.org
(No comment was entered for this change.)
Blockedon: chromium:151692
Oct 17, 2012
#17 bugdroid1@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=162398

------------------------------------------------------------------------
r162398 | rsleevi@chromium.org | 2012-10-17T14:21:04.490563Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=162398&r2=162397&pathrev=162398

Update nss_revision to 159459

This updates NSS to 3.14 pre-release snapshot 2012-09-25, along with
https://bugzilla.mozilla.org/show_bug.cgi?id=795213 to fix a warning.

R=wtc
BUG=153281, 151692, 151927
TEST=none

Review URL: https://chromiumcodereview.appspot.com/11040061

Review URL: https://chromiumcodereview.appspot.com/11143026
------------------------------------------------------------------------
Oct 18, 2012
#18 thakis@chromium.org
A new warning popped up:

../../v8/src/ia32/assembler-ia32.cc:1504:24: warning: comparison of constant 16 with expression of type 'v8::internal::Condition' is always true [-Wtautological-constant-out-of-range-compare]
../../v8/src/ia32/assembler-ia32.cc:1536:27: warning: comparison of constant 16 with expression of type 'v8::internal::Condition' is always true [-Wtautological-constant-out-of-range-compare]

Code looks like this:

  ASSERT(0 <= cc && cc < 16);

Probably best to cast cc to int here?
Oct 18, 2012
#19 thakis@chromium.org
And another one:

../../third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBCursor.cpp:174:28:error: comparison of constant 9007199254740991 with expression of type 'long' is always false [-Werror,-Wtautological-constant-out-of-range-compare]

Oct 18, 2012
#20 thakis@chromium.org
    if (count < 1 || count > maxECMAScriptInteger) {

Oct 19, 2012
#22 thakis@chromium.org
A clang version with this warning is now active, so this should be easier to fix now.
Oct 22, 2012
#23 jsbell@chromium.org
Fix for the issue in #c20 / #c21 in webkit.org/b/100014
Labels: webkit-id-100014
Oct 22, 2012
#24 bugdroid1@chromium.org
https://bugs.webkit.org/show_bug.cgi?id=100014
Labels: -webkit-id-100014 WebKit-ID-100014-NEW
Oct 22, 2012
#25 bugdroid1@chromium.org
https://bugs.webkit.org/show_bug.cgi?id=100014
http://trac.webkit.org/changeset/132140
Labels: -WebKit-ID-100014-NEW WebKit-ID-100014-RESOLVED WebKit-Rev-132140
Oct 25, 2012
#27 wtc@chromium.org
This bug is no longer blocked on  bug 151692 .
Blockedon: -chromium:151692
Nov 2, 2012
#28 bugdroid1@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=165793

------------------------------------------------------------------------
r165793 | hans@chromium.org | 2012-11-02T23:09:07.349805Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chromeos/web_socket_proxy.cc?r1=165793&r2=165792&pathrev=165793
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/common.gypi?r1=165793&r2=165792&pathrev=165793

Clang: enable -Wtautological-constant-out-of-range-compare

This warning was implemented in Clang a couple of weeks ago,
and now our code base is clean enough to enable it.

BUG=151927


Review URL: https://chromiumcodereview.appspot.com/11366060
------------------------------------------------------------------------
Nov 2, 2012
#29 thakis@chromium.org
Thanks!
Status: Fixed
Nov 2, 2012
#30 thakis@chromium.org
Thanks!
Mar 9, 2013
#31 bugdroid1@chromium.org
(No comment was entered for this change.)
Labels: -Area-Build Build
Sign in to add a comment

Powered by Google Project Hosting