[Bug c/34389] New: -Wconversion produces wrong warning

[Bug c/34389] New: -Wconversion produces wrong warning

[Raimund dot Merkert at baesystems dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

If compiled with gcc -c -Wconversion the following code produces the warning:
test.c:7: warning: conversion to ‘short int’ from ‘int’ may alter its value

but if -DNO_WARNING is turned on it doesn't even though it should be same code.

cat > test.c <<EOF
short  mask(short x)
{
#if NO_WARNING
  short y = 0x7fff;
  return x&y;
#else
  return x & (short)0x7fff;
#endif
}
EOF


On a side node, I think it would be better to also change the warning to :
"conversion from 'int' to 'short int' may alter its value", which i find easier
 to read.


-- 
           Summary: -Wconversion produces wrong warning
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: Raimund dot Merkert at baesystems dot com
GCC target triplet: gcc (GCC) 4.3.0 20071130 (experimental)


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #1 from manu at gcc dot gnu dot org  2007-12-13 21:38 -------
Confirmed. 

When the expression "x & (short)0x7fff" reaches the warning code, it sees
"(int)x & 0x7fff". So it rightfully warns that an "int" expression is converted
to "short". This comes from conversions performed in build_binary_op. Not sure
what the appropriate fix would be...


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-12-13 21:38:26
               date|                            |


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

[Bug c/34389] -Wconversion produces wrong warning

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2007-12-13 21:53 -------
I think this is related to PR 32643.


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #3 from manu at gcc dot gnu dot org  2007-12-17 11:57 -------
(In reply to comment #2)
> I think this is related to PR 32643.
> 

How is it related? There is no overflow here, is there? It is related in the
sense that build_binary_op is playing tricks with bitwise AND ?


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #4 from manu at gcc dot gnu dot org  2008-01-07 10:40 -------
@Andrew,

I would like to fix this before GCC 4.3 is released but I just don't know how.
Any hints?


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #5 from manu at gcc dot gnu dot org  2008-01-23 18:00 -------
I don't think this will be fixed before GCC 4.3 is released, which is a shame.
But if anyone has a suggestion on how to fix this, I will try to submit a patch
ASAP.


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #6 from manu at gcc dot gnu dot org  2008-02-15 16:34 -------
Some more findings:

build_binary_op receives two parameters of type int: '(int)x' and '0x7fff'
then it performs the shorten magic that seems right and produces:

<bit_and_expr 
   type <integer_type short int 
    arg 0 <parm_decl x type <integer_type short int>
    arg 1 <integer_cst type <integer_type short int> constant invariant 32767>>

which looks good. But just before returning, it converts that to 'int', what
results in:

<bit_and_expr 
    type <integer_type int
  arg 0 <nop_expr type <integer_type int>
        arg 0 <parm_decl x type <integer_type short int>>
    arg 1 <integer_cst type <integer_type int>>

which causes problems later. If instead it returned:

<nop_expr type <integer_type int>
  <bit_and_expr 
     type <integer_type short int 
      arg 0 <parm_decl x type <integer_type short int>
      arg 1 <integer_cst type <integer_type short int> constant invariant>>

No warning would be generated!

Why bother with the shorten magic if convert is going to override it? Where it
is overriding it?

The current code converts the constant back and forth between short int and int
at least 3 times.

Any ideas on how to stop this madness?


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #7 from manu at gcc dot gnu dot org  2008-02-15 16:53 -------
So fold_unary is transforming (T)(x & c) into (T)x & (T)c. Which is exactly the
opposite transformation that build_binary_op just performed! Weird...

So I see two options:

* Either teach fold_unary to avoid this particular transformation in this
particular case.

* Hack conversion_warning to realize that (short)((int)x & c) is the same as
(short)(x & c) if x is signed int and c fits in a signed int. Which will be
undoing the transformation once more (fourth? fifth time?). And I am not even
sure how to do this.


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #8 from manu at gcc dot gnu dot org  2008-02-15 18:04 -------
This patch fixes this bug but it is an ugly hack. Perhaps such an ugly hack is
the only thing we can do at the moment.

Once bootstrapping + testing finishes, I will submit.

Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c      (revision 132310)
+++ gcc/c-common.c      (working copy)
@@ -1270,65 +1270,98 @@ conversion_warning (tree type, tree expr
                  "conversion to %qT alters %qT constant value",
                  type, TREE_TYPE (expr));
     }
   else /* 'expr' is not a constant.  */
     {
+      tree expr_type = TREE_TYPE (expr);
+
       /* Warn for real types converted to integer types.  */
-      if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE
+      if (TREE_CODE (expr_type) == REAL_TYPE
           && TREE_CODE (type) == INTEGER_TYPE)
         give_warning = true;

-      else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE
+      else if (TREE_CODE (expr_type) == INTEGER_TYPE
                && TREE_CODE (type) == INTEGER_TYPE)
         {
          /* Don't warn about unsigned char y = 0xff, x = (int) y;  */
          expr = get_unwidened (expr, 0);
+         expr_type = TREE_TYPE (expr);

+         /* Don't warn for short y; short x = ((int)y & 0xff);  */
+         if (TREE_CODE (expr) == BIT_AND_EXPR
+             && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST)
+           {
+             int unsigned0, unsigned1;
+             tree arg0, arg1;
+             tree tmp_type;
+             tree op0, op1;
+
+             op0 = convert (expr_type, TREE_OPERAND (expr, 0));
+             op1 = convert (expr_type, TREE_OPERAND (expr, 1));
+
+             arg0 = get_narrower (op0, &unsigned0);
+             arg1 = get_narrower (op1, &unsigned1);
+
+             /* Handle the case that OP0 (or OP1) does not *contain* a
conversion
+                but it *requires* conversion to TYPE.  */
+             if ((TYPE_PRECISION (TREE_TYPE (op0))
+                  == TYPE_PRECISION (TREE_TYPE (arg0)))
+                 && TREE_TYPE (op0) != expr_type)
+               unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0));
+
+             if (TYPE_PRECISION (TREE_TYPE (arg0)) < TYPE_PRECISION
(expr_type)
+                 && (tmp_type
+                     = c_common_signed_or_unsigned_type (unsigned0,
+                                                          TREE_TYPE (arg0)))
+                 && !POINTER_TYPE_P (tmp_type)
+                 && int_fits_type_p (arg1, tmp_type))
+               expr_type = tmp_type;
+           }
           /* Warn for integer types converted to smaller integer types.  */
-          if (formal_prec < TYPE_PRECISION (TREE_TYPE (expr)))
+          if (formal_prec < TYPE_PRECISION (expr_type))
            give_warning = true;

          /* When they are the same width but different signedness,
             then the value may change.  */
-         else if ((formal_prec == TYPE_PRECISION (TREE_TYPE (expr))
-                   && TYPE_UNSIGNED (TREE_TYPE (expr)) != TYPE_UNSIGNED
(type))
+         else if ((formal_prec == TYPE_PRECISION (expr_type)
+                   && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type))
                   /* Even when converted to a bigger type, if the type is
                      unsigned but expr is signed, then negative values
                      will be changed.  */
-                  || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (TREE_TYPE
(expr))))
+                  || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type)))
            warning (OPT_Wsign_conversion,
                     "conversion to %qT from %qT may change the sign of the
result",
-                    type, TREE_TYPE (expr));
+                    type, expr_type);
         }

       /* Warn for integer types converted to real types if and only if
          all the range of values of the integer type cannot be
          represented by the real type.  */
-      else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE
+      else if (TREE_CODE (expr_type) == INTEGER_TYPE
                && TREE_CODE (type) == REAL_TYPE)
         {
-          tree type_low_bound = TYPE_MIN_VALUE (TREE_TYPE (expr));
-          tree type_high_bound = TYPE_MAX_VALUE (TREE_TYPE (expr));
+          tree type_low_bound = TYPE_MIN_VALUE (expr_type);
+          tree type_high_bound = TYPE_MAX_VALUE (expr_type);
           REAL_VALUE_TYPE real_low_bound = real_value_from_int_cst (0,
type_low_bound);
           REAL_VALUE_TYPE real_high_bound = real_value_from_int_cst (0,
type_high_bound);

           if (!exact_real_truncate (TYPE_MODE (type), &real_low_bound)
               || !exact_real_truncate (TYPE_MODE (type), &real_high_bound))
             give_warning = true;
         }

       /* Warn for real types converted to smaller real types.  */
-      else if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE
+      else if (TREE_CODE (expr_type) == REAL_TYPE
                && TREE_CODE (type) == REAL_TYPE
-               && formal_prec < TYPE_PRECISION (TREE_TYPE (expr)))
+               && formal_prec < TYPE_PRECISION (expr_type))
         give_warning = true;


       if (give_warning)
         warning (OPT_Wconversion,
                  "conversion to %qT from %qT may alter its value",
-                 type, TREE_TYPE (expr));
+                 type, expr_type);
     }
 }

 /* Produce warnings after a conversion. RESULT is the result of
    converting EXPR to TYPE.  This is a helper function for


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2007-12-13 21:38:26         |2008-02-15 18:04:44
               date|                            |


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

[Bug c/34389] -Wconversion produces wrong warning

[rainy6144 at gmail dot com]

------- Comment #9 from rainy6144 at gmail dot com  2008-04-23 07:20 -------
Does the patch also fix the warning for conditional expressions?  For example:

short f(int cond, short x, short y)
{
  return cond ? x : y;
}


-- 

rainy6144 at gmail dot com changed:

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


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #10 from manu at gcc dot gnu dot org  2008-06-08 17:30 -------
(In reply to comment #9)
> Does the patch also fix the warning for conditional expressions?  For example:
> 
> short f(int cond, short x, short y)
> {
>   return cond ? x : y;
> }
> 

No, that is a completely different issue.


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #11 from manu at gcc dot gnu dot org  2008-07-30 08:31 -------
Subject: Bug 34389

Author: manu
Date: Wed Jul 30 08:30:32 2008
New Revision: 138296

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=138296
Log:
2008-07-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

        PR 34389
        * c-typeck.c (build_binary_op): Encapsulate code into...
        * c-common.c (shorten_binary_op): ...this new function.
        (conversion_warning): Use the new function. Handle non-negative
        constant in bitwise-and.
        * c-common.h (shorten_binary_op): Declare.
cp/
        * typeck.c (build_binary_op): Encapsulate code into
        shorten_binary_op.
testsuite/
        * gcc.dg/Wconversion-pr34389.c: New.
        * g++.dg/warn/Wconversion-pr34389.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C
    trunk/gcc/testsuite/gcc.dg/Wconversion-pr34389.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-common.c
    trunk/gcc/c-common.h
    trunk/gcc/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #12 from manu at gcc dot gnu dot org  2008-07-30 08:36 -------
This should be mostly fixed except the following testcase in C++:

short  mask1(short x)
{
  short y = 0x7fff;
  return x & y; /* { dg-bogus "conversion" "conversion" { xfail *-*-* } 8 } */
}

This works in C, so it seems "same_type" does different things in C and in C++.
I don't have time to investigate this at the moment so any help is appreciated.


-- 


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

[Bug c/34389] -Wconversion produces wrong warning

[manu at gcc dot gnu dot org]

------- Comment #13 from manu at gcc dot gnu dot org  2008-08-01 19:36 -------
I created PR 37004 to cover the issue with C++ front-end. Apart from that, this
bug is FIXED in GCC 4.4.


-- 

manu at gcc dot gnu dot org changed:

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


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