Skip to content

Commit e9df6fe

Browse files
felix642felix642
authored
[clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (#76315)
The check currently emits warnings for the following code: `uint64_t fn() { return 1024 * 1024; }` But the code generated after applying the notes will look like this: `uint64_t fn() { return static_cast<uint64_t>(1024 * )1024; }` This is because when generating the notes the check will use the beginLoc() and EndLoc() of the subexpr of the implicit cast. But in some cases the AST Node might not have a beginLoc and EndLoc. This seems to be true when the Node is composed of only 1 token (for example an integer literal). Calling the getEndLoc() on this type of node will simply return the known location which is, in this case, the beginLoc. Fixes #63070 #56728
1 parent 60ac394 commit e9df6fe

5 files changed

+31
-16
lines changed

‎clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "ImplicitWideningOfMultiplicationResultCheck.h"
1010
#include "clang/AST/ASTContext.h"
1111
#include "clang/ASTMatchers/ASTMatchFinder.h"
12+
#include "clang/Lex/Lexer.h"
1213
#include <optional>
1314

1415
using namespace clang::ast_matchers;
@@ -99,15 +100,16 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
99100
"make conversion explicit to silence this warning",
100101
DiagnosticIDs::Note)
101102
<< E->getSourceRange();
102-
103+
const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
104+
E->getEndLoc(), 0, *Result->SourceManager, getLangOpts());
103105
if (ShouldUseCXXStaticCast)
104106
Diag << FixItHint::CreateInsertion(
105107
E->getBeginLoc(), "static_cast<" + Ty.getAsString() + ">(")
106-
<< FixItHint::CreateInsertion(E->getEndLoc(), ")");
108+
<< FixItHint::CreateInsertion(EndLoc, ")");
107109
else
108110
Diag << FixItHint::CreateInsertion(E->getBeginLoc(),
109111
"(" + Ty.getAsString() + ")(")
110-
<< FixItHint::CreateInsertion(E->getEndLoc(), ")");
112+
<< FixItHint::CreateInsertion(EndLoc, ")");
111113
Diag << includeStddefHeader(E->getBeginLoc());
112114
}
113115

@@ -137,7 +139,11 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
137139
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
138140
"static_cast<" +
139141
WideExprTy.getAsString() + ">(")
140-
<< FixItHint::CreateInsertion(LHS->getEndLoc(), ")");
142+
<< FixItHint::CreateInsertion(
143+
Lexer::getLocForEndOfToken(LHS->getEndLoc(), 0,
144+
*Result->SourceManager,
145+
getLangOpts()),
146+
")");
141147
else
142148
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
143149
"(" + WideExprTy.getAsString() + ")");
@@ -206,16 +212,17 @@ void ImplicitWideningOfMultiplicationResultCheck::handlePointerOffsetting(
206212
"make conversion explicit to silence this warning",
207213
DiagnosticIDs::Note)
208214
<< IndexExpr->getSourceRange();
209-
215+
const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
216+
IndexExpr->getEndLoc(), 0, *Result->SourceManager, getLangOpts());
210217
if (ShouldUseCXXStaticCast)
211218
Diag << FixItHint::CreateInsertion(
212219
IndexExpr->getBeginLoc(),
213220
(Twine("static_cast<") + TyAsString + ">(").str())
214-
<< FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
221+
<< FixItHint::CreateInsertion(EndLoc, ")");
215222
else
216223
Diag << FixItHint::CreateInsertion(IndexExpr->getBeginLoc(),
217224
(Twine("(") + TyAsString + ")(").str())
218-
<< FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
225+
<< FixItHint::CreateInsertion(EndLoc, ")");
219226
Diag << includeStddefHeader(IndexExpr->getBeginLoc());
220227
}
221228

@@ -229,7 +236,11 @@ void ImplicitWideningOfMultiplicationResultCheck::handlePointerOffsetting(
229236
Diag << FixItHint::CreateInsertion(
230237
LHS->getBeginLoc(),
231238
(Twine("static_cast<") + TyAsString + ">(").str())
232-
<< FixItHint::CreateInsertion(LHS->getEndLoc(), ")");
239+
<< FixItHint::CreateInsertion(
240+
Lexer::getLocForEndOfToken(IndexExpr->getEndLoc(), 0,
241+
*Result->SourceManager,
242+
getLangOpts()),
243+
")");
233244
else
234245
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
235246
(Twine("(") + TyAsString + ")").str());

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,10 @@ Changes in existing checks
260260
casting during type conversions at variable initialization, now with improved
261261
compatibility for C++17 and later versions.
262262

263+
- Improved :doc:`bugprone-implicit-widening-of-multiplication-result
264+
<clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result>` check
265+
to correctly emit fixes.
266+
263267
- Improved :doc:`bugprone-lambda-function-name
264268
<clang-tidy/checks/bugprone/lambda-function-name>` check by adding option
265269
`IgnoreMacros` to ignore warnings in macros.

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
1818
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
1919
// CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider type
2020
// CHECK-NOTES-C: (ptrdiff_t)
21-
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>()
21+
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
2222
}
2323
void *t1(char *base, int a, int b) {
2424
return &((a * b)[base]);
@@ -35,7 +35,7 @@ char *t2(char *base, unsigned int a, int b) {
3535
// CHECK-NOTES-CXX: static_cast<size_t>( )
3636
// CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider type
3737
// CHECK-NOTES-C: (size_t)
38-
// CHECK-NOTES-CXX: static_cast<size_t>()
38+
// CHECK-NOTES-CXX: static_cast<size_t>( )
3939
}
4040

4141
char *t3(char *base, int a, unsigned int b) {

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ long t0(int a, int b) {
1818
// CHECK-NOTES-CXX: static_cast<long>( )
1919
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
2020
// CHECK-NOTES-C: (long)
21-
// CHECK-NOTES-CXX: static_cast<long>()
21+
// CHECK-NOTES-CXX: static_cast<long>( )
2222
}
2323
unsigned long t1(int a, int b) {
2424
return a * b;
@@ -28,7 +28,7 @@ unsigned long t1(int a, int b) {
2828
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
2929
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
3030
// CHECK-NOTES-C: (long)
31-
// CHECK-NOTES-CXX: static_cast<long>()
31+
// CHECK-NOTES-CXX: static_cast<long>( )
3232
}
3333

3434
long t2(unsigned int a, int b) {
@@ -39,7 +39,7 @@ long t2(unsigned int a, int b) {
3939
// CHECK-NOTES-CXX: static_cast<long>( )
4040
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
4141
// CHECK-NOTES-C: (unsigned long)
42-
// CHECK-NOTES-CXX: static_cast<unsigned long>()
42+
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
4343
}
4444
unsigned long t3(unsigned int a, int b) {
4545
return a * b;
@@ -49,7 +49,7 @@ unsigned long t3(unsigned int a, int b) {
4949
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
5050
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
5151
// CHECK-NOTES-C: (unsigned long)
52-
// CHECK-NOTES-CXX: static_cast<unsigned long>()
52+
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
5353
}
5454

5555
long t4(int a, unsigned int b) {

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
1818
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
1919
// CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider type
2020
// CHECK-NOTES-C: (ptrdiff_t)
21-
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>()
21+
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
2222
}
2323
char *t1(char *base, int a, int b) {
2424
return a * b + base;
@@ -35,7 +35,7 @@ char *t2(char *base, unsigned int a, int b) {
3535
// CHECK-NOTES-CXX: static_cast<size_t>( )
3636
// CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider type
3737
// CHECK-NOTES-C: (size_t)
38-
// CHECK-NOTES-CXX: static_cast<size_t>()
38+
// CHECK-NOTES-CXX: static_cast<size_t>( )
3939
}
4040

4141
char *t3(char *base, int a, unsigned int b) {

0 commit comments

Comments
 (0)