[PATCH] Add readability-simplify-boolean-expr check to clang-tidy

[Alexander Kornienko]

I'm finally back to work and got around to look at this patch again. The latest changes introduced a few issues (see inline comments). I can fix these and submit the patch for you.


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:89
@@ +88,3 @@
+  for (auto NegatableOp : opposites) {
+    if (Opcode == NegatableOp.first || Opcode == NegatableOp.second) {
+      return true;
----------------
nit: In LLVM style there should be no braces around a single-line `if` body.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:101
@@ +100,3 @@
+      return BinOp->getOpcodeStr(NegatableOp.second);
+    } else if (Opcode == NegatableOp.second) {
+      return BinOp->getOpcodeStr(NegatableOp.first);
----------------
nit: Don't use `else` after a `return`.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:105
@@ +104,3 @@
+  }
+  assert(!"Non-negatable operator given to negatedOperator");
+  return BinOp->getOpcodeStr();
----------------
nit: This style seems to be more common in LLVM and more idiomatic: `assert(false && "...")`.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:116
@@ +115,3 @@
+  }
+  if (const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(Expression)) {
+    if (BinOp && Negated && isNegatable(BinOp->getOpcode())) {
----------------
nit: Use `const auto *`.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:118
@@ +117,3 @@
+    if (BinOp && Negated && isNegatable(BinOp->getOpcode())) {
+      Text = (getText(Result, *BinOp->getLHS()) + " " + negatedOperator(BinOp) +
+              " " + getText(Result, *BinOp->getRHS())).str();
----------------
`Text` should be an `std::string`, otherwise you're storing a reference to a temporary string.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:123
@@ +122,3 @@
+  }
+  if (Text.size() == 0) {
+    Text = getText(Result, *Expression);
----------------
nit: Use `Text.empty()` instead.

http://reviews.llvm.org/D7648

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/