[clang] 48e1829 - [Diagnostics] Fix inconsistent shift-overflow warnings in C++20

[Shivam Gupta via cfe-commits]

Author: Shivam Gupta
Date: 2022-06-14T20:19:46+05:30
New Revision: 48e1829874df15fd57e731d4824604f28e177585

URL: https://github.com/llvm/llvm-project/commit/48e1829874df15fd57e731d4824604f28e177585
DIFF: https://github.com/llvm/llvm-project/commit/48e1829874df15fd57e731d4824604f28e177585.diff

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

This fixes https://github.com/llvm/llvm-project/issues/52873.
Don't warn in C++2A mode (and newer), as signed left shifts
always wrap and never overflow. Ref. -
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1236r1.html.

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    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

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e68640074cea5..8766adb27a09a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -256,6 +256,10 @@ Improvements to Clang's diagnostics
 - The ``-Wdeprecated`` diagnostic will now warn on out-of-line ``constexpr``
   declarations downgraded to definitions in C++1z, in addition to the
   existing warning on out-of-line ``const`` declarations.
+- ``-Wshift-overflow`` will not warn for signed left shifts in C++20 mode
+  (and newer), as it will always wrap and never overflow. This fixes
+  `Issue 52873 <https://github.com/llvm/llvm-project/issues/52873>`_.
+
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7e6618bd2d11d..6edf5b355d029 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11416,10 +11416,16 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
     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());

diff  --git a/clang/test/CXX/expr/expr.const/p2-0x.cpp b/clang/test/CXX/expr/expr.const/p2-0x.cpp
index 81efbb0a47271..6d2bc3f25179b 100644
--- a/clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ b/clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -162,9 +162,9 @@ namespace UndefinedBehavior {
   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}}

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 4aaf6328aaf63..760821eb88373 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -530,8 +530,8 @@ namespace TwosComplementShifts {
   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);

diff  --git a/clang/test/SemaCXX/shift.cpp b/clang/test/SemaCXX/shift.cpp
index 144c18849050c..89a98791d3eba 100644
--- a/clang/test/SemaCXX/shift.cpp
+++ b/clang/test/SemaCXX/shift.cpp
@@ -41,8 +41,8 @@ void test() {
 
   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 @@ void test() {
   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);
 }