[clangd] Try harder to get accurate ranges for documentSymbols in macros

Fixes clangd/clangd#500

Differential Revision: https://reviews.llvm.org/D88463

clang-tools-extra/clangd/FindSymbols.cpp

@@ -171,18 +171,13 @@ namespace {
 llvm::Optional<DocumentSymbol> declToSym(ASTContext &Ctx, const NamedDecl &ND) {
   auto &SM = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = nameLocation(ND, SM);
   SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc()));
   SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc()));
   const auto SymbolRange =
       toHalfOpenFileRange(SM, Ctx.getLangOpts(), {BeginLoc, EndLoc});
   if (!SymbolRange)
     return llvm::None;
 
-  Position NameBegin = sourceLocToPosition(SM, NameLoc);
-  Position NameEnd = sourceLocToPosition(
-      SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts()));
-
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
   // FIXME: this is not classifying constructors, destructors and operators
   //        correctly (they're all "methods").
@@ -194,10 +189,35 @@ llvm::Optional<DocumentSymbol> declToSym(ASTContext &Ctx, const NamedDecl &ND) {
   SI.deprecated = ND.isDeprecated();
   SI.range = Range{sourceLocToPosition(SM, SymbolRange->getBegin()),
                    sourceLocToPosition(SM, SymbolRange->getEnd())};
-  SI.selectionRange = Range{NameBegin, NameEnd};
+
+  SourceLocation NameLoc = ND.getLocation();
+  SourceLocation FallbackNameLoc;
+  if (NameLoc.isMacroID()) {
+    if (isSpelledInSource(NameLoc, SM)) {
+      // Prefer the spelling loc, but save the expansion loc as a fallback.
+      FallbackNameLoc = SM.getExpansionLoc(NameLoc);
+      NameLoc = SM.getSpellingLoc(NameLoc);
+    } else {
+      NameLoc = SM.getExpansionLoc(NameLoc);
+    }
+  }
+  auto ComputeSelectionRange = [&](SourceLocation L) -> Range {
+    Position NameBegin = sourceLocToPosition(SM, L);
+    Position NameEnd = sourceLocToPosition(
+        SM, Lexer::getLocForEndOfToken(L, 0, SM, Ctx.getLangOpts()));
+    return Range{NameBegin, NameEnd};
+  };
+
+  SI.selectionRange = ComputeSelectionRange(NameLoc);
+  if (!SI.range.contains(SI.selectionRange) && FallbackNameLoc.isValid()) {
+    // 'selectionRange' must be contained in 'range'. In cases where clang
+    // reports unrelated ranges, we first try falling back to the expansion
+    // loc for the selection range.
+    SI.selectionRange = ComputeSelectionRange(FallbackNameLoc);
+  }
   if (!SI.range.contains(SI.selectionRange)) {
-    // 'selectionRange' must be contained in 'range', so in cases where clang
-    // reports unrelated ranges we need to reconcile somehow.
+    // If the containment relationship still doesn't hold, throw away
+    // 'range' and use 'selectionRange' for both.
     SI.range = SI.selectionRange;
   }
   return SI;

clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp

@@ -639,19 +639,27 @@ TEST(DocumentSymbols, FromMacro) {
     #define FF(name) \
       class name##_Test {};
 
-    $expansion[[FF]](abc);
+    $expansion1[[FF]](abc);
 
     #define FF2() \
-      class $spelling[[Test]] {};
+      class Test {};
 
-    FF2();
+    $expansion2[[FF2]]();
+
+    #define FF3() \
+      void waldo()
+
+    $fullDef[[FF3() {
+      int var = 42;
+    }]]
   )");
   TU.Code = Main.code().str();
   EXPECT_THAT(
       getSymbols(TU.build()),
       ElementsAre(
-          AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion"))),
-          AllOf(WithName("Test"), SymNameRange(Main.range("spelling")))));
+          AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion1"))),
+          AllOf(WithName("Test"), SymNameRange(Main.range("expansion2"))),
+          AllOf(WithName("waldo"), SymRange(Main.range("fullDef")))));
 }
 
 TEST(DocumentSymbols, FuncTemplates) {