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 @@ -132,7 +132,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 = @@ -140,10 +139,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"). @@ -155,10 +150,34 @@ SI.deprecated = ND.isDeprecated(); SI.range = Range{sourceLocToPosition(SM, SymbolRange->getBegin()), sourceLocToPosition(SM, SymbolRange->getEnd())}; - SI.selectionRange = Range{NameBegin, NameEnd}; + + SourceLocation Loc = ND.getLocation(); + SourceLocation NameLoc; + SourceLocation FallbackNameLoc; + if (isSpelledInSource(Loc, SM)) { + // Prefer the spelling loc, but save the expansion loc as a fallback. + NameLoc = SM.getSpellingLoc(Loc); + FallbackNameLoc = SM.getExpansionLoc(Loc); + } else { + NameLoc = SM.getExpansionLoc(Loc); + } + 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 @@ -36,6 +36,7 @@ MATCHER_P(WithName, N, "") { return arg.name == N; } MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; } MATCHER_P(SymRange, Range, "") { return arg.range == Range; } +MATCHER_P(SymRangeContains, Range, "") { return arg.range.contains(Range); } // GMock helpers for matching DocumentSymbol. MATCHER_P(SymNameRange, Range, "") { return arg.selectionRange == Range; } @@ -613,19 +614,28 @@ #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() + + FF3() { + $bodyStatement[[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"), + SymRangeContains(Main.range("bodyStatement"))))); } TEST(DocumentSymbols, FuncTemplates) {