search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.85k stars 11.47k forks source link

[clang-tidy] Handle missing parenthesis around non-trivial conditions in ternary expressions. #91077

Open RKSimon opened 4 months ago

RKSimon commented 4 months ago

As reported on #85868 (and case inside llvm/clang fixed by #90391)

We have cases like:

    for (unsigned i = TSFlags & X86II::EVEX_K ? 2 : 1;

Similar to #84481 it'd be much easier to read if this was:

    for (unsigned i = (TSFlags & X86II::EVEX_K) ? 2 : 1;
llvmbot commented 4 months ago

@llvm/issue-subscribers-clang-tidy

Author: Simon Pilgrim (RKSimon)

As reported on #85868 (and case inside llvm/clang fixed by #90391) We have cases like: ```cpp for (unsigned i = TSFlags & X86II::EVEX_K ? 2 : 1; ``` Similar to #84481 it'd be much easier to read if this was: ```cpp for (unsigned i = (TSFlags & X86II::EVEX_K) ? 2 : 1; ```
MaskRay commented 4 months ago

Code bases that prefer more parens can adopt this pattern, but llvm-project should probably avoid it.

The ternary conditional pattern [-+*%] \w+ \? is a common idiom used in many codebases, e.g. I == E - 1 ? Size : Layout->getElementOffset(I + 1).

Adding parentheses around these expressions seems to provide little benefit in terms of readability or maintainability, given the churn cost. Consider suppressing these static analyzer reports for this specific pattern.