aaron.ballman added inline comments.
================ Comment at: clang/test/AST/Interp/shifts.cpp:57 + //c >>= 999999; // expected-warning {{shift count >= width of type}} + //c <<= CHAR_BIT; // expected-warning {{shift count >= width of type}} + //c >>= CHAR_BIT; // expected-warning {{shift count >= width of type}} ---------------- shafik wrote: > aaron.ballman wrote: > > tbaeder wrote: > > > shafik wrote: > > > > This is not correct, the operands go through integral promotions first > > > > and the result is the promoted type of the left operand see [dcl.shift > > > > p1](https://eel.is/c++draft/expr.shift#1). > > > > > > > > Also see godbolt: https://godbolt.org/z/7qzKjojMb > > > Hmm, this is copy-pasted from `test/SemaCXX/shift.cpp`. > > FWIW, I agree with Shafik -- you can also see the casts in the AST: > > https://godbolt.org/z/97dqr1TEs > This weird, you can see this is indeed valid in a constant expression: > https://godbolt.org/z/W33janMxv > > but we do obtain that diagnostic and I think I see what it is saying, that > the result type is 8bit but the shift is larger than that type even though > the operation is being done on the promoted type. I feel like the diagnostic > is not great but could be improved to make it better. > > wdyt @aaron.ballman > This weird, you can see this is indeed valid in a constant expression: > https://godbolt.org/z/W33janMxv Yes, because of the integral promotion of `c` to `int`. > but we do obtain that diagnostic and I think I see what it is saying, that > the result type is 8bit but the shift is larger than that type even though > the operation is being done on the promoted type. I feel like the diagnostic > is not great but could be improved to make it better. I think it's telling the programmer that something strange is happening here. Yes, it's well-defined because of integral promotions, but it still looks like suspect code. We could say "width of the unpromoted type" but that might make the diagnostic more confusing for non-language lawyer folks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136532/new/ https://reviews.llvm.org/D136532 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits