[clangd] Fix bordering tokens wrong order

[timon-ul]

Fixed an issue where if two tokens are bordering clangd would always prefer the right one, but if the right one is a comma this would lead to unintuitive and wrong results. See issue #2457 for more details. Also adjusted a test to also test for this.

[llvmbot]

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

@llvm/pr-subscribers-clangd

Author: None (timon-ul)

Changes

Fixed an issue where if two tokens are bordering clangd would always prefer the right one, but if the right one is a comma this would lead to unintuitive and wrong results. See issue #2457 for more details. Also adjusted a test to also test for this.

---

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

2 Files Affected:

• (modified) clang-tools-extra/clangd/Selection.cpp (+8-2)
• (modified) clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp (+1-1)

diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index 277cb8769a1b1..fd1db66bbb4a6 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -1039,14 +1039,20 @@ pointBounds(unsigned Offset, const syntax::TokenBuffer &Tokens) {
   const auto &SM = Tokens.sourceManager();
   SourceLocation Loc = SM.getComposedLoc(SM.getMainFileID(), Offset);
   llvm::SmallVector<std::pair<unsigned, unsigned>, 2> Result;
-  // Prefer right token over left.
+  llvm::SmallVector<std::pair<unsigned, unsigned>, 2> ResultLowPrio;
+  // Prefer right token over left, also non comma over comma.
   for (const syntax::Token &Tok :
        llvm::reverse(spelledTokensTouching(Loc, Tokens))) {
     if (shouldIgnore(Tok))
       continue;
     unsigned Offset = Tokens.sourceManager().getFileOffset(Tok.location());
-    Result.emplace_back(Offset, Offset + Tok.length());
+    if (Tok.kind() == tok::comma){
+      ResultLowPrio.emplace_back(Offset, Offset + Tok.length());
+    } else {
+      Result.emplace_back(Offset, Offset + Tok.length());
+    }
   }
+  Result.append(ResultLowPrio);
   if (Result.empty())
     Result.emplace_back(Offset, Offset);
   return Result;
diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
index 08cc80ff8981e..2c0e5f1fed7df 100644
--- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -636,7 +636,7 @@ TEST(CallHierarchy, HierarchyOnVar) {
 TEST(CallHierarchy, HierarchyOnEnumConstant) {
   // Tests that the call hierarchy works on enum constants.
   Annotations Source(R"cpp(
-    enum class Coin { heads$Heads^ , tai$Tails^ls };
+    enum class Coin { heads$Heads^, tai$Tails^ls };
     void caller() {
       Coin::$CallerH[[heads]];
       Coin::$CallerT[[tails]];

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.