[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)

via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 3 16:48:04 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Richard Li (chomosuke)

<details>
<summary>Changes</summary>

The problem:
When running the code action for `mordernize-use-ranges` on `std::reverse(v.begin(), v.end())`, the code gets modified to `std::reverse(v,)` instead of `std::reverse(v)`. This PR fixes both `mordernize-use-ranges` and `boost-use-ranges`.

The more general problem:
This does not show up in tests for clang-tidy because `clang-tidy --fix` runs `format::cleanupAroundReplacements()` before committing the fixes where as clangd does not. There are many other clang-tidy check that work with `clant-tidy --fix` but does not work with clangd code action. I will soon open an PR that fix this in the general case by running `format::cleanupAroundReplacements()` in clangd.


---
Full diff: https://github.com/llvm/llvm-project/pull/118568.diff


1 Files Affected:

- (modified) clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp (+10-9) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index aba4d17ccd035e..88cba70b931d5d 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -28,6 +28,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
+#include <iostream>
 #include <optional>
 #include <string>
 
@@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
   for (unsigned Index : Sorted) {
     const Expr *Arg = Call.getArg(Index);
     if (Commas[Index]) {
-      if (Index >= Commas.size()) {
-        Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
-      } else {
+      if (Index + 1 < Call.getNumArgs()) {
         // Remove the next comma
         Commas[Index + 1] = true;
+        const Expr *NextArg = Call.getArg(Index + 1);
         Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-            {Arg->getBeginLoc(),
-             Lexer::getLocForEndOfToken(
-                 Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts())
-                 .getLocWithOffset(1)}));
+            {Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)}));
+      } else {
+        Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
       }
     } else {
-      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-          Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc()));
+      // At this point we know Index > 0 because `Commas[0] = true` earlier
       Commas[Index] = true;
+      const Expr *PrevArg = Call.getArg(Index - 1);
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+          PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc()));
     }
   }
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/118568


More information about the cfe-commits mailing list