Skip to content

Commit

Permalink
[clang-tidy] Invalid Fix-It generated for implicit-widening-multiplic…
Browse files Browse the repository at this point in the history 
…ation-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
  • Loading branch information
@felix642
felix642 committed Jan 13, 2024
1 parent 60ac394 commit e9df6fe
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 16 deletions.
27 changes: 19 additions & 8 deletions clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "ImplicitWideningOfMultiplicationResultCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
#include <optional>

using namespace clang::ast_matchers;
Expand Down Expand Up @@ -99,15 +100,16 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
"make conversion explicit to silence this warning",
DiagnosticIDs::Note)
<< E->getSourceRange();

const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
E->getEndLoc(), 0, *Result->SourceManager, getLangOpts());
if (ShouldUseCXXStaticCast)
Diag << FixItHint::CreateInsertion(
E->getBeginLoc(), "static_cast<" + Ty.getAsString() + ">(")
<< FixItHint::CreateInsertion(E->getEndLoc(), ")");
<< FixItHint::CreateInsertion(EndLoc, ")");
else
Diag << FixItHint::CreateInsertion(E->getBeginLoc(),
"(" + Ty.getAsString() + ")(")
<< FixItHint::CreateInsertion(E->getEndLoc(), ")");
<< FixItHint::CreateInsertion(EndLoc, ")");
Diag << includeStddefHeader(E->getBeginLoc());
}

Expand Down Expand Up @@ -137,7 +139,11 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
"static_cast<" +
WideExprTy.getAsString() + ">(")
<< FixItHint::CreateInsertion(LHS->getEndLoc(), ")");
<< FixItHint::CreateInsertion(
Lexer::getLocForEndOfToken(LHS->getEndLoc(), 0,
*Result->SourceManager,
getLangOpts()),
")");
else
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
"(" + WideExprTy.getAsString() + ")");
Expand Down Expand Up @@ -206,16 +212,17 @@ void ImplicitWideningOfMultiplicationResultCheck::handlePointerOffsetting(
"make conversion explicit to silence this warning",
DiagnosticIDs::Note)
<< IndexExpr->getSourceRange();

const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
IndexExpr->getEndLoc(), 0, *Result->SourceManager, getLangOpts());
if (ShouldUseCXXStaticCast)
Diag << FixItHint::CreateInsertion(
IndexExpr->getBeginLoc(),
(Twine("static_cast<") + TyAsString + ">(").str())
<< FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
<< FixItHint::CreateInsertion(EndLoc, ")");
else
Diag << FixItHint::CreateInsertion(IndexExpr->getBeginLoc(),
(Twine("(") + TyAsString + ")(").str())
<< FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
<< FixItHint::CreateInsertion(EndLoc, ")");
Diag << includeStddefHeader(IndexExpr->getBeginLoc());
}

Expand All @@ -229,7 +236,11 @@ void ImplicitWideningOfMultiplicationResultCheck::handlePointerOffsetting(
Diag << FixItHint::CreateInsertion(
LHS->getBeginLoc(),
(Twine("static_cast<") + TyAsString + ">(").str())
<< FixItHint::CreateInsertion(LHS->getEndLoc(), ")");
<< FixItHint::CreateInsertion(
Lexer::getLocForEndOfToken(IndexExpr->getEndLoc(), 0,
*Result->SourceManager,
getLangOpts()),
")");
else
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
(Twine("(") + TyAsString + ")").str());
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ Changes in existing checks
casting during type conversions at variable initialization, now with improved
compatibility for C++17 and later versions.

- Improved :doc:`bugprone-implicit-widening-of-multiplication-result
<clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result>` check
to correctly emit fixes.

- Improved :doc:`bugprone-lambda-function-name
<clang-tidy/checks/bugprone/lambda-function-name>` check by adding option
`IgnoreMacros` to ignore warnings in macros.
Expand Down
4 changes: 2 additions & 2 deletions ...eckers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider type
// CHECK-NOTES-C: (ptrdiff_t)
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>()
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
}
void *t1(char *base, int a, int b) {
return &((a * b)[base]);
Expand All @@ -35,7 +35,7 @@ char *t2(char *base, unsigned int a, int b) {
// CHECK-NOTES-CXX: static_cast<size_t>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider type
// CHECK-NOTES-C: (size_t)
// CHECK-NOTES-CXX: static_cast<size_t>()
// CHECK-NOTES-CXX: static_cast<size_t>( )
}

char *t3(char *base, int a, unsigned int b) {
Expand Down
8 changes: 4 additions & 4 deletions ...xtra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ long t0(int a, int b) {
// CHECK-NOTES-CXX: static_cast<long>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
// CHECK-NOTES-C: (long)
// CHECK-NOTES-CXX: static_cast<long>()
// CHECK-NOTES-CXX: static_cast<long>( )
}
unsigned long t1(int a, int b) {
return a * b;
Expand All @@ -28,7 +28,7 @@ unsigned long t1(int a, int b) {
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
// CHECK-NOTES-C: (long)
// CHECK-NOTES-CXX: static_cast<long>()
// CHECK-NOTES-CXX: static_cast<long>( )
}

long t2(unsigned int a, int b) {
Expand All @@ -39,7 +39,7 @@ long t2(unsigned int a, int b) {
// CHECK-NOTES-CXX: static_cast<long>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
// CHECK-NOTES-C: (unsigned long)
// CHECK-NOTES-CXX: static_cast<unsigned long>()
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
}
unsigned long t3(unsigned int a, int b) {
return a * b;
Expand All @@ -49,7 +49,7 @@ unsigned long t3(unsigned int a, int b) {
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type
// CHECK-NOTES-C: (unsigned long)
// CHECK-NOTES-CXX: static_cast<unsigned long>()
// CHECK-NOTES-CXX: static_cast<unsigned long>( )
}

long t4(int a, unsigned int b) {
Expand Down
4 changes: 2 additions & 2 deletions ...lang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider type
// CHECK-NOTES-C: (ptrdiff_t)
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>()
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
}
char *t1(char *base, int a, int b) {
return a * b + base;
Expand All @@ -35,7 +35,7 @@ char *t2(char *base, unsigned int a, int b) {
// CHECK-NOTES-CXX: static_cast<size_t>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider type
// CHECK-NOTES-C: (size_t)
// CHECK-NOTES-CXX: static_cast<size_t>()
// CHECK-NOTES-CXX: static_cast<size_t>( )
}

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

0 comments on commit e9df6fe

Please sign in to comment.