Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 151927 New Clang warning -Wtautological-constant-out-of-range-compare clean-up
Starred by 1 user Project Member Reported by h...@chromium.org, Sep 24, 2012 Back to list
Status: Fixed
Owner: h...@chromium.org
Closed: Nov 2012
Cc: wtc@chromium.org, tha...@chromium.org
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment
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)
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~
 
Comment 1 by tha...@chromium.org, Sep 25, 2012
(Current testing is at 164036, so we won't get this until the next testing release)
Comment 2 by h...@chromium.org, Sep 25, 2012
Cc: -h...@chromium.org
Owner: h...@chromium.org
Status: Started
V8 CL is here: https://codereview.chromium.org/10987016/
Protobuf patch uploaded here: https://code.google.com/p/protobuf/issues/detail?id=421
Comment 3 by tha...@chromium.org, Sep 25, 2012
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.)
Comment 4 by h...@chromium.org, Sep 25, 2012
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.
Project Member Comment 6 by bugdroid1@chromium.org, Sep 26, 2012
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
------------------------------------------------------------------------
Comment 7 by tha...@chromium.org, Sep 28, 2012
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.
Comment 8 by tha...@chromium.org, Sep 28, 2012
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.
Comment 11 by h...@chromium.org, Oct 1, 2012
The WebKit patch was landed in http://trac.webkit.org/changeset/129523, which has rolled in by now.
The V8 patch was landed in http://code.google.com/p/v8/source/detail?r=12621,
should hopefully roll in this week.
Project Member Comment 12 by bugdroid1@chromium.org, Oct 1, 2012
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
------------------------------------------------------------------------
Project Member Comment 13 by bugdroid1@chromium.org, Oct 1, 2012
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
------------------------------------------------------------------------
Comment 14 by h...@chromium.org, Oct 2, 2012
Status: Fixed
Just built with ToT Clang this morning on Linux; seems all warnings are fixed.
Comment 15 by tha...@chromium.org, Oct 14, 2012
Cc: wtc@chromium.org
Status: Started
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?
Comment 16 by tha...@chromium.org, Oct 15, 2012
Blockedon: chromium:151692
Project Member Comment 17 by bugdroid1@chromium.org, Oct 17, 2012
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
------------------------------------------------------------------------
Comment 18 by tha...@chromium.org, Oct 18, 2012
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?
Comment 19 by tha...@chromium.org, Oct 18, 2012
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]

Comment 20 by tha...@chromium.org, Oct 18, 2012
    if (count < 1 || count > maxECMAScriptInteger) {

Comment 22 by tha...@chromium.org, Oct 19, 2012
A clang version with this warning is now active, so this should be easier to fix now.
Comment 23 by jsb...@chromium.org, Oct 22, 2012
Labels: webkit-id-100014
Fix for the issue in #c20 / #c21 in webkit.org/b/100014
Project Member Comment 24 by bugdroid1@chromium.org, Oct 22, 2012
Labels: -webkit-id-100014 WebKit-ID-100014-NEW
https://bugs.webkit.org/show_bug.cgi?id=100014
Project Member Comment 25 by bugdroid1@chromium.org, Oct 22, 2012
Labels: -WebKit-ID-100014-NEW WebKit-ID-100014-RESOLVED WebKit-Rev-132140
https://bugs.webkit.org/show_bug.cgi?id=100014
http://trac.webkit.org/changeset/132140
Comment 26 by h...@chromium.org, Oct 23, 2012
V8 fix: http://code.google.com/p/v8/source/detail?r=12792 (not rolled in yet)
Comment 27 by wtc@chromium.org, Oct 26, 2012
Blockedon: -chromium:151692
This bug is no longer blocked on bug 151692.
Project Member Comment 28 by bugdroid1@chromium.org, Nov 2, 2012
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
------------------------------------------------------------------------
Thanks!
Status: Fixed
Thanks!
Project Member Comment 31 by bugdroid1@chromium.org, Mar 10, 2013
Labels: -Area-Build Build
Sign in to add a comment