7 files changed, 48 insertions, 33 deletions
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 843cac4f36c..47c8182a3bc 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -8,6 +8,7 @@
 
 #include "AST.h"
 
+#include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
@@ -42,34 +43,13 @@ getTemplateSpecializationArgLocs(const NamedDecl &ND) {
 }
 } // namespace
 
-// Returns true if the complete name of decl \p D is spelled in the source code.
-// This is not the case for:
-//   * symbols formed via macro concatenation, the spelling location will
-//     be "<scratch space>"
-//   * symbols controlled and defined by a compile command-line option
-//     `-DName=foo`, the spelling location will be "<command line>".
-bool isSpelledInSourceCode(const Decl *D) {
-  const auto &SM = D->getASTContext().getSourceManager();
-  auto Loc = D->getLocation();
-  // FIXME: Revisit the strategy, the heuristic is limitted when handling
-  // macros, we should use the location where the whole definition occurs.
-  if (Loc.isMacroID()) {
-    std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
-    if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
-        llvm::StringRef(PrintLoc).startswith("<command line>"))
-      return false;
-  }
-  return true;
+bool isImplementationDetail(const Decl *D) {
+  return !isSpelledInSource(D->getLocation(),
+                            D->getASTContext().getSourceManager());
 }
 
-bool isImplementationDetail(const Decl *D) { return !isSpelledInSourceCode(D); }
-
-SourceLocation findNameLoc(const clang::Decl *D) {
-  const auto &SM = D->getASTContext().getSourceManager();
-  if (!isSpelledInSourceCode(D))
-    // Use the expansion location as spelling location is not interesting.
-    return SM.getExpansionRange(D->getLocation()).getBegin();
-  return SM.getSpellingLoc(D->getLocation());
+SourceLocation findName(const clang::Decl *D) {
+  return D->getLocation();
 }
 
 std::string printQualifiedName(const NamedDecl &ND) {
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index f4ca14ad01a..9b428946a2a 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -33,7 +33,7 @@ bool isImplementationDetail(const Decl *D);
 ///
 /// The returned location is usually the spelling location where the name of the
 /// decl occurs in the code.
-SourceLocation findNameLoc(const clang::Decl *D);
+SourceLocation findName(const clang::Decl *D);
 
 /// Returns the qualified name of ND. The scope doesn't contain unwritten scopes
 /// like inline namespaces.
diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp
index 5e806fd7932..fcd725faef8 100644
--- a/clang-tools-extra/clangd/FindSymbols.cpp
+++ b/clang-tools-extra/clangd/FindSymbols.cpp
@@ -138,7 +138,7 @@ namespace {
 llvm::Optional<DocumentSymbol> declToSym(ASTContext &Ctx, const NamedDecl &ND) {
   auto &SM = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = findNameLoc(&ND);
+  SourceLocation NameLoc = spellingLocIfSpelled(findName(&ND), SM);
   // getFileLoc is a good choice for us, but we also need to make sure
   // sourceLocToPosition won't switch files, so we call getSpellingLoc on top of
   // that to make sure it does not switch files.
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index f8057be416c..7a2f7c78a73 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -200,6 +200,24 @@ Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc) {
   return P;
 }
 
+bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
+  if (Loc.isMacroID()) {
+    std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
+    if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
+        llvm::StringRef(PrintLoc).startswith("<command line>"))
+      return false;
+  }
+  return true;
+}
+
+SourceLocation spellingLocIfSpelled(SourceLocation Loc,
+                                    const SourceManager &SM) {
+  if (!isSpelledInSource(Loc, SM))
+    // Use the expansion location as spelling location is not interesting.
+    return SM.getExpansionRange(Loc).getBegin();
+  return SM.getSpellingLoc(Loc);
+}
+
 llvm::Optional<Range> getTokenRange(const SourceManager &SM,
                                     const LangOptions &LangOpts,
                                     SourceLocation TokLoc) {
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index 4bbf3cf49fc..6517043759f 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -83,6 +83,20 @@ llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
 /// the main file.
 bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM);
 
+/// Returns true if the token at Loc is spelled in the source code.
+/// This is not the case for:
+///   * symbols formed via macro concatenation, the spelling location will
+///     be "<scratch space>"
+///   * symbols controlled and defined by a compile command-line option
+///     `-DName=foo`, the spelling location will be "<command line>".
+bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM);
+
+/// Returns the spelling location of the token at Loc if isSpelledInSource,
+/// otherwise its expansion location.
+/// FIXME: Most callers likely want some variant of "file location" instead.
+SourceLocation spellingLocIfSpelled(SourceLocation Loc,
+                                    const SourceManager &SM);
+
 /// Turns a token range into a half-open range and checks its correctness.
 /// The resulting range will have only valid source location on both sides, both
 /// of which are file locations.
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index b520c4daeba..fba04bb8936 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -308,7 +308,9 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
 
   // Emit all symbol locations (declaration or definition) from AST.
   for (const Decl *D : Symbols.Decls) {
-    auto Loc = makeLocation(AST.getASTContext(), findNameLoc(D), *MainFilePath);
+    auto Loc =
+        makeLocation(AST.getASTContext(), spellingLocIfSpelled(findName(D), SM),
+                     *MainFilePath);
     if (!Loc)
       continue;
 
@@ -1044,7 +1046,8 @@ static llvm::Optional<TypeHierarchyItem>
 declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND) {
   auto &SM = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = findNameLoc(&ND);
+  SourceLocation NameLoc =
+      spellingLocIfSpelled(findName(&ND), Ctx.getSourceManager());
   // getFileLoc is a good choice for us, but we also need to make sure
   // sourceLocToPosition won't switch files, so we call getSpellingLoc on top of
   // that to make sure it does not switch files.
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index cc4217ad741..5e3e1c21b04 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -81,7 +81,7 @@ static const char *PROTO_HEADER_COMMENT =
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = findNameLoc(&ND);
+  auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   auto FileName = SM.getFilename(Loc);
   if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
     return false;
@@ -587,7 +587,7 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID,
     S.Flags |= Symbol::VisibleOutsideFile;
   S.SymInfo = index::getSymbolInfo(&ND);
   std::string FileURI;
-  auto Loc = findNameLoc(&ND);
+  auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
   // FIXME: use the result to filter out symbols.
   shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
@@ -647,8 +647,8 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
   // in clang::index. We should only see one definition.
   Symbol S = DeclSym;
   std::string FileURI;
-  auto Loc = findNameLoc(&ND);
   const auto &SM = ND.getASTContext().getSourceManager();
+  auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   // FIXME: use the result to filter out symbols.
   shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
   if (auto DefLoc =