[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589 John Downing changed: What|Removed |Added CC||jrdowning at yahoo dot com --- Comment #11 from John Downing --- I think I understand, but one more question. The "*" operator promotes the RHS to int, then it converted back to short. But you say the warning is a "false positive". So if it's a "false positive", then why isn't that false positive suppressed? I know that clang does not show an error for the exact same code, so clearly it is compliant with the C++ standard to do so.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589 --- Comment #10 from Jonathan Wakely --- No, you've misunderstood. It doesn't mean "can, if the compiler chooses to". There is no design decision involved. Types smaller than int *can* be promoted to int, and in certain contexts they *are* promoted to int. The compiler doesn't get to choose when that happens. The warning is a false positive because the range of possible values of the RHS is such that promotion to int and back to short cannot alter the value, but that doesn't change the fact that there *is* a promotion to int and then conversion back to short.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589
--- Comment #9 from John Downing ---
(In reply to Jason Merrill from comment #7)
> Rather, it was suppressed for the simple case where the RHS has a suitable
> type. As Jonathan says, (CHAR_BIT * static_cast(i)) has type int (it
> would be unsigned without the cast). And then the << expression has type
> int. And then the | expression has type int.
>
> Before the patch for 40752, we would warn based just on the type of the |
> expression. Now we also look at the type of the << expression, see that it
> is also int, and warn. You can avoid the warning by breaking up the
> expression more:
>
> #define CHAR_BIT 8
> void print_byte_order( short )
> {
>short val = 0;
>unsigned i = 1;
>short pat1 = (CHAR_BIT * static_cast(i));
>short pat2 = pat1 << (CHAR_BIT * static_cast(i));
>val |= pat2;
> }
>
> or adding another cast as in comment #1.
The code I posted has two casts, as in comment #1. When I compile code like
this:
#define CHAR_BIT 8
void print_byte_order_short( short )
{
short val = 0;
unsigned int i =1;
for(i = 1; i < sizeof(short); ++i)
{
short part1 = (CHAR_BIT * static_cast(i));
short part2 = part1 << CHAR_BIT * static_cast(i);
val |= part2;
}
cout << val;
}
I get this warning:
example2.cpp: In function void print_byte_order_short(short int):
example2.cpp:15:32: warning: conversion from int to short int may change value
[-Wconversion]
short part1 = (CHAR_BIT * static_cast(i));
~~^~~~
example2.cpp:16:27: warning: conversion from int to short int may change value
[-Wconversion]
short part2 = part1 << CHAR_BIT * static_cast(i);
~~^~~
Which seems like the same "design decision" I referenced in comment #8.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589 --- Comment #8 from John Downing --- > (In reply to John Downing from comment #5) > > This will generate a warning, for the char and short cases, when the "|=" > > happens. This is despite "val" being explicitly declared as a char or an > > short. > > It's not *despite* being declared as char or short, it's *because* they are > declare as char and short. This means that the "|=" operator or the "<<" operator changes the RHS side to an int? While that is within the guidelines of the cpp standard, the standard as referenced in this thread says "can" change type, not "must". So then why does this change happen? > > > I can see this being correct if "val" has been declared as "unsigned", which > > i shorthand for "unsigned int". But if "val" is explicitly declared as > > something, there should be a potential for the conversion to change the > > value. > > That's not true. Given: You are right, that's my opps - I meant to say "there should *no* potential". > > val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT * > static_cast(i)); > > The type of (CHAR_BIT * static_cast(i)) is int, due to: > https://en.cppreference.com/w/cpp/language/ > implicit_conversion#Integral_promotion It is this the reference which uses "can" not "must". > > And the type of the entire right-hand side is int, so you are doing val |= > int(some_value) which the compiler correctly says might not fit in a short > or char. In this specific case, your loop conditions mean that the value of > the right hand side will fit, but the warning doesn't know about the values, > it only cares that conversion from int to short int involves a potentially > lossy conversion. Yes, and with the standard using the word "can" to change the RHS to an int, it is a "design decision".
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589
Jason Merrill changed:
What|Removed |Added
CC||jason at gcc dot gnu.org
--- Comment #7 from Jason Merrill ---
(In reply to Jonathan Wakely from comment #6)
> Oops, I meant to say the warning was just suppressed for the += case, see PR
> 40752.
Rather, it was suppressed for the simple case where the RHS has a suitable
type. As Jonathan says, (CHAR_BIT * static_cast(i)) has type int (it
would be unsigned without the cast). And then the << expression has type int.
And then the | expression has type int.
Before the patch for 40752, we would warn based just on the type of the |
expression. Now we also look at the type of the << expression, see that it is
also int, and warn. You can avoid the warning by breaking up the expression
more:
#define CHAR_BIT 8
void print_byte_order( short )
{
short val = 0;
unsigned i = 1;
short pat1 = (CHAR_BIT * static_cast(i));
short pat2 = pat1 << (CHAR_BIT * static_cast(i));
val |= pat2;
}
or adding another cast as in comment #1.
Perhaps I should adjust the 40752 patch to work recursively.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589 Jonathan Wakely changed: What|Removed |Added Status|RESOLVED|NEW Last reconfirmed||2020-02-15 Resolution|INVALID |--- Ever confirmed|0 |1 --- Comment #6 from Jonathan Wakely --- (In reply to Jonathan Wakely from comment #4) > The warning was just Oops, I meant to say the warning was just suppressed for the += case, see PR 40752. (In reply to John Downing from comment #5) > This will generate a warning, for the char and short cases, when the "|=" > happens. This is despite "val" being explicitly declared as a char or an > short. It's not *despite* being declared as char or short, it's *because* they are declare as char and short. > I can see this being correct if "val" has been declared as "unsigned", which > i shorthand for "unsigned int". But if "val" is explicitly declared as > something, there should be a potential for the conversion to change the > value. That's not true. Given: val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT * static_cast(i)); The type of (CHAR_BIT * static_cast(i)) is int, due to: https://en.cppreference.com/w/cpp/language/implicit_conversion#Integral_promotion And the type of the entire right-hand side is int, so you are doing val |= int(some_value) which the compiler correctly says might not fit in a short or char. In this specific case, your loop conditions mean that the value of the right hand side will fit, but the warning doesn't know about the values, it only cares that conversion from int to short int involves a potentially lossy conversion. The fact that GCC still warns for this case seems like a bug though: val |= static_cast((CHAR_BIT * i) << (CHAR_BIT * i)); Even though the right operand still gets promoted to type 'int', it clearly doesn't have a value that would be changed by the conversion.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589
John Downing changed:
What|Removed |Added
CC||jrdowning at yahoo dot com
--- Comment #5 from John Downing ---
I'm not sure if this will be seen but here goes.
Try compiling the following with -Wconversion:
#include
#include
#define CHAR_BIT 8
using std::cout;
void print_byte_order_short( short )
{
unsigned short i;
short val = 0;
for(i = 1; i < sizeof(short); ++i)
{
val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT *
static_cast(i));
}
cout << val;
}
void print_byte_order_char( char )
{
unsigned char i;
char val = 0;
for(i = 1; i < sizeof(char); ++i)
{
val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT *
static_cast(i));
}
cout << val;
}
void print_byte_order_long( long )
{
unsigned long i;
long val = 0;
for(i = 1; i < sizeof(long); ++i)
{
val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT *
static_cast(i));
}
cout << val;
}
void print_byte_order_int( int )
{
unsigned int i;
int val = 0;
for(i = 1; i < sizeof(int); ++i)
{
val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT *
static_cast(i));
}
cout << val;
}
int main()
{
char c;
short s;
int i;
long l;
print_byte_order_char(c);
print_byte_order_short(s);
print_byte_order_int(i);
print_byte_order_long(l);
}
This will generate a warning, for the char and short cases, when the "|="
happens. This is despite "val" being explicitly declared as a char or an
short.
I can see this being correct if "val" has been declared as "unsigned", which i
shorthand for "unsigned int". But if "val" is explicitly declared as
something, there should be a potential for the conversion to change the value.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589 Jonathan Wakely changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=40752 --- Comment #4 from Jonathan Wakely --- The warning was just
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589
Andrew Pinski changed:
What|Removed |Added
Status|UNCONFIRMED |RESOLVED
Resolution|--- |INVALID
--- Comment #3 from Andrew Pinski ---
(In reply to Lokesh Janghel from comment #2)
> I reduce the test case for non-template:
> #define CHAR_BIT 8
> void print_byte_order( short )
> {
>short val = 0;
>unsigned i = 1;
>val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT *
> static_cast(i));
> }
>
> That gives warning in GCC but Clang not.
Right and GCC is correct here.
The promotion rules of multiply and shift is to int from short.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589
--- Comment #2 from Lokesh Janghel ---
>Note your example code does not match the warning message you have in comment
>#0.
Sorry, I used some reduce test case. here is the correct one:
$ g++ -Wconversion test.cpp
test.cpp: In instantiation of ‘void print_byte_order(T) [with T = short int]’:
test.cpp:12:28: required from here
test.cpp:7:8: warning: conversion to ‘short int’ from ‘int’ may alter its value
[-Wconversion]
val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT * static_cast(i));
^
>Does clang warn even in a non-template case?
I reduce the test case for non-template:
#define CHAR_BIT 8
void print_byte_order( short )
{
short val = 0;
unsigned i = 1;
val |= (CHAR_BIT * static_cast(i)) << (CHAR_BIT *
static_cast(i));
}
That gives warning in GCC but Clang not.
[Bug c++/93589] Template instantiation creates a conversion warning when it should not
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93589 --- Comment #1 from Andrew Pinski --- I don't see the issue here due to promotion rules in C++. Note your example code does not match the warning message you have in comment #0. Just add an extra cast: static_cast((CHAR_BIT * static_cast(i)) << (CHAR_BIT * static_cast(i))) Will fix the warning. >This does not happen with CLANG. So what two different compilers, two different choices here :). Does clang warn even in a non-template case?
