tbaeder added inline comments.
================ Comment at: clang/lib/AST/Interp/Interp.h:1246-1256 if (V.isSigned() && !S.getLangOpts().CPlusPlus20) { // C++11 [expr.shift]p2: A signed left shift must have a non-negative // operand, and must not overflow the corresponding unsigned type. // C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to // E1 x 2^E2 module 2^N. if (V.isNegative()) { const Expr *E = S.Current->getExpr(OpPC); ---------------- aaron.ballman wrote: > Why do `Shr` and `Shl` check for a negative shift amount to issue a > diagnostic but we check for signed in `ShiftLeft` to issue a diagnostic? (I > would have expected the checks to all live in the same place.) This was already implemented and I tried to keep as much code as possible, but I'll change that strategy ;) ================ 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: > 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`. ================ Comment at: clang/test/AST/Interp/shifts.cpp:70 + i = 1 << (WORD_BIT - 1); // cxx17-warning-not {{sets the sign bit of the shift expression}} + i = -1 << (WORD_BIT - 1); // cxx17-warning {{shifting a negative signed value is undefined}} \ + // ref-cxx17-warning {{shifting a negative signed value is undefined}} ---------------- shafik wrote: > shafik wrote: > > A negative left operand was made well-formed in C++20 I believe see > > godbolt: https://godbolt.org/z/7qzKjojMb > > > > My reference from above for `expr.shift/p1` also applies. > > > > Although a negative right operand is still UB. > > > > Also note shifting into the sign bit was made well-formed in C++11: > > https://stackoverflow.com/questions/19593938/is-left-shifting-a-negative-integer-undefined-behavior-in-c11#comment29091986_19593938 > Typo, shifting into the sign bit was made well-formed after C++11 This is in line with the test, isn't it? The warning is only for `cxx17`, the c++20 tests don't expect any output. Repository: rG LLVM Github Monorepo 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