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

LegalizeAdulthood abandoned this revision. LegalizeAdulthood added a comment. Discarding in favor of Nathan James's improved implementation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124650/new/ https://reviews.llvm.org/D124650 ___

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

LegalizeAdulthood added a comment. In D124650#3486859 , @njames93 wrote: > I've done my own implementation, but its definitely over engineered, WDYT? > D124806 Hey, that looks great at a glance. I need to prepare for

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

njames93 added a comment. I've done my own implementation, but its definitely over engineered, WDYT? D124806 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124650/new/ https://reviews.llvm.org/D124650 ___

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

LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:598-599 + auto UnlessNotLHS = unless(hasLHS(NotOp)); + // match !(!a || b) +

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

njames93 added inline comments. 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( +

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

LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:598-599 + auto UnlessNotLHS = unless(hasLHS(NotOp)); + // match !(!a || b) +

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

LegalizeAdulthood added a comment. I also noticed that some of the simplifications being performed resulted in `!(x || !y)` coming out of other parts of the simplifier, so it should be the case that running this check on your code should result in no further changes to your code if you run the

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

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

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

LegalizeAdulthood added a comment. Testing on VTK revealed this change: - this->SliceMapper->SetBackground((this->Background && -!(this->SliceFacesCamera && this->InternalResampleToScreenPixels && - !this->SeparateWindowLevelOperation))); + this->SliceMapper->SetBackground(

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

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:586 + + auto Not = hasOperatorName("!"); + auto Or = hasOperatorName("||"); njames93 wrote: > This whole implementation would be alot simpler(and

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

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

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

LegalizeAdulthood updated this revision to Diff 426163. LegalizeAdulthood added a comment. Surround replacements with parens CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124650/new/ https://reviews.llvm.org/D124650 Files:

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

LegalizeAdulthood added a comment. In D124650#3481514 , @LegalizeAdulthood wrote: > General question: > > There is the question of whether or not the replacement should have `()`s > around the whole > thing because we don't analyze the context to see

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

LegalizeAdulthood updated this revision to Diff 426111. LegalizeAdulthood added a comment. Sort changes by check name in docs CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124650/new/ https://reviews.llvm.org/D124650 Files:

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

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:139 +- Expanded :doc:`readability-simplify-boolean-expr + ` to simplify expressions Please use alphabetical order for such entries. Repository: rG LLVM Github

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

LegalizeAdulthood added a comment. General question: There is the question of whether or not the replacement should have `()`s around the whole thing because we don't analyze the context to see if the change in precedence between `operator!` and `operator&&` or `operator||` makes any

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

LegalizeAdulthood created this revision. LegalizeAdulthood added reviewers: aaron.ballman, njames93. LegalizeAdulthood added projects: clang-tools-extra, All. Herald added subscribers: carlosgalvezp, xazax.hun. LegalizeAdulthood requested review of this revision. Make the following