LegalizeAdulthood added inline comments.
================
Comment at: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:586
+
+  auto Not = hasOperatorName("!");
+  auto Or = hasOperatorName("||");
----------------
njames93 wrote:
> njames93 wrote:
> > 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);
> > ```
> Come to think of it, you wouldn't even need to work out which. Just remove 
> the outer `!`, Exchange `||` with `&&` and invert each side of the binary 
> operator.
These are good ideas, I'll take a look.  I wasn't aware of `hasEitherOperand`.  
I mean, it's in that giant AST matcher reference page, but somehow I overlooked 
it.  On Windows, we don't have TAB completion in clang-query either, so it 
becomes more difficult to interactively explore possibilities.


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

https://reviews.llvm.org/D124650

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

Reply via email to