Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
RKSimon opened this issue May 4, 2024 · 2 comments
Labels
check-request Request for a new check in clang-tidy clang-tidy

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented May 4, 2024

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;
@RKSimon RKSimon added clang-tidy check-request Request for a new check in clang-tidy labels May 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 4, 2024

@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:

    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;

@MaskRay
Copy link
Member

MaskRay commented May 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check-request Request for a new check in clang-tidy clang-tidy
Projects
None yet
Development

No branches or pull requests

3 participants