[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

In D124806#3523228 <https://reviews.llvm.org/D124806#3523228>, @njames93 wrote:

> In D124806#3523109 <https://reviews.llvm.org/D124806#3523109>, @LegalizeAdulthood wrote:
>
>> LGTM!  Thanks for taking my idea and implementing it much better than what I had `:)`.
>
> Cheers.
> Can I ask why you feel that this should be behind an option?

When I've brought it up in manual code reviews before, there were some objections.
That's why I thought it was worth adding an option to disable it.  Many people aren't
even familiar with DeMorgan's Theorem/Law and might be afraid that the simplification
changes semantics, although the underlying github issue shows that this is not the case.

> Also do you think there is merit in handling the case `!(A && B)` -> `!A || !B` obviously that could be behind an option.

The reason I proposed the simplification `!(!A && B)` -> `A || !B` is because the former employs
"double negatives" and provides a stumbling block for readability.  All readability "checks"
are somewhat influenced by opinion and style to a certain extent, but I think the avoid
"double negative" idea is widely accepted.

In the case of `!(A && B)` -> `!A || !B`, no double negative is involved and one could argue
that it went from one negative to two, which is why I didn't include that in my original
proposal.  The "goodness" of it just doesn't seem universal and personally if the check
grew to enable that case, I'd want an option to disable that behavior `:)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806