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

Wed Apr 26 04:34:07 PDT 2023

aaron.ballman added a comment.

In D147844#4293743 <https://reviews.llvm.org/D147844#4293743>, @cjdb wrote:

> In D147844#4293693 <https://reviews.llvm.org/D147844#4293693>, @dblaikie wrote:
>
>>> I think some of the cases are ambiguous while others are not.
>>
>> Data would be good to have - if this assessment is true, we'd expect this to bear out in terms of bug finding, yeah? (that the cases you find to be ambiguous turn up as real bugs with some significant frequency in a codebase?)
>
> I disagree that there's a need for bugs to add this warning. Reading ambiguous-but-correct code isn't going to be buggy, but it is going to cause readability issues for any reviewers or maintainers.

The number of changes to test cases convinces me that we definitely need data on whether it's worth it or not. This warning now looks chatty enough that it needs to be disabled by default, and that doesn't meet our usual bar.

>> Readability improvements that don't cross the threshold to be the cause of a significant number of bugs are moreso the domain of clang-tidy, not clang warnings.

Strong +1, this is starting to feel more like a style warning due to the chattiness.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844