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

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 430473.
njames93 added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125874/new/

https://reviews.llvm.org/D125874

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


Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -478,6 +478,14 @@
   // 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 @@
     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.
Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
===================================================================
--- /dev/null
+++ 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;
+}
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -351,6 +351,9 @@
   }
 
   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 @@
          * 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 @@
             : 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 &&

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits