[clang] 06c6fac - Update static_assert message for redundant cases

[Aaron Ballman via cfe-commits]

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}}