Whats going on with the conversion warning?

Whats going on with the conversion warning?

[Andy H]

I came across this odd issue with testsuite test Wconversion-5.c and AVR 
target.
I should get warning going from a unsigned value that is wider than 
signed result.

As I am not skilled in the art of the all conversions rules. I would 
appreciate some guidance before I report this as bug.

FYI AVR has 16 bit int, 16 bit short  int and 32 bit long.

I extracted the problematic line with a few variants and compiled -O0 
and -Wconversion,

void foo(void)
{
   signed char sc;
   signed char xi;
 
  xi =  (int) (unsigned short int) sc;    /* testcase NO WARNING - think 
this is bug*/
  xi =  (unsigned short int) sc;   /* NO WARNING - think this is bug*/
  xi =  (long) (unsigned short int) sc;    /* warning: conversion to 
'signed char' from 'short unsigned int' may alter its value - correct*/
  xi =  (long) ( short int) sc;    /* NO WARNING - correct */
  }

It would seem Wconversion:want to see 32bit result before it gives warning.
That can't be right - can it?

best regards

Re: Whats going on with the conversion warning?

[Manuel López-Ibáñez]

2008/5/20 Andy H <hutchinsonandy@aim.com>:
>
> I came across this odd issue with testsuite test Wconversion-5.c and AVR
> target.
> I should get warning going from a unsigned value that is wider than signed
> result.
>

Yes. You should also get a warning from a unsigned value converted to
a same-width signed type.

> void foo(void)
> {
>  signed char sc;
>  signed char xi;
>
>  xi =  (int) (unsigned short int) sc;    /* testcase NO WARNING - think this
> is bug*/

I may be wrong but I think (unsigned short int) sc is zero-extended,
then (int)(unsigned short int) sc is again zero extended. This means
that the complete conversion results in an integer value that when
converted to signed char gives back the original signed char. So the
assignment is actually equivalent to xi = sc. Ergo, no conversion
warning.

>  xi =  (unsigned short int) sc;   /* NO WARNING - think this is bug*/

The same applies here. Zero-extending to a wider type and then
conversion to the original type does not change the value. So now
warning. (That is, Wconversion can see whether the casts actually
affect the result or not.)

So I think this is not a bug. There are bugs in Wconversion, nonetheless.

http://gcc.gnu.org/PR35635
http://gcc.gnu.org/PR35701
http://gcc.gnu.org/PR34389
http://gcc.gnu.org/PR35852

Cheers,

Manuel.

Re: Whats going on with the conversion warning?

[Andy H]

Thanks for explanation and help

But this leave me with the conclusion that one of the following must be 
wrong:

signed char xi;

 xi =  (int) (unsigned short int) sc;    /* testcase NO WARNING - think 
this is bug*/
 xi =  (long) (unsigned short int) sc;    /* warning: conversion to 
'signed char' from 'short unsigned int' may alter its value - correct*/

Following your logic, (long).... appears to be wrong. Yet for i686 the 
first case  (int)..... generates PASS with expected warning and that 
would seem similar.

So  I'm still completely stuck knowing how I can patch testcase 
correctly for AVR or  post a bug.

best regards
Andy


Manuel López-Ibáñez wrote:
> 2008/5/20 Andy H <hutchinsonandy@aim.com>:
>   
>> I came across this odd issue with testsuite test Wconversion-5.c and AVR
>> target.
>> I should get warning going from a unsigned value that is wider than signed
>> result.
>>
>>     
>
> Yes. You should also get a warning from a unsigned value converted to
> a same-width signed type.
>
>   
>> void foo(void)
>> {
>>  signed char sc;
>>  signed char xi;
>>
>>  xi =  (int) (unsigned short int) sc;    /* testcase NO WARNING - think this
>> is bug*/
>>     
>
> I may be wrong but I think (unsigned short int) sc is zero-extended,
> then (int)(unsigned short int) sc is again zero extended. This means
> that the complete conversion results in an integer value that when
> converted to signed char gives back the original signed char. So the
> assignment is actually equivalent to xi = sc. Ergo, no conversion
> warning.
>
>   
>>  xi =  (unsigned short int) sc;   /* NO WARNING - think this is bug*/
>>     
>
> The same applies here. Zero-extending to a wider type and then
> conversion to the original type does not change the value. So now
> warning. (That is, Wconversion can see whether the casts actually
> affect the result or not.)
>
> So I think this is not a bug. There are bugs in Wconversion, nonetheless.
>
> http://gcc.gnu.org/PR35635
> http://gcc.gnu.org/PR35701
> http://gcc.gnu.org/PR34389
> http://gcc.gnu.org/PR35852
>
> Cheers,
>
> Manuel.
>   

Re: Whats going on with the conversion warning?

[Manuel López-Ibáñez]

2008/5/21 Andy H <hutchinsonandy@aim.com>:
> Thanks for explanation and help
>
> But this leave me with the conclusion that one of the following must be
> wrong:
>
> signed char xi;
>
> xi =  (int) (unsigned short int) sc;    /* testcase NO WARNING - think this
> is bug*/
> xi =  (long) (unsigned short int) sc;    /* warning: conversion to 'signed
> char' from 'short unsigned int' may alter its value - correct*/
>
> Following your logic, (long).... appears to be wrong. Yet for i686 the first
> case  (int)..... generates PASS with expected warning and that would seem
> similar.

My logic may be wrong. There may be something I am missing here.
Although I tried all possible values of signed char and both sc and xi
appear to end up with the same value (no matter whether you use int or
long, since by the text of the warning, it seems it is dropped
anyway).

I would call it a bug, perhaps it is one of the bugs I mentioned
before. To know what is really going on you would have to step in
c-common.c (conversion_warning) and see why the cast is not dropped.
Perhaps get_unwidened() is not the function that should be used there.
Perhaps we should use get_narrower() somehow. Or perhaps the whole
expression has been transformed and folded into something different
before it reaches that point.

I am sorry but I don't have time to look deeply into this. Please,
open a bug report and add a testcase (better if it can be tested in
i686). I hope somebody else can take a look at it.

Thanks,

Manuel.