[clangd] Add more symbol information for code completion.

[sammccall]

clangd/index/Index.h

130

AFAIK this information isn't needed for retrieval/scoring, just for display.

LSP has completionItem/resolve that adds additional info to a completion item. This allows us to avoid sending a bunch of bulky comments, most of which will never be looked at.

In practice, there's nothing we particularly want to do differently for the memory index: we have to load the data into memory, and so including a pointer to it right away is no extra work.

However Symbol has two roles, and being the in-memory representation for MemIndex is only secondary. Its primary role is defining the protocol between indexes and clangd, including remote indexes where returning all documentation *is* expensive.

One option is to have Symbol just have the "core stuff" that's suitable for returning in bulk, and have an index method to retrieve more details that would be a point lookup only. (Maybe this is just the getSymbol method we've thought about). I'm not sure what it means for the data structure. OK if we discuss offline?

137

What are you planning to put here other than the return type of a function?
It's probably OK if we explicitly want this to be "whatever should go in the LSP detail field" but we should think about whether there's something more specific we can say.

138

How are you planning to use this?

This seems to be related to the completion text/edits. We had some early discussions about whether we'd encode something like CodeCompletionString, or LSP snippets, or something else entirely.
Honestly it would be great to have a doc describing this mapping between source -> index -> LSP for completion data.

clangd/index/SymbolCollector.cpp

58

it seems we'll want to share the(some?) doc logic between hover, AST-based complete, and indexing... See D35894 (which is ... complicated, no idea if it'll land soon).

Among other things:

• we may not want to make the logic too elaborate until we're able to merge interfaces
• we may want to consume AST nodes rather than CCS in the long run

64

Should these annotations go at the end?

87

nit: lowercase

87

How does this relate to the code in AST-based completion?

[ioeric]

• Merge with origin/master
• Fixed an error in merge
• Make documentation etc optional in symbols
• Merge remote-tracking branch 'origin/master' into symbol
• Merge branch 'index-completion' into symbol
• Address review comments; merge with D41450.

[ioeric]

Thanks for the review!

Logic around CodeCompletionString is pulled into a separate file in D41450.

I also propagate the new completion info to completion code. I am happy to split it out if it adds too much noise for the review.

clangd/index/Index.h

130

As discussed offline, putting non-core stuff in an optional structure.

138

As discussed offline, we now store the whole snippet in the insertion text.

clangd/index/SymbolCollector.cpp

58

I pulled CodeCompletionString handling logic into a separate file in D41450.

[sammccall]

clangd/index/Index.h

139

insert texts can be in details I think? They're not required for completion until after resolve.

156

I think you probably want a raw pointer rather than optional:

• reduce the size of the struct when it's absent
• make it inheritance-friendly so we can hang index-specific info off it

(raw pointer rather than unique_ptr because it's owned by a slab not by malloc, but unique_ptr is ok for now)

clangd/index/SymbolCollector.cpp

60

this doesn't seem like the kind of thing we should be allocating per-symbol!
You can use a single one and Reset() it, I think

also why globalcodecompletionallocator, vs just codecompletionallocator?

[ioeric]

• Merged with origin/master
• Addressed some more comments.
• Add new fields to YAML.

clangd/index/Index.h

139

Quote from https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request

The request can delay the computation od the detail and documentation properties. However, properties that are needed for the inital sorting and filtering, like sortText, filterText, insertText, and textEdit must be provided in the textDocument/completion request and must not be changed during resolve.

156

This is not easy for now with unique_ptr because of this line :( https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141).

This shouldn't be an issue when we have the optimized symbol slab, where we store raw pointers. And we would probably want to serialize the whole slab instead of the individual symbols anyway.

reduce the size of the struct when it's absent

llvm::Optional doesn't take much more space, so the size should be fine.

make it inheritance-friendly so we can hang index-specific info off it

Could you elaborate on index-specific info? It's unclear to me how this would be used.

clangd/index/SymbolCollector.cpp

60

You can use a single one and Reset() it, I think

It's unclear how reset would affect CodeCompletionTUInfo, but we can simply maintain one allocator for each TU.

also why globalcodecompletionallocator, vs just codecompletionallocator?

They are actually the same thing, but CodeCompletionTUInfo requires a global one. I don't really know why...

[sammccall]

clangd/index/Index.h

156

This is not easy for now with unique_ptr because of this line

Oh no, somehow i missed this during review.
We shouldn't be relying on symbols being copyable. I'll try to work out how to fix this and delete the copy constructor.

This shouldn't be an issue when we have the optimized symbol slab, where we store raw pointers.

Sure. That's not a big monolithic/mysterous thing though, storing the details in the slab can be done in this patch... If you think it'll be easier once strings are arena-based, then maybe we should delay this patch until that's done, rather than make that work bigger.

And we would probably want to serialize the whole slab instead of the individual symbols anyway.

This i'm less sure about, but I don't think it matters.

llvm::Optional doesn't take much more space, so the size should be fine.

Optional takes the same size as the details itself (plus one bool). This is fairly small for now, but I think a major point of Details is to expand it in the future?

Could you elaborate on index-specific info? It's unclear to me how this would be used.

Yeah, this is something we talked about in the meeting with Marc-Andre but it's not really obvious - what's the point of allowing Details to be extended if clangd has to consume it?

It sounded like he might have use cases for using index infrastructure outside clangd. We might also have google-internal index features we want (matching generated code to proto fields?). I'm not really sure how compelling this argument is.

clangd/index/SymbolCollector.h

25

can you add a comment to the class indicating that it needs to be used for one TU and then thrown away? This seems unfortunate but is probably simpler than the alternative. It also seems to be a new restriction with this patch.

[ioeric]

• Merge with origin/master. Use Arena for symbol details.

[ioeric]

clangd/index/Index.h

156

Thanks for the allocator change!

Details now contains just stringrefs. I wonder how much we want it to be inherit-friendly at this point, as the size is relatively small now. If you think this is a better way to go, I'll make the structure contain strings and store the whole structure in arena. This would require some tweaks for yamls tho but shouldn't be hard.

clangd/index/SymbolCollector.h

25

How about re-initializing the allocator for each new AST in intialize?

[sammccall]

Just some nits

clangd/index/Index.h

156

So this needs to be at least optional I think - at the moment, how does an API user know whether to fetch the rest of the details?

FWIW, the size difference is already significant: symbol is 136 bytes if this is a unique_ptr, and 164 bytes if it's optional (+20%) - StringRefs are still pretty big.
But I don't feel really strongly about this.

clangd/index/SymbolCollector.cpp

71

you shouldn't initialize these both here and in initialize(), right?

146

why conditional? note we're interning the strings anyway, and an empty stringref is the same size as a full one

[ioeric]

• [clangd] Address review comments; made Detail a pointer.

[ioeric]

• Minor cleanup.

[ioeric]

• Merge branch 'master' of http://llvm.org/git/clang-tools-extra into symbol

clangd/index/Index.h

156

Done. Made it a raw pointer. PTAL

[sammccall]

clangd/index/SymbolYAML.cpp

78

this removes the Detail object if it's empty - this seems maybe unneccesary and certainly the wrong layer. It seems enough to do

if (!outputting)
  Detail = (allocate)
else if (!Detail)
  return;

io.mapOptional("Documentation", Detail->Documentation);
// etc

[ioeric]

• Addrress review comment.

clangd/ClangdUnit.h

public:
   ParsedAST &operator=(ParsedAST &&Other);
 
   ~ParsedAST();
 
   ASTContext &getASTContext();
   const ASTContext &getASTContext() const;
 
   Preprocessor &getPreprocessor();
+  std::shared_ptr<Preprocessor> getPreprocessorPtr();
   const Preprocessor &getPreprocessor() const;
 
   /// This function returns all top-level decls, including those that come
   /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
   /// this call might be expensive.
   ArrayRef<const Decl *> getTopLevelDecls();
 
   const std::vector<DiagWithFixIts> &getDiagnostics() const;

clangd/ClangdUnit.cpp

 ASTContext &ParsedAST::getASTContext() { return Clang->getASTContext(); }
 
 const ASTContext &ParsedAST::getASTContext() const {
   return Clang->getASTContext();
 }
 
 Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); }
 
+std::shared_ptr<Preprocessor> ParsedAST::getPreprocessorPtr() {
+  return Clang->getPreprocessorPtr();
+}
+
 const Preprocessor &ParsedAST::getPreprocessor() const {
   return Clang->getPreprocessor();
 }
 
 ArrayRef<const Decl *> ParsedAST::getTopLevelDecls() {
   ensurePreambleDeclsDeserialized();
   return TopLevelDecls;
 }

clangd/CodeComplete.cpp

 }
 
 CompletionItem indexCompletionItem(const Symbol &Sym, llvm::StringRef Filter,
                                    const SpecifiedScope &SSInfo) {
   CompletionItem Item;
   Item.kind = toCompletionItemKind(Sym.SymInfo.Kind);
   Item.label = Sym.Name;
   // FIXME(ioeric): support inserting/replacing scope qualifiers.
-  Item.insertText = Sym.Name;
+
   // FIXME(ioeric): support snippets.
+  Item.insertText = Sym.CompletionPlainInsertText;
   Item.insertTextFormat = InsertTextFormat::PlainText;
   Item.filterText = Sym.Name;
 
   // FIXME(ioeric): sort symbols appropriately.
   Item.sortText = "";
 
-  // FIXME(ioeric): use more symbol information (e.g. documentation, label) to
-  // populate the completion item.
+  if (Sym.Detail) {
+    Item.documentation = Sym.Detail->Documentation;
+    Item.detail = Sym.Detail->CompletionDetail;
+  }
 
   return Item;
 }
 
 void completeWithIndex(const Context &Ctx, const SymbolIndex &Index,
                        llvm::StringRef Code, const SpecifiedScope &SSInfo,
                        llvm::StringRef Filter, CompletionList *Items) {
   FuzzyFindRequest Req;

clangd/index/FileIndex.cpp

 #include "clang/Index/IndexingAction.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 /// Retrieves namespace and class level symbols in \p Decls.
 std::unique_ptr<SymbolSlab> indexAST(ASTContext &Ctx,
+                                     std::shared_ptr<Preprocessor> PP,
                                      llvm::ArrayRef<const Decl *> Decls) {
   auto Collector = std::make_shared<SymbolCollector>();
+  Collector->setPreprocessor(std::move(PP));
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = false;
 
   index::indexTopLevelDecls(Ctx, Decls, Collector, IndexOpts);
   auto Symbols = llvm::make_unique<SymbolSlab>();
   *Symbols = Collector->takeSymbols();
std::shared_ptr<std::vector<const Symbol *>> FileSymbols::allSymbols() {
   // pointers.
   return {std::move(Snap), Pointers};
 }
 
 void FileIndex::update(const Context &Ctx, PathRef Path, ParsedAST *AST) {
   if (!AST) {
     FSymbols.update(Path, nullptr);
   } else {
-    auto Slab = indexAST(AST->getASTContext(), AST->getTopLevelDecls());
+    auto Slab = indexAST(AST->getASTContext(), AST->getPreprocessorPtr(),
+                         AST->getTopLevelDecls());
     FSymbols.update(Path, std::move(Slab));
   }
   auto Symbols = FSymbols.allSymbols();
   Index.build(std::move(Symbols));
 }
 
 bool FileIndex::fuzzyFind(
     const Context &Ctx, const FuzzyFindRequest &Req,
     llvm::function_ref<void(const Symbol &)> Callback) const {
   return Index.fuzzyFind(Ctx, Req, Callback);
 }
 
 } // namespace clangd
 } // namespace clang

clangd/index/Index.h

 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
 
 #include "../Context.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
 #include <array>
 #include <string>
 
 namespace clang {
 namespace clangd {
 
 struct SymbolLocation {
struct Symbol {
   //
   // A C++ symbol could have multiple declarations and one definition (e.g.
   // a function is declared in ".h" file, and is defined in ".cc" file).
   //   * For classes, the canonical declaration is usually definition.
   //   * For non-inline functions, the canonical declaration is a declaration
   //     (not a definition), which is usually declared in ".h" file.
   SymbolLocation CanonicalDeclaration;
 
+  /// A brief description of the symbol that can be displayed in the completion
+  /// candidate list. For example, "Foo(X x, Y y) const" is a labal for a
+  /// function.
+  llvm::StringRef CompletionLabel;
+  /// The piece of text that the user is expected to type to match the
+  /// code-completion string, typically a keyword or the name of a declarator or
+  /// macro.
+  llvm::StringRef CompletionFilterText;
+  /// What to insert when completing this symbol (plain text version).
+  llvm::StringRef CompletionPlainInsertText;
+  /// What to insert when completing this symbol (snippet version). This is
+  /// empty if it is the same as the plain insert text above.
+  llvm::StringRef CompletionSnippetInsertText;
+
+  /// Optional symbol details that are not required to be set. For example, an
+  /// index fuzzy match can return a large number of symbol candidates, and it
+  /// is preferable to send only core symbol information in the batched results
+  /// and have clients resolve full symbol information for a specific candidate
+  /// if needed.
+  struct Details {
+    // Documentation including comment for the symbol declaration.
+    llvm::StringRef Documentation;
+    // This is what goes into the LSP detail field in a completion item. For
+    // example, the result type of a function.
+    llvm::StringRef CompletionDetail;
+  };
+
+  // Optional details of the symbol.
+  Details *Detail = nullptr;
+
   // FIXME: add definition location of the symbol.
   // FIXME: add all occurrences support.
   // FIXME: add extra fields for index scoring signals.
-  // FIXME: add code completion information.
 };
 
 // An immutable symbol container that stores a set of symbols.
 // The container will maintain the lifetime of the symbols.
 class SymbolSlab {
 public:
   using const_iterator = std::vector<Symbol>::const_iterator;
 

clangd/index/Index.cpp

auto Intern = [&](StringRef &V) {
     }
     V = *R.first;
   };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
   Intern(S.Scope);
   Intern(S.CanonicalDeclaration.FilePath);
+
+  Intern(S.CompletionLabel);
+  Intern(S.CompletionFilterText);
+  Intern(S.CompletionPlainInsertText);
+  Intern(S.CompletionSnippetInsertText);
+
+  if (S.Detail) {
+    // Copy values of StringRefs into arena.
+    auto *Detail = Arena.Allocate<Symbol::Details>();
+    Detail->Documentation = S.Detail->Documentation;
+    Detail->CompletionDetail = S.Detail->CompletionDetail;
+    S.Detail = Detail;
+
+    // Intern the actual strings.
+    Intern(S.Detail->Documentation);
+    Intern(S.Detail->CompletionDetail);
+  }
 }
 
 void SymbolSlab::Builder::insert(const Symbol &S) {
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
     Symbols.push_back(S);
     own(Symbols.back(), Strings, Arena);
   } else {

clangd/index/SymbolCollector.h

 //===--- SymbolCollector.h ---------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Index.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
 
 namespace clang {
 namespace clangd {
 
 // Collect all symbols from an AST.
 //
 // Clients (e.g. clangd) can use SymbolCollector together with
 // index::indexTopLevelDecls to retrieve all symbols when the source file is
 // changed.
 class SymbolCollector : public index::IndexDataConsumer {
 public:
-  SymbolCollector() = default;
+  void initialize(ASTContext &Ctx) override;
+
+  void setPreprocessor(std::shared_ptr<Preprocessor> PP) override {
+    this->PP = std::move(PP);
+  }
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                       ArrayRef<index::SymbolRelation> Relations, FileID FID,
                       unsigned Offset,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
 
 private:
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
+  ASTContext *ASTCtx;
+  std::shared_ptr<Preprocessor> PP;
+  std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
+  std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
 };
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolCollector.cpp

 //===--- SymbolCollector.cpp -------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolCollector.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/AST/Decl.h"
+#include "../CodeCompletionStrings.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
if (Dir) {
     SmallString<128> AbsoluteFilename;
     llvm::sys::path::append(AbsoluteFilename, DirName,
                             llvm::sys::path::filename(AbsolutePath.str()));
     return AbsoluteFilename.str();
   }
   return AbsolutePath.str();
 }
 
-// Split a qualified symbol name into scope and unqualified name, e.g. given
 // "a::b::c", return {"a::b", "c"}. Scope is empty if it doesn't exist.
 std::pair<llvm::StringRef, llvm::StringRef>
 splitQualifiedName(llvm::StringRef QName) {
   assert(!QName.startswith("::") && "Qualified names should not start with ::");
   size_t Pos = QName.rfind("::");
   if (Pos == llvm::StringRef::npos)
     return {StringRef(), QName};
   return {QName.substr(0, Pos), QName.substr(Pos + 2)};
 }
 
 } // namespace
 
+void SymbolCollector::initialize(ASTContext &Ctx) {
+  ASTCtx = &Ctx;
+  CompletionAllocator = std::make_shared<GlobalCodeCompletionAllocator>();
+  CompletionTUInfo =
+      llvm::make_unique<CodeCompletionTUInfo>(CompletionAllocator);
+}
+
 // Always return true to continue indexing.
 bool SymbolCollector::handleDeclOccurence(
     const Decl *D, index::SymbolRoleSet Roles,
     ArrayRef<index::SymbolRelation> Relations, FileID FID, unsigned Offset,
     index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   // FIXME: collect all symbol references.
   if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
         Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
     return true;
 
+  assert(CompletionAllocator && CompletionTUInfo);
+
   if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
     // FIXME: figure out a way to handle internal linkage symbols (e.g. static
     // variables, function) defined in the .cc files. Also we skip the symbols
     // in anonymous namespace as the qualifier names of these symbols are like
     // `foo::<anonymous>::bar`, which need a special handling.
     // In real world projects, we have a relatively large set of header files
     // that define static variables (like "static const int A = 1;"), we still
     // want to collect these symbols, although they cause potential ODR
if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
     auto ScopeAndName = splitQualifiedName(QName);
 
     Symbol S;
     S.ID = std::move(ID);
     S.Scope = ScopeAndName.first;
     S.Name = ScopeAndName.second;
     S.SymInfo = index::getSymbolInfo(D);
     S.CanonicalDeclaration = Location;
+
+    // Add completion info.
+    assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
+    CodeCompletionResult SymbolCompletion(ND, 0);
+    const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
+        *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
+        *CompletionTUInfo,
+        /*IncludeBriefComments*/ true);
+    std::string Label;
+    std::string SnippetInsertText;
+    std::string IgnoredLabel;
+    std::string PlainInsertText;
+    getLabelAndInsertText(*CCS, &Label, &SnippetInsertText,
+                          /*EnableSnippets=*/true);
+    getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText,
+                          /*EnableSnippets=*/false);
+    std::string FilterText = getFilterText(*CCS);
+    std::string Documentation = getDocumentation(*CCS);
+    std::string CompletionDetail = getDetail(*CCS);
+
+    S.CompletionFilterText = FilterText;
+    S.CompletionLabel = Label;
+    S.CompletionPlainInsertText = PlainInsertText;
+    S.CompletionSnippetInsertText = SnippetInsertText;
+    Symbol::Details Detail;
+    Detail.Documentation = Documentation;
+    Detail.CompletionDetail = CompletionDetail;
+    S.Detail = &Detail;
+
     Symbols.insert(S);
   }
 
   return true;
 }
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolYAML.cpp

 //===--- SymbolYAML.cpp ------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolYAML.h"
 #include "Index.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 
 LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(clang::clangd::Symbol)
 
 namespace llvm {

 template <> struct MappingTraits<SymbolInfo> {
   static void mapping(IO &io, SymbolInfo &SymInfo) {
     // FIXME: expose other fields?
     io.mapRequired("Kind", SymInfo.Kind);
     io.mapRequired("Lang", SymInfo.Lang);
   }
 };
 
-template<> struct MappingTraits<Symbol> {
+template <>
+struct MappingTraits<Symbol::Details *> {
+  static void mapping(IO &io, Symbol::Details *&Detail) {
+    if (!io.outputting()) {
+      assert(io.getContext() && "Expecting an arena (as context) to allocate "
+                                "data for new symbols.");
+      Detail = static_cast<llvm::BumpPtrAllocator *>(io.getContext())
+                   ->Allocate<Symbol::Details>();
+    } else if (!Detail) {
+      // Detail is optional in outputting.
+      return;
+    }
+    assert(Detail);
+    io.mapOptional("Documentation", Detail->Documentation);
+    io.mapOptional("CompletionDetail", Detail->CompletionDetail);
+  }
+};
+
+template <> struct MappingTraits<Symbol> {
   static void mapping(IO &IO, Symbol &Sym) {
-    MappingNormalization<NormalizedSymbolID, SymbolID> NSymbolID(
-        IO, Sym.ID);
+    MappingNormalization<NormalizedSymbolID, SymbolID> NSymbolID(IO, Sym.ID);
     IO.mapRequired("ID", NSymbolID->HexString);
     IO.mapRequired("Name", Sym.Name);
     IO.mapRequired("Scope", Sym.Scope);
     IO.mapRequired("SymInfo", Sym.SymInfo);
     IO.mapRequired("CanonicalDeclaration", Sym.CanonicalDeclaration);
+    IO.mapRequired("CompletionLabel", Sym.CompletionLabel);
+    IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText);
+    IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText);
+
+    IO.mapOptional("CompletionSnippetInsertText",
+                   Sym.CompletionSnippetInsertText);
+    IO.mapOptional("Detail", Sym.Detail);
   }
 };
 
 template <> struct ScalarEnumerationTraits<SymbolLanguage> {
   static void enumeration(IO &IO, SymbolLanguage &Value) {
     IO.enumCase(Value, "C", SymbolLanguage::C);
     IO.enumCase(Value, "Cpp", SymbolLanguage::CXX);
     IO.enumCase(Value, "ObjC", SymbolLanguage::ObjC);

 
 } // namespace yaml
 } // namespace llvm
 
 namespace clang {
 namespace clangd {
 
 SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent) {
+  // Store data of pointer fields (excl. `StringRef`) like `Detail`.
+  llvm::BumpPtrAllocator Arena;
+  llvm::yaml::Input Yin(YAMLContent, &Arena);
   std::vector<Symbol> S;
-  llvm::yaml::Input Yin(YAMLContent);
   Yin >> S;
+
   SymbolSlab::Builder Syms;
-  for (auto& Sym : S)
+  for (auto &Sym : S)
     Syms.insert(Sym);
   return std::move(Syms).build();
 }
 
 std::string SymbolsToYAML(const SymbolSlab& Symbols) {
   std::string Str;
   llvm::raw_string_ostream OS(Str);
   llvm::yaml::Output Yout(OS);
   for (Symbol S : Symbols) // copy: Yout<< requires mutability.
-    Yout<< S;
+    Yout << S;
   return OS.str();
 }
 
 std::string SymbolToYAML(Symbol Sym) {
   std::string Str;
   llvm::raw_string_ostream OS(Str);
   llvm::yaml::Output Yout(OS);
   Yout << Sym;
   return OS.str();
 }
 
 } // namespace clangd
 } // namespace clang

unittests/clangd/CodeCompleteTests.cpp

void onDiagnosticsReady(
       PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {}
 };
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.insertText == Name; }
 MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
 MATCHER_P(Kind, K, "") { return arg.kind == K; }
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
 MATCHER_P(PlainText, Text, "") {
   return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
          arg.insertText == Text;
 }
 MATCHER_P(Snippet, Text, "") {
   return arg.insertTextFormat == clangd::InsertTextFormat::Snippet &&
          arg.insertText == Text;
 }
for (const auto &Pair : Symbols) {
     size_t Pos = QName.rfind("::");
     if (Pos == llvm::StringRef::npos) {
       Sym.Name = QName;
       Sym.Scope = "";
     } else {
       Sym.Name = QName.substr(Pos + 2);
       Sym.Scope = QName.substr(0, Pos);
     }
+    Sym.CompletionPlainInsertText = Sym.Name;
     Sym.SymInfo.Kind = Pair.second;
     Slab.insert(Sym);
   }
   Snap->Slab = std::move(Slab).build();
   for (auto &Iter : Snap->Slab)
     Snap->Pointers.push_back(&Iter);
   auto S = std::shared_ptr<std::vector<const Symbol *>>(std::move(Snap),
                                                         &Snap->Pointers);
TEST(CompletionTest, ASTIndexMultiFile) {
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
                       /*StorePreamblesInMemory=*/true,
                       /*BuildDynamicSymbolIndex=*/true);
 
   Server
       .addDocument(Context::empty(), getVirtualTestFilePath("foo.cpp"), R"cpp(
-      namespace ns { class XYZ {}; void foo() {} }
+      namespace ns { class XYZ {}; void foo(int x) {} }
   )cpp")
       .wait();
 
   auto File = getVirtualTestFilePath("bar.cpp");
   Annotations Test(R"cpp(
-      namespace ns { class XXX {}; void fooooo() {} }
+      namespace ns {
+      class XXX {};
+      /// Doooc
+      void fooooo() {}
+      }
       void f() { ns::^ }
   )cpp");
   Server.addDocument(Context::empty(), File, Test.code()).wait();
 
   auto Results = Server.codeComplete(Context::empty(), File, Test.point(), {})
                      .get()
                      .second.Value;
   // "XYZ" and "foo" are not included in the file being completed but are still
   // visible through the index.
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
   EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function));
   EXPECT_THAT(Results.items, Has("XXX", CompletionItemKind::Class));
-  EXPECT_THAT(Results.items, Has("fooooo", CompletionItemKind::Function));
+  EXPECT_THAT(Results.items, Contains(AllOf(Named("fooooo"), Filter("fooooo"),
+                                            Kind(CompletionItemKind::Function),
+                                            Doc("Doooc"), Detail("void"))));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/SymbolCollectorTests.cpp

 //===-- SymbolCollectorTests.cpp  -------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include <memory>
 #include <string>
 
+using testing::AllOf;
 using testing::Eq;
 using testing::Field;
+using testing::Not;
 using testing::UnorderedElementsAre;
 using testing::UnorderedElementsAreArray;
 
 // GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+  return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+  return arg.CompletionSnippetInsertText == S;
+}
 MATCHER_P(QName, Name, "") {
   return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
 }
 
 namespace clang {
 namespace clangd {
 
 namespace {
EXPECT_THAT(Symbols,
                    QName("foo::bar"),
                    QName("foo::int32"),
                    QName("foo::int32_t"),
                    QName("foo::v1"),
                    QName("foo::bar::v2"),
                    QName("foo::baz")}));
 }
 
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+  const std::string Main = R"(
+    namespace nx {
+    /// Foo comment.
+    int ff(int x, double y) { return 0; }
+    }
+  )";
+  runSymbolCollector(/*Header=*/"", Main);
+  EXPECT_THAT(Symbols,
+              UnorderedElementsAre(QName("nx"),
+                                   AllOf(QName("nx::ff"),
+                                         Labeled("ff(int x, double y)"),
+                                         Detail("int"), Doc("Foo comment."))));
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+  const std::string Main = R"(
+    namespace nx {
+    void f() {}
+    int ff(int x, double y) { return 0; }
+    }
+  )";
+  runSymbolCollector(/*Header=*/"", Main);
+  EXPECT_THAT(
+      Symbols,
+      UnorderedElementsAre(
+          QName("nx"),
+          AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+          AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+                Snippet("ff(${1:int x}, ${2:double y})"))));
+}
+
 TEST_F(SymbolCollectorTest, YAMLConversions) {
   const std::string YAML1 = R"(
 ---
 ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
 Name:   'Foo1'
 Scope:   'clang'
 SymInfo:
   Kind:            Function
   Lang:            Cpp
 CanonicalDeclaration:
   StartOffset:     0
   EndOffset:       1
   FilePath:        /path/foo.h
+CompletionLabel:    'Foo1-label'
+CompletionFilterText:    'filter'
+CompletionPlainInsertText:    'plain'
+Detail:
+  Documentation:    'Foo doc'
+  CompletionDetail:    'int'
 ...
 )";
   const std::string YAML2 = R"(
 ---
 ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
 Name:   'Foo2'
 Scope:   'clang'
 SymInfo:
   Kind:            Function
   Lang:            Cpp
 CanonicalDeclaration:
   StartOffset:     10
   EndOffset:       12
   FilePath:        /path/foo.h
+CompletionLabel:    'Foo2-label'
+CompletionFilterText:    'filter'
+CompletionPlainInsertText:    'plain'
+CompletionSnippetInsertText:    'snippet'
 ...
 )";
 
   auto Symbols1 = SymbolFromYAML(YAML1);
-  EXPECT_THAT(Symbols1,
-              UnorderedElementsAre(QName("clang::Foo1")));
+  EXPECT_THAT(Symbols1, UnorderedElementsAre(
+                            AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+                                  Doc("Foo doc"), Detail("int"))));
   auto Symbols2 = SymbolFromYAML(YAML2);
-  EXPECT_THAT(Symbols2,
-              UnorderedElementsAre(QName("clang::Foo2")));
+  EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+                                                   Labeled("Foo2-label"),
+                                                   Not(HasDetail()))));
 
   std::string ConcatenatedYAML =
       SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
   auto ConcatenatedSymbols = SymbolFromYAML(ConcatenatedYAML);
   EXPECT_THAT(ConcatenatedSymbols,
               UnorderedElementsAre(QName("clang::Foo1"),
                                    QName("clang::Foo2")));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang