Skip to content

Commit c9df272

Browse files
authored
[clang-tidy] Fix parentheses handling in readability-math-missing-parentheses (#172423)
Previously, the check would stop AST traversal when encountering a `ParenExpr`. This PR fixes this problem Closes #172269
1 parent 8a6fae4 commit c9df272

File tree

3 files changed

+33
-6
lines changed

3 files changed

+33
-6
lines changed

clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,17 @@ static int getPrecedence(const BinaryOperator *BinOp) {
4848
return 0;
4949
}
5050
}
51-
static void addParentheses(const BinaryOperator *BinOp,
52-
const BinaryOperator *ParentBinOp,
51+
static void addParentheses(const Expr *E, const BinaryOperator *ParentBinOp,
5352
ClangTidyCheck *Check,
5453
const clang::SourceManager &SM,
5554
const clang::LangOptions &LangOpts) {
55+
if (const auto *Paren = dyn_cast<ParenExpr>(E)) {
56+
addParentheses(Paren->getSubExpr()->IgnoreImpCasts(), nullptr, Check, SM,
57+
LangOpts);
58+
return;
59+
}
60+
61+
const auto *BinOp = dyn_cast<BinaryOperator>(E);
5662
if (!BinOp)
5763
return;
5864

@@ -81,10 +87,8 @@ static void addParentheses(const BinaryOperator *BinOp,
8187
}
8288
}
8389

84-
addParentheses(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreImpCasts()),
85-
BinOp, Check, SM, LangOpts);
86-
addParentheses(dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreImpCasts()),
87-
BinOp, Check, SM, LangOpts);
90+
addParentheses(BinOp->getLHS()->IgnoreImpCasts(), BinOp, Check, SM, LangOpts);
91+
addParentheses(BinOp->getRHS()->IgnoreImpCasts(), BinOp, Check, SM, LangOpts);
8892
}
8993

9094
void MathMissingParenthesesCheck::check(

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,10 @@ Changes in existing checks
619619
by not enforcing parameter name consistency between a variadic parameter pack
620620
in the primary template and specific parameters in its specializations.
621621

622+
- Improved :doc:`readability-math-missing-parentheses
623+
<clang-tidy/checks/readability/math-missing-parentheses>` check by correctly
624+
diagnosing operator precedence issues inside parenthesized expressions.
625+
622626
- Improved :doc:`readability-qualified-auto
623627
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
624628
`IgnoreAliasing`, that allows not looking at underlying types of type aliases.

clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,22 @@ void CompareAsParentBinOp(int b) {
173173

174174
}
175175
}
176+
177+
void test_with_parentheses() {
178+
// CHECK-MESSAGES: :[[@LINE+2]]:14: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
179+
// CHECK-FIXES: int z = (2-(4*3/2)) / (3-1);
180+
int z = (2-4*3/2) / (3-1);
181+
182+
// CHECK-MESSAGES: :[[@LINE+2]]:14: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
183+
// CHECK-FIXES: int x = (2-(4*3/2));
184+
int x = (2-4*3/2);
185+
186+
// CHECK-MESSAGES: :[[@LINE+2]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
187+
// CHECK-FIXES: int y = ((1 + (2 * 3)));
188+
int y = ((1 + 2 * 3));
189+
190+
short s = 0;
191+
// CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
192+
// CHECK-FIXES: s = ((1 + (2 * 3)));
193+
s = ((1 + 2 * 3));
194+
}

0 commit comments

Comments
 (0)