[clang-tools-extra] 3566024 - [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-05-18T20:47:37+01:00
New Revision: 35660247dd9ce850dccd60cc55428a510dbdd1c8

URL: https://github.com/llvm/llvm-project/commit/35660247dd9ce850dccd60cc55428a510dbdd1c8
DIFF: https://github.com/llvm/llvm-project/commit/35660247dd9ce850dccd60cc55428a510dbdd1c8.diff

LOG: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

Fixes https://github.com/llvm/llvm-project/issues/55553.

Reviewed By: LegalizeAdulthood

Differential Revision: https://reviews.llvm.org/D125874

Added: 
    clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp

Modified: 
    clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 1db440120aa3a..323cf1f2d5ecb 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -351,6 +351,9 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
   }
 
   bool VisitIfStmt(IfStmt *If) {
+    // Skip any if's that have a condition var or an init statement.
+    if (If->hasInitStorage() || If->hasVarStorage())
+      return true;
     /*
      * if (true) ThenStmt(); -> ThenStmt();
      * if (false) ThenStmt(); -> <Empty>;
@@ -461,14 +464,17 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
          * if (Cond) return false; return true; -> return !Cond;
          */
         auto *If = cast<IfStmt>(*First);
-        ExprAndBool ThenReturnBool =
-            checkSingleStatement(If->getThen(), parseReturnLiteralBool);
-        if (ThenReturnBool && ThenReturnBool.Bool != TrailingReturnBool.Bool) {
-          if (Check->ChainedConditionalReturn ||
-              (!PrevIf && If->getElse() == nullptr)) {
-            Check->replaceCompoundReturnWithCondition(
-                Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool, If,
-                ThenReturnBool.Item);
+        if (!If->hasInitStorage() && !If->hasVarStorage()) {
+          ExprAndBool ThenReturnBool =
+              checkSingleStatement(If->getThen(), parseReturnLiteralBool);
+          if (ThenReturnBool &&
+              ThenReturnBool.Bool != TrailingReturnBool.Bool) {
+            if (Check->ChainedConditionalReturn ||
+                (!PrevIf && If->getElse() == nullptr)) {
+              Check->replaceCompoundReturnWithCondition(
+                  Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
+                  If, ThenReturnBool.Item);
+            }
           }
         }
       } else if (isa<LabelStmt, CaseStmt, DefaultStmt>(*First)) {
@@ -481,7 +487,8 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
             : isa<CaseStmt>(*First) ? cast<CaseStmt>(*First)->getSubStmt()
                                     : cast<DefaultStmt>(*First)->getSubStmt();
         auto *SubIf = dyn_cast<IfStmt>(SubStmt);
-        if (SubIf && !SubIf->getElse()) {
+        if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
+            !SubIf->hasVarStorage()) {
           ExprAndBool ThenReturnBool =
               checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
           if (ThenReturnBool &&

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
new file mode 100644
index 0000000000000..310afe04672ae
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
@@ -0,0 +1,19 @@
+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++17 | count 0
+struct RAII {};
+bool foo(bool Cond) {
+  bool Result;
+
+  if (RAII Object; Cond)
+    Result = true;
+  else
+    Result = false;
+
+  if (bool X = Cond; X)
+    Result = true;
+  else
+    Result = false;
+
+  if (bool X = Cond; X)
+    return true;
+  return false;
+}

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
index acf0cfe301279..c14438aa93801 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -478,6 +478,14 @@ void lambda_conditional_return_statements() {
   // CHECK-FIXES-NEXT: {{^}}  };{{$}}
 }
 
+bool condition_variable_return_stmt(int i) {
+  // Unchanged: condition variable.
+  if (bool Res = i == 0)
+    return true;
+  else
+    return false;
+}
+
 void simple_conditional_assignment_statements(int i) {
   bool b;
   if (i > 10)
@@ -594,6 +602,13 @@ void complex_conditional_assignment_statements(int i) {
     h = true;
   } else
     h = false;
+
+  // Unchanged: condition variable.
+  bool k;
+  if (bool Res = j > 10)
+    k = true;
+  else
+    k = false;
 }
 
 // Unchanged: chained return statements, but ChainedConditionalReturn not set.