Skip to content

Commit 8dc7db7

Browse files
11happy11happy
authored
[clang-tidy] Add clang-tidy check readability-math-missing-parentheses (#84481)
This commit closes #80850 where author suggests adding a readability check to detect missing parentheses around mathematical expressions when operators of different priorities are used. Signed-off-by: 11happy <[email protected]>
1 parent d94aeb5 commit 8dc7db7

File tree

8 files changed

+289
-0
lines changed

8 files changed

+289
-0
lines changed

‎clang-tools-extra/clang-tidy/readability/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ add_clang_library(clangTidyReadabilityModule
2828
IsolateDeclarationCheck.cpp
2929
MagicNumbersCheck.cpp
3030
MakeMemberFunctionConstCheck.cpp
31+
MathMissingParenthesesCheck.cpp
3132
MisleadingIndentationCheck.cpp
3233
MisplacedArrayIndexCheck.cpp
3334
NamedParameterCheck.cpp

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
//===--- MathMissingParenthesesCheck.cpp - clang-tidy ---------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "MathMissingParenthesesCheck.h"
10+
#include "clang/AST/ASTContext.h"
11+
#include "clang/ASTMatchers/ASTMatchFinder.h"
12+
#include "clang/Lex/Lexer.h"
13+
14+
using namespace clang::ast_matchers;
15+
16+
namespace clang::tidy::readability {
17+
18+
void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
19+
Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
20+
unless(isAssignmentOperator()),
21+
unless(isComparisonOperator()),
22+
unless(hasAnyOperatorName("&&", "||")),
23+
hasDescendant(binaryOperator()))
24+
.bind("binOp"),
25+
this);
26+
}
27+
28+
static int getPrecedence(const BinaryOperator *BinOp) {
29+
if (!BinOp)
30+
return 0;
31+
switch (BinOp->getOpcode()) {
32+
case BO_Mul:
33+
case BO_Div:
34+
case BO_Rem:
35+
return 5;
36+
case BO_Add:
37+
case BO_Sub:
38+
return 4;
39+
case BO_And:
40+
return 3;
41+
case BO_Xor:
42+
return 2;
43+
case BO_Or:
44+
return 1;
45+
default:
46+
return 0;
47+
}
48+
}
49+
static void addParantheses(const BinaryOperator *BinOp,
50+
const BinaryOperator *ParentBinOp,
51+
ClangTidyCheck *Check,
52+
const clang::SourceManager &SM,
53+
const clang::LangOptions &LangOpts) {
54+
if (!BinOp)
55+
return;
56+
57+
int Precedence1 = getPrecedence(BinOp);
58+
int Precedence2 = getPrecedence(ParentBinOp);
59+
60+
if (ParentBinOp != nullptr && Precedence1 != Precedence2) {
61+
const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
62+
const clang::SourceLocation EndLoc =
63+
clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
64+
if (EndLoc.isInvalid())
65+
return;
66+
67+
Check->diag(StartLoc,
68+
"'%0' has higher precedence than '%1'; add parentheses to "
69+
"explicitly specify the order of operations")
70+
<< (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
71+
: ParentBinOp->getOpcodeStr())
72+
<< (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
73+
: BinOp->getOpcodeStr())
74+
<< FixItHint::CreateInsertion(StartLoc, "(")
75+
<< FixItHint::CreateInsertion(EndLoc, ")")
76+
<< SourceRange(StartLoc, EndLoc);
77+
}
78+
79+
addParantheses(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreImpCasts()),
80+
BinOp, Check, SM, LangOpts);
81+
addParantheses(dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreImpCasts()),
82+
BinOp, Check, SM, LangOpts);
83+
}
84+
85+
void MathMissingParenthesesCheck::check(
86+
const MatchFinder::MatchResult &Result) {
87+
const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binOp");
88+
std::vector<
89+
std::pair<clang::SourceRange, std::pair<const clang::BinaryOperator *,
90+
const clang::BinaryOperator *>>>
91+
Insertions;
92+
const SourceManager &SM = *Result.SourceManager;
93+
const clang::LangOptions &LO = Result.Context->getLangOpts();
94+
addParantheses(BinOp, nullptr, this, SM, LO);
95+
}
96+
97+
} // namespace clang::tidy::readability

‎clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.h

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//===--- MathMissingParenthesesCheck.h - clang-tidy -------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MATHMISSINGPARENTHESESCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MATHMISSINGPARENTHESESCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::readability {
15+
16+
/// Check for mising parantheses in mathematical expressions that involve
17+
/// operators of different priorities.
18+
///
19+
/// For the user-facing documentation see:
20+
/// http://clang.llvm.org/extra/clang-tidy/checks/readability/math-missing-parentheses.html
21+
class MathMissingParenthesesCheck : public ClangTidyCheck {
22+
public:
23+
MathMissingParenthesesCheck(StringRef Name, ClangTidyContext *Context)
24+
: ClangTidyCheck(Name, Context) {}
25+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
26+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
27+
std::optional<TraversalKind> getCheckTraversalKind() const override {
28+
return TK_IgnoreUnlessSpelledInSource;
29+
}
30+
};
31+
32+
} // namespace clang::tidy::readability
33+
34+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MATHMISSINGPARENTHESESCHECK_H

‎clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "IsolateDeclarationCheck.h"
3333
#include "MagicNumbersCheck.h"
3434
#include "MakeMemberFunctionConstCheck.h"
35+
#include "MathMissingParenthesesCheck.h"
3536
#include "MisleadingIndentationCheck.h"
3637
#include "MisplacedArrayIndexCheck.h"
3738
#include "NamedParameterCheck.h"
@@ -105,6 +106,8 @@ class ReadabilityModule : public ClangTidyModule {
105106
"readability-identifier-naming");
106107
CheckFactories.registerCheck<ImplicitBoolConversionCheck>(
107108
"readability-implicit-bool-conversion");
109+
CheckFactories.registerCheck<MathMissingParenthesesCheck>(
110+
"readability-math-missing-parentheses");
108111
CheckFactories.registerCheck<RedundantInlineSpecifierCheck>(
109112
"readability-redundant-inline-specifier");
110113
CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,12 @@ New checks
149149
Enforces consistent style for enumerators' initialization, covering three
150150
styles: none, first only, or all initialized explicitly.
151151

152+
- New :doc:`readability-math-missing-parentheses
153+
<clang-tidy/checks/readability/math-missing-parentheses>` check.
154+
155+
Check for missing parentheses in mathematical expressions that involve
156+
operators of different priorities.
157+
152158
- New :doc:`readability-use-std-min-max
153159
<clang-tidy/checks/readability/use-std-min-max>` check.
154160

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ Clang-Tidy Checks
364364
:doc:`readability-isolate-declaration <readability/isolate-declaration>`, "Yes"
365365
:doc:`readability-magic-numbers <readability/magic-numbers>`,
366366
:doc:`readability-make-member-function-const <readability/make-member-function-const>`, "Yes"
367+
:doc:`readability-math-missing-parentheses <readability/math-missing-parentheses>`, "Yes"
367368
:doc:`readability-misleading-indentation <readability/misleading-indentation>`,
368369
:doc:`readability-misplaced-array-index <readability/misplaced-array-index>`, "Yes"
369370
:doc:`readability-named-parameter <readability/named-parameter>`, "Yes"

‎clang-tools-extra/docs/clang-tidy/checks/readability/math-missing-parentheses.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
.. title:: clang-tidy - readability-math-missing-parentheses
2+
3+
readability-math-missing-parentheses
4+
====================================
5+
6+
Check for missing parentheses in mathematical expressions that involve operators
7+
of different priorities.
8+
9+
Parentheses in mathematical expressions clarify the order
10+
of operations, especially with different-priority operators. Lengthy or multiline
11+
expressions can obscure this order, leading to coding errors. IDEs can aid clarity
12+
by highlighting parentheses. Explicitly using parentheses also clarifies what the
13+
developer had in mind when writing the expression. Ensuring their presence reduces
14+
ambiguity and errors, promoting clearer and more maintainable code.
15+
16+
Before:
17+
18+
.. code-block:: c++
19+
20+
int x = 1 + 2 * 3 - 4 / 5;
21+
22+
23+
After:
24+
25+
.. code-block:: c++
26+
27+
int x = 1 + (2 * 3) - (4 / 5);

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

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// RUN: %check_clang_tidy %s readability-math-missing-parentheses %t
2+
3+
#define MACRO_AND &
4+
#define MACRO_ADD +
5+
#define MACRO_OR |
6+
#define MACRO_MULTIPLY *
7+
#define MACRO_XOR ^
8+
#define MACRO_SUBTRACT -
9+
#define MACRO_DIVIDE /
10+
11+
int foo(){
12+
return 5;
13+
}
14+
15+
int bar(){
16+
return 4;
17+
}
18+
19+
class fun{
20+
public:
21+
int A;
22+
double B;
23+
fun(){
24+
A = 5;
25+
B = 5.4;
26+
}
27+
};
28+
29+
void f(){
30+
//CHECK-MESSAGES: :[[@LINE+2]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
31+
//CHECK-FIXES: int a = 1 + (2 * 3);
32+
int a = 1 + 2 * 3;
33+
34+
int a_negative = 1 + (2 * 3); // No warning
35+
36+
int b = 1 + 2 + 3; // No warning
37+
38+
int c = 1 * 2 * 3; // No warning
39+
40+
//CHECK-MESSAGES: :[[@LINE+3]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
41+
//CHECK-MESSAGES: :[[@LINE+2]]:25: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
42+
//CHECK-FIXES: int d = 1 + (2 * 3) - (4 / 5);
43+
int d = 1 + 2 * 3 - 4 / 5;
44+
45+
int d_negative = 1 + (2 * 3) - (4 / 5); // No warning
46+
47+
//CHECK-MESSAGES: :[[@LINE+4]]:13: warning: '&' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
48+
//CHECK-MESSAGES: :[[@LINE+3]]:17: warning: '+' has higher precedence than '&'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
49+
//CHECK-MESSAGES: :[[@LINE+2]]:25: warning: '*' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
50+
//CHECK-FIXES: int e = (1 & (2 + 3)) | (4 * 5);
51+
int e = 1 & 2 + 3 | 4 * 5;
52+
53+
int e_negative = (1 & (2 + 3)) | (4 * 5); // No warning
54+
55+
//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
56+
//CHECK-FIXES: int f = (1 * -2) + 4;
57+
int f = 1 * -2 + 4;
58+
59+
int f_negative = (1 * -2) + 4; // No warning
60+
61+
//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
62+
//CHECK-FIXES: int g = (1 * 2 * 3) + 4 + 5;
63+
int g = 1 * 2 * 3 + 4 + 5;
64+
65+
int g_negative = (1 * 2 * 3) + 4 + 5; // No warning
66+
67+
//CHECK-MESSAGES: :[[@LINE+4]]:13: warning: '&' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
68+
//CHECK-MESSAGES: :[[@LINE+3]]:19: warning: '+' has higher precedence than '&'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
69+
//CHECK-MESSAGES: :[[@LINE+2]]:27: warning: '*' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
70+
//CHECK-FIXES: int h = (120 & (2 + 3)) | (22 * 5);
71+
int h = 120 & 2 + 3 | 22 * 5;
72+
73+
int h_negative = (120 & (2 + 3)) | (22 * 5); // No warning
74+
75+
int i = 1 & 2 & 3; // No warning
76+
77+
int j = 1 | 2 | 3; // No warning
78+
79+
int k = 1 ^ 2 ^ 3; // No warning
80+
81+
//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '+' has higher precedence than '^'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
82+
//CHECK-FIXES: int l = (1 + 2) ^ 3;
83+
int l = 1 + 2 ^ 3;
84+
85+
int l_negative = (1 + 2) ^ 3; // No warning
86+
87+
//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
88+
//CHECK-FIXES: int m = (2 * foo()) + bar();
89+
int m = 2 * foo() + bar();
90+
91+
int m_negative = (2 * foo()) + bar(); // No warning
92+
93+
//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
94+
//CHECK-FIXES: int n = (1.05 * foo()) + double(bar());
95+
int n = 1.05 * foo() + double(bar());
96+
97+
int n_negative = (1.05 * foo()) + double(bar()); // No warning
98+
99+
//CHECK-MESSAGES: :[[@LINE+3]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
100+
//CHECK-FIXES: int o = 1 + (obj.A * 3) + obj.B;
101+
fun obj;
102+
int o = 1 + obj.A * 3 + obj.B;
103+
104+
int o_negative = 1 + (obj.A * 3) + obj.B; // No warning
105+
106+
//CHECK-MESSAGES: :[[@LINE+2]]:18: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
107+
//CHECK-FIXES: int p = 1U + (2 * 3);
108+
int p = 1U + 2 * 3;
109+
110+
int p_negative = 1U + (2 * 3); // No warning
111+
112+
//CHECK-MESSAGES: :[[@LINE+7]]:13: warning: '+' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
113+
//CHECK-MESSAGES: :[[@LINE+6]]:25: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
114+
//CHECK-MESSAGES: :[[@LINE+5]]:53: warning: '&' has higher precedence than '^'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
115+
//CHECK-MESSAGES: :[[@LINE+4]]:53: warning: '^' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
116+
//CHECK-MESSAGES: :[[@LINE+3]]:77: warning: '-' has higher precedence than '^'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
117+
//CHECK-MESSAGES: :[[@LINE+2]]:94: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
118+
//CHECK-FIXES: int q = (1 MACRO_ADD (2 MACRO_MULTIPLY 3)) MACRO_OR ((4 MACRO_AND 5) MACRO_XOR (6 MACRO_SUBTRACT (7 MACRO_DIVIDE 8)));
119+
int q = 1 MACRO_ADD 2 MACRO_MULTIPLY 3 MACRO_OR 4 MACRO_AND 5 MACRO_XOR 6 MACRO_SUBTRACT 7 MACRO_DIVIDE 8; // No warning
120+
}

0 commit comments

Comments
 (0)