[PATCH] D127518: [Diagnostics] Fix inconsistent shift-overflow warnings in C++20

[Shivam Gupta via Phabricator via cfe-commits]

xgupta updated this revision to Diff 436746.
xgupta added a comment.

.


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

https://reviews.llvm.org/D127518

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaCXX/shift.cpp


Index: clang/test/SemaCXX/shift.cpp
===================================================================
--- clang/test/SemaCXX/shift.cpp
+++ clang/test/SemaCXX/shift.cpp
@@ -41,8 +41,8 @@
 
   int i;
   i = 1 << (WORD_BIT - 2);
-  i = 2 << (WORD_BIT - 1); // expected-warning {{bits to represent, but 'int' only has}}
-  i = 1 << (WORD_BIT - 1); // expected-warning {{sets the sign bit of the shift expression}}
+  i = 2 << (WORD_BIT - 1); // cxx17-warning {{bits to represent, but 'int' only has}}
+  i = 1 << (WORD_BIT - 1); // cxx17-warning {{sets the sign bit of the shift expression}}
   i = -1 << (WORD_BIT - 1); // cxx17-warning {{shifting a negative signed value is undefined}}
   i = -1 << 0; // cxx17-warning {{shifting a negative signed value is undefined}}
   i = 0 << (WORD_BIT - 1);
@@ -53,7 +53,7 @@
   u = 5U << (WORD_BIT - 1);
 
   long long int lli;
-  lli = INT_MIN << 2; // cxx17-warning {{shifting a negative signed value is undefined}} cxx2a-warning {{requires 34 bits to represent}}
+  lli = INT_MIN << 2; // cxx17-warning {{shifting a negative signed value is undefined}}
   lli = 1LL << (sizeof(long long) * CHAR_BIT - 2);
 }
 
Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===================================================================
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -530,8 +530,8 @@
   using int32 = __INT32_TYPE__;
   static_assert(uint32(int32(0x1234) << 16) == 0x12340000);
   static_assert(uint32(int32(0x1234) << 19) == 0x91a00000);
-  static_assert(uint32(int32(0x1234) << 20) == 0x23400000); // expected-warning {{requires 34 bits}}
-  static_assert(uint32(int32(0x1234) << 24) == 0x34000000); // expected-warning {{requires 38 bits}}
+  static_assert(uint32(int32(0x1234) << 20) == 0x23400000);
+  static_assert(uint32(int32(0x1234) << 24) == 0x34000000);
   static_assert(uint32(int32(-1) << 31) == 0x80000000);
 
   static_assert(-1 >> 1 == -1);
Index: clang/test/CXX/expr/expr.const/p2-0x.cpp
===================================================================
--- clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -162,9 +162,9 @@
   constexpr int shl_signed_ok = 1 << 30; // ok
   constexpr int shl_signed_into_sign = 1 << 31; // ok (DR1457)
   constexpr int shl_signed_into_sign_2 = 0x7fffffff << 1; // ok (DR1457)
-  constexpr int shl_signed_off_end = 2 << 31; // cxx11-error {{constant expression}} cxx11-note {{signed left shift discards bits}} expected-warning {{signed shift result (0x100000000) requires 34 bits to represent, but 'int' only has 32 bits}}
-  constexpr int shl_signed_off_end_2 = 0x7fffffff << 2; // cxx11-error {{constant expression}} cxx11-note {{signed left shift discards bits}} expected-warning {{signed shift result (0x1FFFFFFFC) requires 34 bits to represent, but 'int' only has 32 bits}}
-  constexpr int shl_signed_overflow = 1024 << 31; // cxx11-error {{constant expression}} cxx11-note {{signed left shift discards bits}} expected-warning {{requires 43 bits to represent}}
+  constexpr int shl_signed_off_end = 2 << 31; // cxx11-error {{constant expression}} cxx11-note {{signed left shift discards bits}} cxx11-warning {{signed shift result (0x100000000) requires 34 bits to represent, but 'int' only has 32 bits}}
+  constexpr int shl_signed_off_end_2 = 0x7fffffff << 2; // cxx11-error {{constant expression}} cxx11-note {{signed left shift discards bits}} cxx11-warning {{signed shift result (0x1FFFFFFFC) requires 34 bits to represent, but 'int' only has 32 bits}}
+  constexpr int shl_signed_overflow = 1024 << 31; // cxx11-error {{constant expression}} cxx11-note {{signed left shift discards bits}} cxx11-warning {{requires 43 bits to represent}}
   constexpr int shl_signed_ok2 = 1024 << 20; // ok
 
   constexpr int shr_m1 = 0 >> -1; // expected-error {{constant expression}} expected-note {{negative shift count -1}}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11416,10 +11416,16 @@
     return;
   llvm::APSInt Left = LHSResult.Val.getInt();
 
-  // If LHS does not have a signed type and non-negative value
-  // then, the behavior is undefined before C++2a. Warn about it.
-  if (Left.isNegative() && !S.getLangOpts().isSignedOverflowDefined() &&
-      !S.getLangOpts().CPlusPlus20) {
+  // Don't warn if signed overflow is defined, then all the rest of the
+  // diagnostics will not be triggered because the behavior is defined.
+  // Also don't warn in C++20 mode (and newer), as signed left shifts
+  // always wrap and never overflow.
+  if (S.getLangOpts().isSignedOverflowDefined() || S.getLangOpts().CPlusPlus20)
+    return;
+
+  // If LHS does not have a non-negative value then, the
+  // behavior is undefined before C++2a. Warn about it.
+  if (Left.isNegative()) {
     S.DiagRuntimeBehavior(Loc, LHS.get(),
                           S.PDiag(diag::warn_shift_lhs_negative)
                             << LHS.get()->getSourceRange());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127518.436746.patch
Type: text/x-patch
Size: 5096 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220614/9b7a78c9/attachment.bin>