[Bug c/51294] New: spurious warning from -Wconversion in C and C++

[Bug c/51294] New: spurious warning from -Wconversion in C and C++

[tortoise_74 at yahoo dot co.uk]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

             Bug #: 51294
           Summary: spurious warning from -Wconversion in C and C++
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: tortoise_74@yahoo.co.uk


The code below produces a spurious warning when compiled with -Wconversion.
0 is a const so the compiler should be able to choose the correct type. This
works with gcc 4.1 on redhat 5 but not the latest snapshot of gcc 4.7 (which I
am trialing for its c++11 support). 
I do not have any intermediate compiler versions to test with.

spurious.cpp:

class foo
{
   char bar;

   foo(bool haveBar, char bar_):
      bar(haveBar?bar_:0)
   {
   }
};

##

test.sh
#!/bin/sh

gcc --version
gcc -Wconversion -c spurious.cpp

export LD_LIBRARY_PATH=/opt/gcc4.7/lib:$LD_LIBRARY_PATH
export PATH=/opt/gcc4.7/bin:$PATH
/opt/gcc4.7/bin/gcc --version
/opt/gcc4.7/bin/gcc -Werror -Wconversion -c spurious.cpp

##

brucea@:home/brucea/spurious>./test.sh
gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-51)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

gcc (GCC) 4.7.0 20111119 (experimental)
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

spurious.cpp: In constructor ‘foo::foo(bool, char)’:
spurious.cpp:8:25: error: conversion to ‘char’ from ‘int’ may alter its value
[-Werror=conversion]
cc1plus: all warnings being treated as errors

##

The workaround is either to disable the warning with -Wno-conversion or add
unecessary constants as below. I don't think this is acceptable.

class foo2
{
   static const char zero = 0;

   char bar;

   foo2(bool haveBar, char bar_):
      bar(haveBar?bar_:zero)
   {
   }
};

This is also a bug in the C front-end:

void foo(int haveBar, char bar_)
{
  char zuul = haveBar?bar_:0;
}

[Bug c/51294] spurious warning from -Wconversion in C and C++

[tortoise_74 at yahoo dot co.uk]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

--- Comment #1 from Bruce Adams <tortoise_74 at yahoo dot co.uk> 2011-11-24 11:35:24 UTC ---
Probably unncessary but the OS is RHEL5 on a 32bit machine.

brucea@:home/brucea/spurious>lsb_release -a
LSB Version:   
:core-4.0-ia32:core-4.0-noarch:graphics-4.0-ia32:graphics-4.0-noarch:printing-4.0-ia32:printing-4.0-noarch
Distributor ID: RedHatEnterpriseClient
Description:    Red Hat Enterprise Linux Client release 5.7 (Tikanga)
Release:        5.7
Codename:       Tikanga

[Bug c/51294] spurious warning from -Wconversion in C and C++

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-11-24 12:01:49 UTC ---
(In reply to comment #0)
> 0 is a const so the compiler should be able to choose the correct type.

0 is an int, so the compiler is required by the standard to make the
conditional expression (haveBar?bar_:0) have type int.

It shouldn't warn though, as the conversion from int to char won't alter the
value.

> The workaround is either to disable the warning with -Wno-conversion or add
> unecessary constants as below. I don't think this is acceptable.

Or (haveBar?bar_:(char)0)

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
                 CC|                            |manu at gcc dot gnu.org
            Summary|spurious warning from       |spurious warning from
                   |-Wconversion in C and C++   |-Wconversion in C and C++
                   |                            |in conditional expressions

--- Comment #3 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-11-24 15:46:16 UTC ---
(In reply to comment #2)
> (In reply to comment #0)
> > 0 is a const so the compiler should be able to choose the correct type.
> 
> 0 is an int, so the compiler is required by the standard to make the
> conditional expression (haveBar?bar_:0) have type int.
> 
> It shouldn't warn though, as the conversion from int to char won't alter the
> value.

This was implemented in this patch:

http://gcc.gnu.org/ml/gcc-patches/2009-08/msg00582.html

which was rejected. Perhaps a cut-down version that addresses only this issue
would be accepted. But perhaps no. Feel free to take it.

BTW, clang does not warn for this case, while it does if bar_ is an int:

pr51294.c:3:23: warning: implicit conversion loses integer precision: 'int' to
'char' [-Wconversion]
  char zuul = haveBar?bar_:0;
                     ~^~~~
1 warning generated.

It also points to the correct location of the issue, whereas GCC doesn't.

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[tortoise_74 at yahoo dot co.uk]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

--- Comment #4 from Bruce Adams <tortoise_74 at yahoo dot co.uk> 2011-11-24 17:09:56 UTC ---
Shouldn't integral conversion rules apply if the types of the second and third
arguments to a conditional expression differ.
So zero should be converted from the default int to a char as presumably the
older version of gcc did. 
Perhaps a language lawyer could explain why this is or isn't a bug.
Though obviously warnings are not covered by the standard.


Note:

(haveBar?bar_:(char)0)

is not an acceptable workaround for C++ if -Wold-style-cast is used (which is
in my experience typical). It would have to be

(haveBar?bar_:static_cast<char>(0))

which is a notch higher in annoyingness.

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-11-24 18:37:31 UTC ---
(In reply to comment #4)
> Shouldn't integral conversion rules apply if the types of the second and third
> arguments to a conditional expression differ.

Yes.

> So zero should be converted from the default int to a char

No, the char is converted to int.  Hence the warning.

> as presumably the
> older version of gcc did. 

Nope.

> Perhaps a language lawyer could explain why this is or isn't a bug.

I did ;)

> Though obviously warnings are not covered by the standard.
> 
> 
> Note:
> 
> (haveBar?bar_:(char)0)
> 
> is not an acceptable workaround for C++ if -Wold-style-cast is used (which is
> in my experience typical). It would have to be
> 
> (haveBar?bar_:static_cast<char>(0))
> 
> which is a notch higher in annoyingness.

OK then:

  (haveBar?bar_:char())

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |james.kanze at gmail dot
                   |                            |com

--- Comment #6 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-01-05 15:32:03 UTC ---
*** Bug 51755 has been marked as a duplicate of this bug. ***

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |DeusExSophismata at gmail
                   |                            |dot com

--- Comment #7 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-03-24 09:54:22 UTC ---
*** Bug 52697 has been marked as a duplicate of this bug. ***

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-03-26
                 CC|                            |paolo.carlini at oracle dot
                   |                            |com
     Ever Confirmed|0                           |1

--- Comment #8 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-03-26 10:15:11 UTC ---
Manuel, we should really make progress on this. I'll try to work on it, help
welcome.

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com
   Target Milestone|---                         |4.8.0

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-03-30 10:52:32 UTC ---
Let's try to finally make progress on this. Note: the locations have been
already fixed.

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
         AssignedTo|paolo.carlini at oracle dot |unassigned at gcc dot
                   |com                         |gnu.org

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

--- Comment #10 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-05-12 18:38:33 UTC ---
Latest patch by Paolo, who run out of time:

http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00861.html

BTW, I think the patch is not wrong, and if fixes some cases, then it is a step
forward.

I think a more complete fix would be something like:

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c     (revision 187427)
+++ gcc/c-family/c-common.c     (working copy)
@@ -2262,10 +2262,15 @@ conversion_warning (tree type, tree expr
   location_t loc = EXPR_LOC_OR_HERE (expr);

   if (!warn_conversion && !warn_sign_conversion)
     return;

+  STRIP_USELESS_TYPE_CONVERSION (expr);
+  /* Don't warn about unsigned char y = 0xff, x = (int) y;  */
+  expr = get_unwidened (expr, 0);
+  expr_type = TREE_TYPE (expr);
+
   switch (TREE_CODE (expr))
     {
     case EQ_EXPR:
     case NE_EXPR:
     case LE_EXPR:
@@ -2294,26 +2299,18 @@ conversion_warning (tree type, tree expr
                    type, expr_type);
       return;

     case COND_EXPR:
       {
-       /* In case of COND_EXPR, if both operands are constants or
-          COND_EXPR, then we do not care about the type of COND_EXPR,
-          only about the conversion of each operand.  */
-       tree op1 = TREE_OPERAND (expr, 1);
-       tree op2 = TREE_OPERAND (expr, 2);
-
-       if ((TREE_CODE (op1) == REAL_CST || TREE_CODE (op1) == INTEGER_CST
-            || TREE_CODE (op1) == COND_EXPR)
-           && (TREE_CODE (op2) == REAL_CST || TREE_CODE (op2) == INTEGER_CST
-               || TREE_CODE (op2) == COND_EXPR))
-         {
-           conversion_warning (type, op1);
-           conversion_warning (type, op2);
-           return;
-         }
-       /* Fall through.  */
+        /* In case of COND_EXPR, we do not care about the type of
+           COND_EXPR, only about the conversion of each operand.  */
+        tree op1 = TREE_OPERAND (expr, 1);
+        tree op2 = TREE_OPERAND (expr, 2);
+ 
+        conversion_warning (type, op1);
+        conversion_warning (type, op2);
+        return;
       }

     default: /* 'expr' is not a constant.  */
       if (unsafe_conversion_p (type, expr, true))
        warning_at (loc, OPT_Wconversion,


Totally untested, and it lacks testcases, ChangeLog, etc. Neither Paolo nor I
have time to pursue this, so someone new will have to step forward.

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

--- Comment #11 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-11-07 16:58:13 UTC ---
Author: manu
Date: Wed Nov  7 16:58:03 2012
New Revision: 193301

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193301
Log:
2012-11-07  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR c/51294
c-family/
    * c-common.c (conversion_warning): Handle conditional expressions.
testsuite/
    * c-c++-common/pr51294.c: New.

Added:
    trunk/gcc/testsuite/c-c++-common/pr51294.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/testsuite/ChangeLog

[Bug c/51294] spurious warning from -Wconversion in C and C++ in conditional expressions

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51294

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-11-07 17:07:15 UTC ---
FIXED in 4.8