[clang-tools-extra] b764edc - [clangd] Try harder to get accurate ranges for documentSymbols in macros
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 12 16:26:48 PDT 2020
Author: Nathan Ridge
Date: 2020-10-12T19:26:36-04:00
New Revision: b764edc59ff7768e052bc2b9e76e3bb69dd5147b
URL: https://github.com/llvm/llvm-project/commit/b764edc59ff7768e052bc2b9e76e3bb69dd5147b
DIFF: https://github.com/llvm/llvm-project/commit/b764edc59ff7768e052bc2b9e76e3bb69dd5147b.diff
LOG: [clangd] Try harder to get accurate ranges for documentSymbols in macros
Fixes https://github.com/clangd/clangd/issues/500
Differential Revision: https://reviews.llvm.org/D88463
Added:
Modified:
clang-tools-extra/clangd/FindSymbols.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp
index 3169ffd98038..abd03ecb0464 100644
--- a/clang-tools-extra/clangd/FindSymbols.cpp
+++ b/clang-tools-extra/clangd/FindSymbols.cpp
@@ -171,7 +171,6 @@ 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 =
@@ -179,10 +178,6 @@ llvm::Optional<DocumentSymbol> declToSym(ASTContext &Ctx, const NamedDecl &ND) {
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;
diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
index a7a0188a85fd..43658284937e 100644
--- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ b/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) {
More information about the cfe-commits
mailing list