shafik 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}}
----------------
aaron.ballman wrote:
> 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.
> > 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.
I am bothered that the same diagnostic message represents two cases 1) a case
which is undefined behavior 2) a case which may be surprising but well defined.
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
