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<T>((CHAR_BIT * static_cast<T>(i)) << (CHAR_BIT * static_cast<T>(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?

>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<T>(i)) << (CHAR_BIT * static_cast<T>(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<short>(i)) << (CHAR_BIT * static_cast<short>(i));
}

That gives warning in GCC but Clang not.

(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<short>(i)) << (CHAR_BIT *
> static_cast<short>(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.

The warning was just

I'm not sure if this will be seen but here goes.

Try compiling the following with -Wconversion:

#include <iostream>
#include <iomanip>

#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<short>(i)) << (CHAR_BIT * static_cast<short>(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<char>(i)) << (CHAR_BIT * static_cast<char>(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<long>(i)) << (CHAR_BIT * static_cast<long>(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<int>(i)) << (CHAR_BIT * static_cast<int>(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.

(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<char>(i)) << (CHAR_BIT * static_cast<char>(i));

The type of (CHAR_BIT * static_cast<char>(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>((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.

(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<short>(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<short>(i));
   short pat2 = pat1 << (CHAR_BIT * static_cast<short>(i));
   val |= pat2;
}

or adding another cast as in comment #1.

Perhaps I should adjust the 40752 patch to work recursively.

> (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<char>(i)) << (CHAR_BIT *
> static_cast<char>(i));
> 
> The type of (CHAR_BIT * static_cast<char>(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".

(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<short>(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<short>(i));
>    short pat2 = pat1 << (CHAR_BIT * static_cast<short>(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<short>(i));
      short part2 = part1 << CHAR_BIT * static_cast<short>(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<short>(i));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
example2.cpp:16:27: warning: conversion from int to short int may change value [-Wconversion]
       short part2 = part1 << CHAR_BIT * static_cast<short>(i);
                     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Which seems like the same "design decision" I referenced in comment #8.

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.

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.