diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -171,7 +171,6 @@ llvm::Optional 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 = @@ -179,10 +178,6 @@ 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 @@ 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; diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -639,19 +639,27 @@ #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) {