Author: Krishna Narayanan Date: 2023-04-07T08:22:04-04:00 New Revision: 06c6fac696e46fe58dd48ce8b15fafc308688828
URL: https://github.com/llvm/llvm-project/commit/06c6fac696e46fe58dd48ce8b15fafc308688828 DIFF: https://github.com/llvm/llvm-project/commit/06c6fac696e46fe58dd48ce8b15fafc308688828.diff LOG: Update static_assert message for redundant cases There are some simple messages where an expansion isn't particularly helpful or needed. We aim to eliminate expansions that don't add much value for user (this will give us slightly more room or prioritise to have longer diagnostics elsewhere). The test case for which it has been tested: ``` constexpr auto is_gitlab = false; constexpr auto is_weekend = false; static_assert(is_gitlab or is_weekend); ``` Previous warning/error message: ``` <source>:4:1: error: static assertion failed due to requirement 'is_gitlab || is_weekend' static_assert(is_gitlab or is_weekend); ^ ~~~~~~~~~~~~~~~~~~~~~~~ <source>:4:25: note: expression evaluates to 'false || false' static_assert(is_gitlab or is_weekend); ~~~~~~~~~~^~~~~~~~~~~~~ ``` Currrent warning/error message: ``` <source>:4:1: error: static assertion failed due to requirement 'is_gitlab' static_assert(is_gitlab or is_weekend); ^ ~~~~~~~~~ ``` This patch aims to remove some redundant cases of static assert messages where the expansions are particularly unhelpful. In this particular patch, we have ignored the printing of diagnostic warnings for binary operators with logical OR operations. This is done to prioritise and prefer to emit longer diagnostic warnings for more important concerns elsewhere. Differential Revision: https://reviews.llvm.org/D146376 Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDeclCXX.cpp clang/test/SemaCXX/static-assert.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 5de436f94a6be..3fc2839b93e50 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -213,6 +213,9 @@ Improvements to Clang's diagnostics - There were some cases in which the diagnostic for the unavailable attribute might not be issued, this fixes those cases. (`61815 <https://github.com/llvm/llvm-project/issues/61815>`_) +- Clang now avoids unnecessary diagnostic warnings for obvious expressions in + the case of binary operators with logical OR operations. + (`#57906 <https://github.com/llvm/llvm-project/issues/57906>`_) Bug Fixes in This Version ------------------------- diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 511e87f229254..eb6a4f3296e15 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -16718,7 +16718,8 @@ static bool UsefulToPrintExpr(const Expr *E) { /// Try to print more useful information about a failed static_assert /// with expression \E void Sema::DiagnoseStaticAssertDetails(const Expr *E) { - if (const auto *Op = dyn_cast<BinaryOperator>(E)) { + if (const auto *Op = dyn_cast<BinaryOperator>(E); + Op && Op->getOpcode() != BO_LOr) { const Expr *LHS = Op->getLHS()->IgnoreParenImpCasts(); const Expr *RHS = Op->getRHS()->IgnoreParenImpCasts(); diff --git a/clang/test/SemaCXX/static-assert.cpp b/clang/test/SemaCXX/static-assert.cpp index 97a3aaec0a08e..ea8037815a203 100644 --- a/clang/test/SemaCXX/static-assert.cpp +++ b/clang/test/SemaCXX/static-assert.cpp @@ -258,8 +258,14 @@ namespace Diagnostics { constexpr bool invert(bool b) { return !b; } - static_assert(invert(true) == invert(false), ""); // expected-error {{failed}} \ + + static_assert(invert(true) || invert(true), ""); // expected-error {{static assertion failed due to requirement 'invert(true) || invert(true)'}} + static_assert(invert(true) == invert(false), ""); // expected-error {{static assertion failed due to requirement 'invert(true) == invert(false)'}} \ // expected-note {{evaluates to 'false == true'}} + static_assert(true && false, ""); // expected-error {{static assertion failed due to requirement 'true && false'}} + static_assert(invert(true) || invert(true) || false, ""); // expected-error {{static assertion failed due to requirement 'invert(true) || invert(true) || false'}} + static_assert((true && invert(true)) || false, ""); // expected-error {{static assertion failed due to requirement '(true && invert(true)) || false'}} + static_assert(true && invert(false) && invert(true), ""); // expected-error {{static assertion failed due to requirement 'invert(true)'}} /// No notes here since we compare a bool expression with a bool literal. static_assert(invert(true) == true, ""); // expected-error {{failed}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
