Bug 91312 - -Wconversion warning with += operator
Summary: -Wconversion warning with += operator
Status: RESOLVED DUPLICATE of bug 40752
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 8.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2019-07-31 16:45 UTC by Kostas Sotiropoulos
Modified: 2020-01-10 19:20 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kostas Sotiropoulos 2019-07-31 16:45:58 UTC

    
Comment 1 Kostas Sotiropoulos 2019-07-31 16:59:28 UTC
Hi,

When compiling the following code snippet with gcc 8.3.0 
with -Werror=conversion option:

#include <stdio.h>

#define MACRO 1

int main(void)
{
 unsigned char i;

 i += MACRO;
 return i;
}

the following warning which is treated as an error is reported:

test.c:3:15: error: conversion from ‘int’ to ‘unsigned char’ may change value [-Werror=conversion]
 #define MACRO 1
               ^
test.c:9:7: note: in expansion of macro ‘MACRO’
  i += MACRO;
       ^~~~~
cc1: some warnings being treated as errors

Should this happen?

From what is described about -Wconversion under the following link:

https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc.pdf I am not convinced
that even with casting the right hand of expression this problem should
still appear on this case. Please check this statement:

"Do not warn for explicit casts likeabs ((int) x)andui= (unsigned) -1, 
or if the value is not changed by the conversion like inabs(2.0)."

The only solution to this issue is to expand += and then cast to unsigned
char i.e. i = (unsigned char) (i + MACRO)? Should not we be more flexible
on cases of MACRO's even here that integer promotion takes place?

Thank you,
Kostas
Comment 2 Andrew Pinski 2019-07-31 17:04:59 UTC
THis is not a bug, In C, "i += MACRO;" is equivant to:
i = i + MACRO.
And since you are using a type smaller than int, it is prompted to int.

NOTE there might be another bug associated with this one.
Comment 3 Kostas Sotiropoulos 2019-07-31 20:51:37 UTC
(In reply to Andrew Pinski from comment #2)
> THis is not a bug, In C, "i += MACRO;" is equivant to:
> i = i + MACRO.
> And since you are using a type smaller than int, it is prompted to int.
> 
> NOTE there might be another bug associated with this one.

1)In order to remove the warning we should only do the following: i = (unsigned char)(i + MACRO)?

2)Why if we want to cast like this: i += (unsigned char)MACRO, i on the right side of expression is not also casted to unsigned char (I mean the second i of expanded expression i = i + MACRO?

3)What do you mean with your NOTE can you please elaborate a bit more?

4) If we expand and cast the expression we will probably have not so optimized produced code by the compiler, should we?

Thank you,
Kostas
Comment 4 Kostas Sotiropoulos 2019-08-02 21:23:22 UTC
Any comments on my questions?
Comment 5 Andrew Pinski 2019-08-02 21:26:15 UTC
(In reply to Kostas Sotiropoulos from comment #4)
> Any comments on my questions?

Yes go read the c standard about prompting to int here.
Comment 6 Eric Gallager 2019-08-04 16:15:51 UTC
(In reply to Andrew Pinski from comment #2)
> THis is not a bug, In C, "i += MACRO;" is equivant to:
> i = i + MACRO.
> And since you are using a type smaller than int, it is prompted to int.
> 
> NOTE there might be another bug associated with this one.

bug 40752?
Comment 7 Kostas Sotiropoulos 2019-08-05 15:11:53 UTC
(In reply to Andrew Pinski from comment #5)
> (In reply to Kostas Sotiropoulos from comment #4)
> > Any comments on my questions?
> 
> Yes go read the c standard about prompting to int here.

I had checked the standard from the following link:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf

I suppose that you mean the following two sections that cover our case:

6.5.16.2

A compound assignment of the form E1 op=E2 is equivalent to the simple assignment expression E1 = E1 op (E2), except that the lvalue E1
is evaluated only once, and with respect  to  an  indeterminately-sequenced  function  call,  the  operation  of  a  compound assignment  is  a  single  evaluation. 

6.3.1.1 Boolean, characters, and integers

2 The following may be used in an expression wherever an int or unsigned int
may be used:

—  An object  or  expression  with  an  integer  type  (other  than int
or unsigned int) whose  integer  conversion  rank  is  less  than  or  
equal  to  the  rank  of int and unsigned int.

—  A bit-field of type _Bool, int, signed int, or unsigned int.

If an int can  represent  all  values  of  the  original  type  
(as  restricted  by  the  width,  for  a bit-field),  the  value  is  
converted  to  an int; otherwise,  it  is  converted  to  an unsigned int. 
These  are  called  the integer promotions. All  other  types  are  
unchanged  by  the integer promotions.

So considering the above two sections, if we even cast 
i += (unsigned char) MACRO then this would be equal to
i = i + (unsigned char) MACRO.
 
But, what if I want to avoid this warning then the only way to get 
rid of it is the following way?

i = (unsigned char) (i + MACRO).

But we will lose on code optimization be this way, do not we? Especially
when we are talking about Embedded Systems software.

Please note that the same code snippet passes through clang compiler (clang version 3.8.1-24) without any warning when -Werror=conversion is used during compilation. I suppose that even this compiler follows C standard. In my 
opinion such kind of warnings are false positives that should not be reported from gcc compiler too.

Maybe a change of this kind on gcc compiler opens the Pandora's Box for other
things but someone should not avoid of defining MACROS and use them on compound
statements (+=, -=, *= etc) and sacrifice code optimization by expanding them.

Waiting for your comments.
Comment 8 Marc Glisse 2019-08-05 15:52:20 UTC
We know that the warning is not so useful as is, that's why it isn't part of Wall or Wextra, see the other bugs on the topic. It needs people with time and motivation to work on it.
Comment 9 Jason Merrill 2020-01-10 19:20:45 UTC
This is fixed by the patch for 40752.

*** This bug has been marked as a duplicate of bug 40752 ***