llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Oleksandr T. (a-tarasyuk) <details> <summary>Changes</summary> Fixes #<!-- -->28334 --- This PR introduces the `-Wshift-bool` warning to detect and warn against shifting `bool` values using the `<<` or `>>` operators. Shifting a `bool` implicitly converts it to an `int`, which can lead to unintended behavior. --- Full diff: https://github.com/llvm/llvm-project/pull/127336.diff 5 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+2) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) - (modified) clang/lib/Sema/SemaExpr.cpp (+6) - (added) clang/test/Sema/shift-bool.cpp (+21) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6056a6964fbcc..d623d7daf05ea 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -138,6 +138,7 @@ Improvements to Clang's diagnostics - Fixed a bug where Clang's Analysis did not correctly model the destructor behavior of ``union`` members (#GH119415). - A statement attribute applied to a ``case`` label no longer suppresses 'bypassing variable initialization' diagnostics (#84072). +- The ``-Wshift-bool`` warning has been added to warn about shifting a ``boolean``. (#GH28334) Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 05e39899e6f25..193aea3a8dfc3 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -461,6 +461,8 @@ def DanglingField : DiagGroup<"dangling-field">; def DanglingInitializerList : DiagGroup<"dangling-initializer-list">; def DanglingGsl : DiagGroup<"dangling-gsl">; def ReturnStackAddress : DiagGroup<"return-stack-address">; +def ShiftBool: DiagGroup<"shift-bool">; + // Name of this warning in GCC def : DiagGroup<"return-local-addr", [ReturnStackAddress]>; def Dangling : DiagGroup<"dangling", [DanglingAssignment, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c4f0fc55b4a38..754cf1e14799b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7115,6 +7115,9 @@ def warn_shift_result_sets_sign_bit : Warning< "signed shift result (%0) sets the sign bit of the shift expression's " "type (%1) and becomes negative">, InGroup<DiagGroup<"shift-sign-overflow">>, DefaultIgnore; +def warn_shift_bool : Warning< + "%select{left|right}0 shifting a `bool` implicitly converts it to 'int'">, + InGroup<ShiftBool>, DefaultIgnore; def warn_precedence_bitwise_rel : Warning< "%0 has lower precedence than %1; %1 will be evaluated first">, diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 3cd4010740d19..0816403b2df2a 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -11246,6 +11246,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS, if (S.getLangOpts().OpenCL) return; + if (LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType()) { + S.Diag(Loc, diag::warn_shift_bool) + << (Opc == BO_Shr) /*left|right*/ << LHS.get()->getSourceRange(); + return; + } + // Check right/shifter operand Expr::EvalResult RHSResult; if (RHS.get()->isValueDependent() || diff --git a/clang/test/Sema/shift-bool.cpp b/clang/test/Sema/shift-bool.cpp new file mode 100644 index 0000000000000..dfbc1a1e73307 --- /dev/null +++ b/clang/test/Sema/shift-bool.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -fsyntax-only -Wshift-bool -verify %s + +void t() { + int x = 10; + bool y = true; + + bool a = y << x; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool b = y >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool c = false << x; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool d = false >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool e = y << 5; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool f = y >> 5; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool g = y << -1; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool h = y >> -1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool i = y << 0; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool j = y >> 0; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/127336 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
