[PATCH] D136532: [clang][Interp] Implement left and right shifts

[Shafik Yaghmour via Phabricator via cfe-commits]

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:
> 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 


================
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}}
----------------
aaron.ballman wrote:
> tbaeder wrote:
> > 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.
> Also worth keeping in mind: C and C++ differ here.
> 
> C2x 6.5.7p4: "... If E1 has a signed type and nonnegative value, and E1 × 2E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined."
Apologies, I missed the diagnostic was only firing for C++17.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136532/new/

https://reviews.llvm.org/D136532