[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D140860#4028089 <https://reviews.llvm.org/D140860#4028089>, @hazohelet wrote:

> As you point out, enhancement may be more accurate than bug fix.
> There are rare cases where enabling a warning for missing parentheses in `constexpr` logical expressions can be helpful, I think. For example, consider the following code:
>
>   constexpr A = ...;
>   constexpr B = ...;
>   constexpr C = ...;
>   
>   static_assert(!(A && B || C));
>
> In this case, the static assertion will only be successful if `(A && B || C)` evaluates to `false`, which is equivalent to the following combinations of values for `A`, `B`, and `C`:
>
>   (A, B, C) = (T, F, F) (F, T, F) (F, F, F)
>
> Note that `T` means `true` and `F` means `false`. Here, `C` is always `false`, so `A && B || C` matches the case of `a && b || 0`. Thus, the warning is not issued before this patch.
> If the programmer is not careful and assumes that `(A && B || C)` is equivalent to `(A && (B || C))`, then they expect the values of `A`, `B`, and `C` to also include the following combinations:
>
>   (A, B, C) = (F, T, T) (F, F, T)
>
> This would require the programmer to consider additional, unnecessary combinations after the successful static assertion.
>
> Enabling a warning for missing parentheses in this scenario could help prevent the programmer from making this mistake.

Fair enough. I guess the same argument applies to the non-static case too. I think it was original mostly motivated by people using `&& "This is the reason for the assert"` without parentheses around the LHS & that was generally fine. So, so long as a string literal still does the suppression, it's probably fine.


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

https://reviews.llvm.org/D140860