[clang-tools-extra] e9df6fe - [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (#76315)

via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 13 10:07:15 PST 2024


Author: Félix-Antoine Constantin
Date: 2024-01-13T19:07:11+01:00
New Revision: e9df6fec59b3ea9bc7f66236bc94517bcb00f15a

URL: https://github.com/llvm/llvm-project/commit/e9df6fec59b3ea9bc7f66236bc94517bcb00f15a
DIFF: https://github.com/llvm/llvm-project/commit/e9df6fec59b3ea9bc7f66236bc94517bcb00f15a.diff

LOG: [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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp
    clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
    clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
index 95a7c521eabb0e..6f22f02f301835 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
@@ -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;
@@ -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());
   }
 
@@ -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() + ")");
@@ -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());
   }
 
@@ -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());

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3437b6cf9b59e9..f202e051aad169 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -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.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp
index 2881341c878570..5b432cb2662968 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp
@@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
   // CHECK-NOTES-CXX:                  static_cast<ptr
diff _t>( )
   // CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider type
   // CHECK-NOTES-C:                    (ptr
diff _t)
-  // CHECK-NOTES-CXX:                  static_cast<ptr
diff _t>()
+  // CHECK-NOTES-CXX:                  static_cast<ptr
diff _t>( )
 }
 void *t1(char *base, int a, int b) {
   return &((a * b)[base]);
@@ -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) {

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
index cf13a7ea9fbd8b..8619a4ee86550a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
@@ -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;
@@ -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) {
@@ -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;
@@ -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) {

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp
index c812885507e8f3..5ba6a5639849df 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-pointer-offset.cpp
@@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
   // CHECK-NOTES-CXX:                  static_cast<ptr
diff _t>( )
   // CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider type
   // CHECK-NOTES-C:                    (ptr
diff _t)
-  // CHECK-NOTES-CXX:                  static_cast<ptr
diff _t>()
+  // CHECK-NOTES-CXX:                  static_cast<ptr
diff _t>( )
 }
 char *t1(char *base, int a, int b) {
   return a * b + base;
@@ -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) {


        


More information about the cfe-commits mailing list