[PATCH] D150209: [clang][Interp] Add more shift error checking

[Timm Bäder via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd6b0af0574ca: [clang][Interp] Add more shift error checking 
(authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150209

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/shifts.cpp


Index: clang/test/AST/Interp/shifts.cpp
===================================================================
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -152,4 +152,39 @@
   constexpr signed int R = (sizeof(unsigned) * 8) + 1;
   constexpr decltype(L) M  = (R > 32 && R < 64) ?  L << R : 0;
   constexpr decltype(L) M2 = (R > 32 && R < 64) ?  L >> R : 0;
+
+
+  constexpr int signedShift() { // cxx17-error {{never produces a constant expression}} \
+                                // ref-cxx17-error {{never produces a constant expression}}
+    return 1024 << 31; // cxx17-warning {{signed shift result}} \
+                       // ref-cxx17-warning {{signed shift result}} \
+                       // cxx17-note {{signed left shift discards bits}} \
+                       // ref-cxx17-note {{signed left shift discards bits}}
+  }
+
+  constexpr int negativeShift() { // cxx17-error {{never produces a constant expression}} \
+                                  // ref-cxx17-error {{never produces a constant expression}}
+    return -1 << 2; // cxx17-warning {{shifting a negative signed value is undefined}} \
+                    // ref-cxx17-warning {{shifting a negative signed value is undefined}} \
+                    // cxx17-note {{left shift of negative value -1}} \
+                    // ref-cxx17-note {{left shift of negative value -1}}
+  }
+
+  constexpr int foo(int a) {
+    return -a << 2; // cxx17-note {{left shift of negative value -10}} \
+                    // ref-cxx17-note {{left shift of negative value -10}} \
+                    // cxx17-note {{left shift of negative value -2}} \
+                    // ref-cxx17-note {{left shift of negative value -2}}
+  }
+  static_assert(foo(10)); // cxx17-error {{not an integral constant expression}} \
+                          // cxx17-note {{in call to 'foo(10)'}} \
+                          // ref-cxx17-error {{not an integral constant expression}} \
+                          // ref-cxx17-note {{in call to 'foo(10)'}}
+
+  constexpr int a = -2;
+  static_assert(foo(a));
+  static_assert(foo(-a)); // cxx17-error {{not an integral constant expression}} \
+                          // cxx17-note {{in call to 'foo(2)'}} \
+                          // ref-cxx17-error {{not an integral constant expression}} \
+                          // ref-cxx17-note {{in call to 'foo(2)'}}
 };
Index: clang/lib/AST/Interp/Interp.h
===================================================================
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -105,8 +105,9 @@
 bool CheckCtorCall(InterpState &S, CodePtr OpPC, const Pointer &This);
 
 /// Checks if the shift operation is legal.
-template <typename RT>
-bool CheckShift(InterpState &S, CodePtr OpPC, const RT &RHS, unsigned Bits) {
+template <typename LT, typename RT>
+bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
+                unsigned Bits) {
   if (RHS.isNegative()) {
     const SourceInfo &Loc = S.Current->getSource(OpPC);
     S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
@@ -122,6 +123,20 @@
     S.CCEDiag(E, diag::note_constexpr_large_shift) << Val << Ty << Bits;
     return false;
   }
+
+  if (LHS.isSigned() && !S.getLangOpts().CPlusPlus20) {
+    const Expr *E = S.Current->getExpr(OpPC);
+    // C++11 [expr.shift]p2: A signed left shift must have a non-negative
+    // operand, and must not overflow the corresponding unsigned type.
+    if (LHS.isNegative())
+      S.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
+    else if (LHS.toUnsigned().countLeadingZeros() < static_cast<unsigned>(RHS))
+      S.CCEDiag(E, diag::note_constexpr_lshift_discards);
+  }
+
+  // C++2a [expr.shift]p2: [P0907R4]:
+  //    E1 << E2 is the unique value congruent to
+  //    E1 x 2^E2 module 2^N.
   return true;
 }
 
@@ -1523,7 +1538,7 @@
   const auto &LHS = S.Stk.pop<LT>();
   const unsigned Bits = LHS.bitWidth();
 
-  if (!CheckShift<RT>(S, OpPC, RHS, Bits))
+  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
     return false;
 
   Integral<LT::bitWidth(), false> R;
@@ -1540,7 +1555,7 @@
   const auto &LHS = S.Stk.pop<LT>();
   const unsigned Bits = LHS.bitWidth();
 
-  if (!CheckShift<RT>(S, OpPC, RHS, Bits))
+  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
     return false;
 
   Integral<LT::bitWidth(), false> R;
Index: clang/lib/AST/Interp/Integral.h
===================================================================
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -127,7 +127,11 @@
     return Compare(V, RHS.V);
   }
 
-  unsigned countLeadingZeros() const { return llvm::countl_zero<ReprT>(V); }
+  unsigned countLeadingZeros() const {
+    if constexpr (!Signed)
+      return llvm::countl_zero<ReprT>(V);
+    llvm_unreachable("Don't call countLeadingZeros() on signed types.");
+  }
 
   Integral truncate(unsigned TruncBits) const {
     if (TruncBits >= Bits)

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits