[libcxx-commits] [PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Fangrui Song via Phabricator via libcxx-commits]

MaskRay added a comment.

In D147844#4646633 <https://reviews.llvm.org/D147844#4646633>, @MaskRay wrote:

> FWIW: adding parentheses for expressions, such as `overflow = (4608 * 1024 * 1024) ?  4608 * 1024 * 1024 : 0;`, and `results.reason = (actions & _UA_SEARCH_PHASE) ? ... : ...`, looks unnatural and is too noisy to me.

>From quuxplusone:

The first expression

  overflow = (4608 * 1024 * 1024) ?  4608 * 1024 * 1024 : 0;

is a tautology; it is not possible for (4608 * 1024 * 1024) to be zero. (If it's signed integer overflow, it's UB, not zero; and the compiler knows this.)
But if you make it defined by adding `u`, then IMO it's very little hardship to explicitly write the comparison to zero:

  overflow = (4608u * 1024 * 1024) != 0 ?  4608u * 1024 * 1024 : 0;  // OK, no UB, no warning

The second expression

  results.reason = (actions & _UA_SEARCH_PHASE) ? ... : ...

should IMHO also be written as

  results.reason = (actions & _UA_SEARCH_PHASE) != 0 ? ... : ...

but I think you would get good support if you proposed that the compiler should treat `&` as a "logical operator" in this specific case. This just means changing line 9312 to

  return OP->isComparisonOp() || OP->isLogicalOp() || (OP->getOpcode() == BO_And);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844