Skip to content

Commit fbe4d99

Browse files
authored
[clang-tidy] fix false-negative for macros in readability-math-missing-parentheses (llvm#90279)
When a binary operator is the last operand of a macro, the end location that is past the `BinaryOperator` will be inside the macro and therefore an invalid location to insert a `FixIt` into, which is why the check bails when encountering such a pattern. However, the end location is only required for the `FixIt` and the diagnostic can still be emitted, just without an attached fix.
1 parent 79095b4 commit fbe4d99

File tree

2 files changed

+31
-7
lines changed

2 files changed

+31
-7
lines changed

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,21 @@ static void addParantheses(const BinaryOperator *BinOp,
6161
const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
6262
const clang::SourceLocation EndLoc =
6363
clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
64-
if (EndLoc.isInvalid())
65-
return;
6664

67-
Check->diag(StartLoc,
68-
"'%0' has higher precedence than '%1'; add parentheses to "
69-
"explicitly specify the order of operations")
65+
auto Diag =
66+
Check->diag(StartLoc,
67+
"'%0' has higher precedence than '%1'; add parentheses to "
68+
"explicitly specify the order of operations")
7069
<< (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
7170
: ParentBinOp->getOpcodeStr())
7271
<< (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
7372
: BinOp->getOpcodeStr())
74-
<< FixItHint::CreateInsertion(StartLoc, "(")
75-
<< FixItHint::CreateInsertion(EndLoc, ")")
7673
<< SourceRange(StartLoc, EndLoc);
74+
75+
if (EndLoc.isValid()) {
76+
Diag << FixItHint::CreateInsertion(StartLoc, "(")
77+
<< FixItHint::CreateInsertion(EndLoc, ")");
78+
}
7779
}
7880

7981
addParantheses(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreImpCasts()),

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ int bar(){
1616
return 4;
1717
}
1818

19+
int sink(int);
20+
#define FUN(ARG) (sink(ARG))
21+
#define FUN2(ARG) sink((ARG))
22+
#define FUN3(ARG) sink(ARG)
23+
#define FUN4(ARG) sink(1 + ARG)
24+
#define FUN5(ARG) sink(4 * ARG)
25+
1926
class fun{
2027
public:
2128
int A;
@@ -117,4 +124,19 @@ void f(){
117124
//CHECK-MESSAGES: :[[@LINE+2]]:94: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
118125
//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)));
119126
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
127+
128+
//CHECK-MESSAGES: :[[@LINE+1]]:21: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
129+
int r = FUN(0 + 1 * 2);
130+
131+
//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
132+
int s = FUN2(0 + 1 * 2);
133+
134+
//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
135+
int t = FUN3(0 + 1 * 2);
136+
137+
//CHECK-MESSAGES: :[[@LINE+1]]:18: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
138+
int u = FUN4(1 * 2);
139+
140+
//CHECK-MESSAGES: :[[@LINE+1]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
141+
int v = FUN5(0 + 1);
120142
}

0 commit comments

Comments
 (0)