Skip to content

Commit

Permalink
[Diagnostics] Fix inconsistent shift-overflow warnings in C++20
Browse files Browse the repository at this point in the history
This fixes #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.
  • Loading branch information
xgupta committed Jun 14, 2022
1 parent d5d8366 commit 48e1829
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 12 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------------------------------------------
Expand Down
14 changes: 10 additions & 4 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CXX/expr/expr.const/p2-0x.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/constant-expression-cxx2a.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions clang/test/SemaCXX/shift.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down

0 comments on commit 48e1829

Please sign in to comment.