[clangd] XRef C++ API returns structure results, NFC

[hokein]

Summary

The representation of references in LSP is limitted (just location
information). This patch makes our xref C++ API return structured results,
which allows other clients get more data for references.

[hokein]

As discussed offline, we don't have a clear motivation to do it now, and LSP will likely introduce a new References type of xrefs (https://github.com/Microsoft/language-server-protocol/issues/396), so I'll leave this patch opened.

clangd/ClangdLSPServer.cpp

 
 // FIXME: This function needs to be properly tested.
 void ClangdLSPServer::onChangeConfiguration(
     DidChangeConfigurationParams &Params) {
   applyConfiguration(Params.settings);
 }
 
 void ClangdLSPServer::onReference(ReferenceParams &Params) {
-  Server->findReferences(Params.textDocument.uri.file(), Params.position,
-                         [](llvm::Expected<std::vector<Location>> Locations) {
-                           if (!Locations)
-                             return replyError(
-                                 ErrorCode::InternalError,
-                                 llvm::toString(Locations.takeError()));
-                           reply(llvm::json::Array(*Locations));
-                         });
+  std::string Path = Params.textDocument.uri.file();
+  Server->findReferences(
+      Path, Params.position, [&Path](llvm::Expected<RefResult> Refs) {
+        if (!Refs)
+          return replyError(ErrorCode::InternalError,
+                            llvm::toString(Refs.takeError()));
+        reply(llvm::json::Array(Refs->render(Path)));
 }
 
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
                                  const clangd::CodeCompleteOptions &CCOpts,
                                  llvm::Optional<Path> CompileCommandsDir,
                                  bool ShouldUseInMemoryCDB,
                                  const ClangdServer::Options &Opts)
     : Out(Out), CDB(ShouldUseInMemoryCDB ? CompilationDB::makeInMemory()

clangd/ClangdServer.h

 #include "Cancellation.h"
 #include "ClangdUnit.h"
 #include "CodeComplete.h"
 #include "FSProvider.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "TUScheduler.h"
+#include "XRefs.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include <functional>
public:
   void workspaceSymbols(StringRef Query, int Limit,
                         Callback<std::vector<SymbolInformation>> CB);
 
   /// Retrieve the symbols within the specified file.
   void documentSymbols(StringRef File,
                        Callback<std::vector<SymbolInformation>> CB);
 
   /// Retrieve locations for symbol references.
-  void findReferences(PathRef File, Position Pos,
-                      Callback<std::vector<Location>> CB);
+  void findReferences(PathRef File, Position Pos, Callback<RefResult> CB);
 
   /// Run formatting for \p Rng inside \p File with content \p Code.
   llvm::Expected<tooling::Replacements> formatRange(StringRef Code,
                                                     PathRef File, Range Rng);
 
   /// Run formatting for the whole \p File with content \p Code.
   llvm::Expected<tooling::Replacements> formatFile(StringRef Code,
                                                    PathRef File);

clangd/ClangdServer.cpp

if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::getDocumentSymbols(InpAST->AST));
   };
   WorkScheduler.runWithAST("documentSymbols", File,
                            Bind(Action, std::move(CB)));
 }
 
 void ClangdServer::findReferences(PathRef File, Position Pos,
-                                  Callback<std::vector<Location>> CB) {
-  auto Action = [Pos, this](Callback<std::vector<Location>> CB,
+                                  Callback<RefResult> CB) {
+  auto Action = [Pos, this](Callback<RefResult> CB,
                             llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::findReferences(InpAST->AST, Pos, Index));
   };
 
   WorkScheduler.runWithAST("References", File, Bind(Action, std::move(CB)));
 }

clangd/XRefs.h

 
 /// Returns highlights for all usages of a symbol at \p Pos.
 std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
                                                       Position Pos);
 
 /// Get the hover information when hovering at \p Pos.
 llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
 
-/// Returns reference locations of the symbol at a specified \p Pos.
-std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
-                                     const SymbolIndex *Index = nullptr);
+/// Represents a list of references for clangd C++ API.
+/// We don't use the LSP structures here as LSP ref representation is limitted,
+/// and we want other C++ API users (unlike ClangdLSPServer) to get more
+/// information about references.
+struct RefResult {
+  std::vector<Ref> Refs;
+  // Storage for the underlying data.
+  llvm::StringSet<> Storage;
+  // Serialize to a list of LSP locations.
+  std::vector<Location> render(llvm::StringRef HintPath) const;
+};
+/// Returns symbol references at a specified \p Pos.
+RefResult findReferences(ParsedAST &AST, Position Pos,
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H

clangd/XRefs.cpp

Optional<Hover> getHover(ParsedAST &AST, Position Pos) {
 
   auto DeducedType = getDeducedType(AST, SourceLocationBeg);
   if (DeducedType && !DeducedType->isNull())
     return getHoverContents(*DeducedType, AST.getASTContext());
 
   return None;
 }
 
-std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
-                                     const SymbolIndex *Index) {
+std::vector<Location> RefResult::render(llvm::StringRef HintPath) const {
   std::vector<Location> Results;
+  for (auto &R : Refs) {
+    if (auto LSPLoc = toLSPLocation(R.Location, HintPath))
+      Results.push_back(std::move(*LSPLoc));
+  }
+  return Results;
+}
+
+RefResult findReferences(ParsedAST &AST, Position Pos,
+                         const SymbolIndex *Index) {
+  RefResult Results;
   const SourceManager &SM = AST.getASTContext().getSourceManager();
   auto MainFilePath = getRealPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
   if (!MainFilePath) {
     elog("Failed to get a path for the main file, so no references");
     return Results;
   }
   auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
   auto Symbols = getSymbolAtPosition(AST, Loc);
 
   std::vector<const Decl *> TargetDecls;
   for (const DeclInfo &DI : Symbols.Decls) {
     if (DI.IsReferencedExplicitly)
       TargetDecls.push_back(DI.D);
   }
 
+  auto Intern = [&Results](llvm::StringRef S) {
+    auto It = Results.Storage.find(S);
+    if (It == Results.Storage.end())
+      It = Results.Storage.insert(S).first;
+    return It->first();
+  };
+
   // We traverse the AST to find references in the main file.
   // TODO: should we handle macros, too?
   auto MainFileRefs = findRefs(TargetDecls, AST);
   for (const auto &Ref : MainFileRefs) {
-    Location Result;
-    Result.range = getTokenRange(AST, Ref.Loc);
-    Result.uri = URIForFile(*MainFilePath);
-    Results.push_back(std::move(Result));
+    clang::clangd::Ref R;
+    auto Range = getTokenRange(AST, Ref.Loc);
+    std::tie(R.Location.Start.Line, R.Location.Start.Column) =
+        std::tie(Range.start.line, Range.start.character);
+    std::tie(R.Location.End.Line, R.Location.End.Column) =
+        std::tie(Range.end.line, Range.end.character);
+    std::string FileURI = URIForFile(*MainFilePath).uri();
+    R.Location.FileURI = Intern(FileURI);
+    Results.Refs.push_back(R);
   }
 
   // Now query the index for references from other files.
   if (!Index)
     return Results;
   RefsRequest Req;
   for (const Decl *D : TargetDecls) {
     // Not all symbols can be referenced from outside (e.g. function-locals).
     // TODO: we could skip TU-scoped symbols here (e.g. static functions) if
     // we know this file isn't a header. The details might be tricky.
     if (D->getParentFunctionOrMethod())
       continue;
     if (auto ID = getSymbolID(D))
       Req.IDs.insert(*ID);
   }
   if (Req.IDs.empty())
     return Results;
   Index->refs(Req, [&](const Ref &R) {
     auto LSPLoc = toLSPLocation(R.Location, /*HintPath=*/*MainFilePath);
     // Avoid indexed results for the main file - the AST is authoritative.
-    if (LSPLoc && LSPLoc->uri.file() != *MainFilePath)
-      Results.push_back(std::move(*LSPLoc));
+    if (LSPLoc && LSPLoc->uri.file() != *MainFilePath) {
+      Ref NewR = R;
+      NewR.Location.FileURI = Intern(R.Location.FileURI);
+      Results.Refs.push_back(NewR);
+    }
   });
   return Results;
 }
 
 } // namespace clangd
 } // namespace clang

unittests/clangd/XRefsTests.cpp

const char *Tests[] = {
       )cpp",
   };
   for (const char *Test : Tests) {
     Annotations T(Test);
     auto AST = TestTU::withCode(T.code()).build();
     std::vector<Matcher<Location>> ExpectedLocations;
     for (const auto &R : T.ranges())
       ExpectedLocations.push_back(RangeIs(R));
-    EXPECT_THAT(findReferences(AST, T.point()),
+    EXPECT_THAT(findReferences(AST, T.point()).render(""),
                 ElementsAreArray(ExpectedLocations))
         << Test;
   }
 }
 
 TEST(FindReferences, NeedsIndex) {
   const char *Header = "int foo();";
   Annotations Main("int main() { [[f^oo]](); }");
   TestTU TU;
   TU.Code = Main.code();
   TU.HeaderCode = Header;
   auto AST = TU.build();
 
   // References in main file are returned without index.
-  EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr),
+  EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr).render(""),
               ElementsAre(RangeIs(Main.range())));
   Annotations IndexedMain(R"cpp(
     int main() { [[f^oo]](); }
   )cpp");
 
   // References from indexed files are included.
   TestTU IndexedTU;
   IndexedTU.Code = IndexedMain.code();
   IndexedTU.Filename = "Indexed.cpp";
   IndexedTU.HeaderCode = Header;
-  EXPECT_THAT(findReferences(AST, Main.point(), IndexedTU.index().get()),
-              ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range())));
+  EXPECT_THAT(
+      findReferences(AST, Main.point(), IndexedTU.index().get()).render(""),
 
   // If the main file is in the index, we don't return duplicates.
   // (even if the references are in a different location)
   TU.Code = ("\n\n" + Main.code()).str();
-  EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()).render(""),
               ElementsAre(RangeIs(Main.range())));
 }
 
 TEST(FindReferences, NoQueryForLocalSymbols) {
   struct RecordingIndex : public MemIndex {
     mutable Optional<DenseSet<SymbolID>> RefIDs;
     void refs(const RefsRequest &Req,
               llvm::function_ref<void(const Ref &)>) const override {