[PATCH] D47122: [clang-tidy] SimplifyBoolenExpr doesn't add parens if unary negotiation is of ExprWithCleanups type

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis created this revision.
zinovy.nis added reviewers: alexfh, aaron.ballman.
zinovy.nis added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.

  using A = std::string;
  bool foo(A &s) {
    A S;
    if (GetValue(s) && S != (A)s)
      return false;
    return true;
  }

is fixed into (w/o this patch)

  ...
  return !GetValue(s) && S != (A)s; // negotiation affects first operand only
  }

instead of (with this patch)

  ...
  return !(GetValue(s) && S != (A)s); // note parens for the whole expression
  }


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47122

Files:
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  test/clang-tidy/readability-simplify-bool-expr.cpp


Index: test/clang-tidy/readability-simplify-bool-expr.cpp
===================================================================
--- test/clang-tidy/readability-simplify-bool-expr.cpp
+++ test/clang-tidy/readability-simplify-bool-expr.cpp
@@ -938,3 +938,15 @@
 }
 // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
 // CHECK-FIXES: return p->m != 0;{{$}}
+
+bool operator!=(const A&, const A&) { return false; }
+extern bool GetValue(A s);
+bool expr_with_cleanups(A &s) {
+  A S;
+  if (GetValue(s) && S != (A)s)
+    return false;
+
+  return true;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: !(GetValue(s) && S != (A)s);{{$}}
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -75,6 +75,8 @@
 
 bool needsParensAfterUnaryNegation(const Expr *E) {
   E = E->IgnoreImpCasts();
+  if (auto *EC = dyn_cast<ExprWithCleanups>(E))
+    E = EC->getSubExpr();
   if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
     return true;
 



Index: test/clang-tidy/readability-simplify-bool-expr.cpp
===================================================================
--- test/clang-tidy/readability-simplify-bool-expr.cpp
+++ test/clang-tidy/readability-simplify-bool-expr.cpp
@@ -938,3 +938,15 @@
 }
 // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
 // CHECK-FIXES: return p->m != 0;{{$}}
+
+bool operator!=(const A&, const A&) { return false; }
+extern bool GetValue(A s);
+bool expr_with_cleanups(A &s) {
+  A S;
+  if (GetValue(s) && S != (A)s)
+    return false;
+
+  return true;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: !(GetValue(s) && S != (A)s);{{$}}
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -75,6 +75,8 @@
 
 bool needsParensAfterUnaryNegation(const Expr *E) {
   E = E->IgnoreImpCasts();
+  if (auto *EC = dyn_cast<ExprWithCleanups>(E))
+    E = EC->getSubExpr();
   if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
     return true;
 

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