[PATCH] D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:586
+
+  auto Not = hasOperatorName("!");
+  auto Or = hasOperatorName("||");
----------------
This whole implementation would be alot simpler(and likely faster) if you matched on the generic case then in the check callback work out what replacement you need.
```lang=c++
Finder->addMatcher(
    unaryOperator(
        Not,
        hasUnaryOperand(binaryOperator(
            hasAnyOperatorName("&&", "||"),
            hasEitherOperand(unaryOperator(Not))))).bind(Demorgan),
    this);
```


================
Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:598-599
+  auto UnlessNotLHS = unless(hasLHS(NotOp));
+  // match !(!a || b)
+  Finder->addMatcher(unaryOperator(Not, hasUnaryOperand(binaryOperator(
+                                            Or, UnlessNotRHS, NotLHS, RHS)))
----------------
Maybe I'm overthinking this, but how come you don't need the match on the ParenExpr?
Is there some traversal mode?


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:139
 
+- Expanded :doc:`readability-simplify-boolean-expr
+  <clang-tidy/checks/readability-simplify-boolean-expr>` to simplify expressions
----------------
Eugene.Zelenko wrote:
> Please use alphabetical order for such entries.
@LegalizeAdulthood I've pushed the change to alphabetize the list in rG8a9e2dd48d81 seperately, please rebase.


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

https://reviews.llvm.org/D124650