cjdb added a comment. In D147844#4293988 <https://reviews.llvm.org/D147844#4293988>, @dblaikie wrote:
> 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. > > That's generally been the bar we use for evaluating warnings - does it find > bugs. Especially because if it doesn't, it's unlikely to be turned on on > large pre-existing codebases owing to the cost of cleaning them up with > limited value in terms of improving readability but not finding any bugs. (& > goes hand-in-hand with the general policy of not adding off-by-default > warnings, because they don't get used much and come at a cost to clang's > codebase (& some (death-by-a-thousand-cuts) cost to compile time performance, > even when the warning is disabled)) > > 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. Fair enough, I often mix up which project a warning belongs to, so thanks for the reminder :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147844/new/ https://reviews.llvm.org/D147844 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
