[clangd] Add support for hierarchical documentSymbol

[ilya-biryukov]

Posted to make sure it's visible that I've started doing this.
Still need to update the tests and check for the capability from the client (and fallback to SymbolInformation if client does not support the new implementation).

Nevertheless, it works and looks really good in VSCode.

Will update the thread when the code is ready for review.

[simark]

Ohh awesome, I didn't know the LSP supported that. I'll try it it Theia when I have time.

[simark]

I just tried this, this looks very promising! It should help build our outline view in a much more robust way than we do currently.

A nit for the final patch, I would suggest omitting the fields that are optional, like children (when the list is empty) and deprecated.

In vscode, is there a way to get a tree representation of this data? When I look at "Go to symbol in File..." (ctrl-shift-o) or the outline view at the bottom of the file explorer, they are both a flat list. What difference does this patch make in how vscode shows the data?

[ilya-biryukov]

In D52311#1255876, @simark wrote:

I just tried this, this looks very promising! It should help build our outline view in a much more robust way than we do currently.
A nit for the final patch, I would suggest omitting the fields that are optional, like children (when the list is empty) and deprecated.

SG, will do.

In vscode, is there a way to get a tree representation of this data? When I look at "Go to symbol in File..." (ctrl-shift-o) or the outline view at the bottom of the file explorer, they are both a flat list. What difference does this patch make in how vscode shows the data?

There's an outline view that shows the tree after this patch.
IIRC, the "Go to symbol in File..." also shows the tree view now without the lack of functionality like filtering by name, etc.
Overall, the experience with a tree seemed strictly better in all scenarios for me.

[ilya-biryukov]

• Improve traversal of the AST.
• Update the tests.
• Add a fallback to SymbolInformation (flat structure) if the client does not support the hierarhical reponse.

Still some work left to do:

• Do not drop the qualifiers for out-of-line declarations, i.e. 'X::foo' is presented as 'foo' now, which is a bit confusing.
• See why some names pop up at the global scope (GTest generates macros like that).

[ilya-biryukov]

Another annoyance to fix:

• 'using namespace any_ns' is shown as '<using-directive>'

[sammccall]

Very nice! Mostly just a few style/structure nits.

clangd/AST.cpp

69 ↗

(On Diff #171067)

just use ND?

clangd/ClangdLSPServer.cpp

28

move this nearer the call site?

29

or flattenSymbolHierarchy?

31

auto-typed lambdas can't be recursive, but std::function shouldn't be too slow here? And clearer I think:

vector<SymbolInformation> Results;
std::function<void(const DocumentSymbol&, StringRef)> Process = [&](const DocumentSymbol &Sym, StringRef ParentName) {
  ...
};
for (const auto& TopLevel : Symbols)
  Process(TopLevel);
return Results;

failing that, I found this pattern confusing - I'd suggest either making it a recursive plain function, or a standard functor object as if it were a lambda (with the recursive call being operator())

44

nit: optional<StringRef> for parent name seems slightly neater

clangd/FindSymbols.cpp

203

may want to add a FIXME here: per the tests, it's not classifying constructor/destructor/operator correctly (they're all "methods").

cons/dest is just indexSymbolKindToSymbolKind needing an update, but operator isn't represented in index::SymbolKind.
Maybe we should stop using index::SymbolKind entirely, it doesn't appear to be great.

216

this class seems maybe too long to live inside this function. (I wouldn't worry about it being visible to other stuff, the file is small and focused)

216

(this looks more like a class than a struct?)

216

should this be a RecursiveASTVisitor?
I guess not, but please explain why (there's no documentation on the implementation strategy, and it's complicated)

219

again, doing the work in the constructor is a bit odd - own the data and expose it?

247

we only consider visiting named decls, maybe reflect that here?
(not needed now but may allow future refinement)

255

isn't this covered by D->isImplicit?

267

isn't this covered by D->isImplicit() above?

clangd/Protocol.h

43

why?

137

just an overload of contains()?

unittests/clangd/FindSymbolsTests.cpp

48 ↗

(On Diff #171067)

ChildrenAre() (with no args) and NoChildren() are the same matcher I think.

I'd suggest flattening Children() into ChildrenAre() and renaming it to Children() - less things for the reader to understand.

442 ↗

(On Diff #171067)

this one is to be fixed, right?

521 ↗

(On Diff #171067)

hmm, this seems pretty confusing - I think Tmpl<float> would be a clearer name for a specialization, even if we just have Tmpl for the primary template.
Partial specializations are confusing, though :-/

523 ↗

(On Diff #171067)

why the change in policy with this patch? (one of these previously was deliberately not indexed, now is)

524 ↗

(On Diff #171067)

these assertions are confusing as it's not clear which expected belongs with which piece of actual code. Obviously they all need to have the same name though.
Best workaround I can think of is putting "int a", "int b" etc between the confusing decls, and including them in the expected top-level symbols (since you assert in order)

571 ↗

(On Diff #171067)

hmm, our handling of anonymous enums seems different than anonymous namespaces - why?

[ilya-biryukov]

• Address comments

clangd/FindSymbols.cpp

203

Added a FIXME.
From what I can tell, writing our own classification function shouldn't be too much work and would clearly fix those cases. We should do this!

216

Added a comment.

255

Nope, IIUC things are only marked "implicit" in clangd if they were generated by the compiler from the start.
For this case, the initial template code was written by the user, so isImplicit() returns false for both the template and its instantiations.

267

See the other comment.

clangd/Protocol.h

43

Sorry, accidental change. Reverted.

unittests/clangd/FindSymbolsTests.cpp

442 ↗

(On Diff #171067)

Why? The outline view gives both the declaration and the definition, since both were written in the code.
That seems to be in line with what I'd expect from the outline view.

521 ↗

(On Diff #171067)

Done. Now prints as Tmpl<float>.
This may also include some arguments not written by the users (e.g. some default args), but added a FIXME to fix this, it's not entirely trivial

523 ↗

(On Diff #171067)

We might have different goals in mind here.
From my perspective, the purpose of the outline is to cover all things written in the code: there are 4 decls that the user wrote: one primary template, one template specialization and two template instantiations.

What is your model here?

571 ↗

(On Diff #171067)

No rigorous reasons. The namespaces tend to group more things together, so it's arguably more useful to have an option of folding their subitems in the tree.
Anonymous enums are typically used only in the classes (and on the top-level in C to define constants?) and I feel having an extra node there merely produces noise.
Happy to reconsider, what's your preferred behavior?

[ilya-biryukov]

• Remove accidentally added qualifiers

[sammccall]

unittests/clangd/FindSymbolsTests.cpp

442 ↗

(On Diff #171067)

Sorry, I meant the name should be Foo::def instead of def, which you mentioned in a previous comment.

OK to land without this, but I think it deserves a fixme.
(If you think this is the *right* behavior, let's discuss further!)

521 ↗

(On Diff #171067)

Nice! I thought this was going to be really hard.

(You can specialize without naming the defaulted args? That sounds... obscure. Definitely fine to leave for later)

523 ↗

(On Diff #171067)

I don't have a particular opinion, just wondering if this was a deliberate change or fell out of the implementation.

(The old behavior was tested, so I guess Marc-Andre had a reason, but it didn't come up in the review. I don't think this is common enough to worry much about either way)

571 ↗

(On Diff #171067)

I *think* I might prefer the opposite - namespaces flat and enums grouped :-)
I'm not certain nor adamant about either, so consider my thoughts below, and pick something.

Namespaces: anonymous namespaces are mostly (entirely?) used to control visibility. I don't think they *group* in a meaningful way, they just hide things like static does. In a perfect world I think there'd be a "private" bit on outline elements that was shown in the UI, and we'd put it on both things-in-anon-namespaces and static-as-in-private decls. The main counterargument I can see: because we typically don't indent inside namespaces, the enclosing anonymous namespace can be hard to recognize and jump to.

Enums: conversely, I think these _typically_ do group their elements, and there's often a strong semantic boundary between the last entry in the enum and the first decl after it. When people use a namespacing prefix (C-style VK_Foo, VK_Bar) then it's visually noticeable, but this is far from universal in C++ (particularly at class scope).

Consistency: It's not *really* confusing if these are different, just *slightly* confusing. Anonymous structs etc must certainly be grouped, so this is a (weak) argument for grouping everything.

[ilya-biryukov]

unittests/clangd/FindSymbolsTests.cpp

442 ↗

(On Diff #171067)

Oh, yeah, that I agree with! Will look into fixing this in a follow-up change.

521 ↗

(On Diff #171067)

Yeah, you can. It's especially common with function where specializations don't name any arguments and those are inferred from the function type...

523 ↗

(On Diff #171067)

Yeah, this fell out of the implementation of walking over the "user-written" part of the AST and it looked sensible so I updated the test.

571 ↗

(On Diff #171067)

I'm sold on the consistency argument. Besides, the anonymous enums are not too frequent anyway, so treating them in a special way might be an overkill.
We can always tweak this in the future.

[ilya-biryukov]

• Do not drop anonymous enums

[ilya-biryukov]

• Handle 'using namespace' in printName
• Do not mix-in symbols with locations from other files.

clangd/ClangdLSPServer.cpp

 using namespace clang::clangd;
 using namespace clang;
 using namespace llvm;
 
 namespace {
 
 /// \brief Supports a test URI scheme with relaxed constraints for lit tests.
 /// The path in a test URI will be combined with a platform-specific fake
 /// directory to form an absolute path. For example, test:///a.cpp is resolved
 /// C:\clangd-test\a.cpp on Windows and /clangd-test/a.cpp on Unix.
 class TestScheme : public URIScheme {
 public:
   llvm::Expected<std::string>
   getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
                   llvm::StringRef /*HintPath*/) const override {
     using namespace llvm::sys;
     // Still require "/" in body to mimic file scheme, as we want lengths of an
     // equivalent URI in both schemes to be the same.
     if (!Body.startswith("/"))
       return llvm::make_error<llvm::StringError>(
           "Expect URI body to be an absolute path starting with '/': " + Body,
           llvm::inconvertibleErrorCode());
     Body = Body.ltrim('/');
 #ifdef _WIN32
     constexpr char TestDir[] = "C:\\clangd-test";
 #else
     constexpr char TestDir[] = "/clangd-test";
 #endif
     llvm::SmallVector<char, 16> Path(Body.begin(), Body.end());
     path::native(Path);
     auto Err = fs::make_absolute(TestDir, Path);
     if (Err)
       llvm_unreachable("Failed to make absolute path in test scheme.");
void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) {
   else
     replyError(ErrorCode::UnknownErrorCode,
                llvm::toString(ReplacementsOrError.takeError()));
 }
 
 void ClangdLSPServer::onDocumentSymbol(DocumentSymbolParams &Params) {
   Server.documentSymbols(
       Params.textDocument.uri.file(),
-      [this](llvm::Expected<std::vector<SymbolInformation>> Items) {
+      [this](llvm::Expected<std::vector<DocumentSymbol>> Items) {
         if (!Items)
           return replyError(ErrorCode::InvalidParams,
                             llvm::toString(Items.takeError()));
         for (auto &Sym : *Items)
           Sym.kind = adjustKindToCapability(Sym.kind, SupportedSymbolKinds);
         reply(json::Array(*Items));
       });
 }

clangd/ClangdServer.h

void findHover(PathRef File, Position Pos,
                  Callback<llvm::Optional<Hover>> CB);
 
   /// Retrieve the top symbols from the workspace matching a query.
   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);
+                       Callback<std::vector<DocumentSymbol>> CB);
 
   /// Retrieve locations for symbol references.
   void findReferences(PathRef File, Position Pos,
                       Callback<std::vector<Location>> CB);
 
   /// Run formatting for \p Rng inside \p File with content \p Code.
   llvm::Expected<tooling::Replacements> formatRange(StringRef Code,
                                                     PathRef File, Range Rng);

clangd/ClangdServer.cpp

 }
 
 void ClangdServer::workspaceSymbols(
     StringRef Query, int Limit, Callback<std::vector<SymbolInformation>> CB) {
   CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
                                  RootPath ? *RootPath : ""));
 }
 
-void ClangdServer::documentSymbols(
-    StringRef File, Callback<std::vector<SymbolInformation>> CB) {
-  auto Action = [](Callback<std::vector<SymbolInformation>> CB,
+void ClangdServer::documentSymbols(StringRef File,
+                                   Callback<std::vector<DocumentSymbol>> CB) {
+  auto Action = [](Callback<std::vector<DocumentSymbol>> CB,
                    llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::getDocumentSymbols(InpAST->AST));
   };
   WorkScheduler.runWithAST("documentSymbols", File,
                            Bind(Action, std::move(CB)));
 }

clangd/FindSymbols.h

 /// \p HintPath This is used when resolving URIs. If empty, URI resolution can
 /// fail if a hint path is required for the scheme of a specific URI.
 llvm::Expected<std::vector<SymbolInformation>>
 getWorkspaceSymbols(llvm::StringRef Query, int Limit,
                     const SymbolIndex *const Index, llvm::StringRef HintPath);
 
 /// Retrieves the symbols contained in the "main file" section of an AST in the
 /// same order that they appear.
-llvm::Expected<std::vector<SymbolInformation>>
-getDocumentSymbols(ParsedAST &AST);
+llvm::Expected<std::vector<DocumentSymbol>> getDocumentSymbols(ParsedAST &AST);
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/FindSymbols.cpp

 //===--- FindSymbols.cpp ------------------------------------*- C++-*------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include "FindSymbols.h"
 
 #include "AST.h"
 #include "ClangdUnit.h"
 #include "FuzzyMatch.h"
 #include "Logger.h"
+#include "Protocol.h"
 #include "Quality.h"
 #include "SourceCode.h"
 #include "index/Index.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
 #define DEBUG_TYPE "FindSymbols"
 
 namespace clang {
 namespace clangd {
 
Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) {
     Top.push({Score, std::move(Info)});
   });
   for (auto &R : std::move(Top).items())
     Result.push_back(std::move(R.second));
   return Result;
 }
 
 namespace {
-/// Finds document symbols in the main file of the AST.
-class DocumentSymbolsConsumer : public index::IndexDataConsumer {
-  ASTContext *
-  std::vector<SymbolInformation> Symbols;
-  // We are always list document for the same file, so cache the value.
-  llvm::Optional<URIForFile> MainFileUri;
-
-public:
-  DocumentSymbolsConsumer(ASTContext &AST) : AST(AST) {}
-  std::vector<SymbolInformation> takeSymbols() { return std::move(Symbols); }
-
-  void initialize(ASTContext &Ctx) override {
-    // Compute the absolute path of the main file which we will use for all
-    // results.
-    const SourceManager &SM = AST.getSourceManager();
-    const FileEntry *F = SM.getFileEntryForID(SM.getMainFileID());
-    if (!F)
-      return;
-    auto FilePath = getRealPath(F, SM);
-    if (FilePath)
-      MainFileUri = URIForFile(*FilePath);
-  }
-
-  bool shouldIncludeSymbol(const NamedDecl *ND) {
-    if (!ND || ND->isImplicit())
-      return false;
-    // Skip anonymous declarations, e.g (anonymous enum/class/struct).
-    if (ND->getDeclName().isEmpty())
-      return false;
-    return true;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *, index::SymbolRoleSet Roles,
-                      ArrayRef<index::SymbolRelation> Relations,
-                      SourceLocation Loc,
-                      index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-    assert(ASTNode.OrigD);
-    // No point in continuing the index consumer if we could not get the
-    // absolute path of the main file.
-    if (!MainFileUri)
-      return false;
-    // We only want declarations and definitions, i.e. no references.
-    if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
-          Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
-      return true;
-    SourceLocation NameLoc = findNameLoc(ASTNode.OrigD);
-    const SourceManager &SourceMgr = AST.getSourceManager();
-    // We should be only be looking at "local" decls in the main file.
-    if (!SourceMgr.isWrittenInMainFile(NameLoc)) {
-      // Even thought we are visiting only local (non-preamble) decls,
-      // we can get here when in the presence of "extern" decls.
-      return true;
-    }
-    const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(ASTNode.OrigD);
-    if (!shouldIncludeSymbol(ND))
-      return true;
-
-    SourceLocation EndLoc =
-        Lexer::getLocForEndOfToken(NameLoc, 0, SourceMgr, AST.getLangOpts());
-    Position Begin = sourceLocToPosition(SourceMgr, NameLoc);
-    Position End = sourceLocToPosition(SourceMgr, EndLoc);
-    Range R = {Begin, End};
-    Location L;
-    L.uri = *MainFileUri;
-    L.range = R;
+DocumentSymbol declToSym(ASTContext &Ctx, const NamedDecl &ND,
+                         SourceLocation NameLoc,
+                         const URIForFile &MainFileUri) {
+  auto &SM = Ctx.getSourceManager();
+      Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts());
+  Position Begin = sourceLocToPosition(SM, NameLoc);
+  Position End = sourceLocToPosition(SM, EndLoc);
 
-    std::string QName = printQualifiedName(*ND);
-    StringRef Scope, Name;
-    std::tie(Scope, Name) = splitQualifiedName(QName);
-    Scope.consume_back("::");
+  std::string QName = printQualifiedName(ND);
 
-    index::SymbolInfo SymInfo = index::getSymbolInfo(ND);
-    SymbolKind SK = indexSymbolKindToSymbolKind(SymInfo.Kind);
+  index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
 
-    SymbolInformation SI;
-    SI.name = Name;
-    SI.kind = SK;
-    SI.location = L;
-    SI.containerName = Scope;
-    Symbols.push_back(std::move(SI));
-    return true;
-  }
-};
+  DocumentSymbol SI;
+  SI.deprecated = ND.isDeprecated();
+  // FIXME(ibiryukov): set the range properly.
+  SI.range = R;
+  SI.selectionRange = R;
+  return SI;
+}
+
+std::vector<DocumentSymbol> collectDocSymbols(ASTContext &Ctx,
+                                              const DeclContext &Scope,
+                                              const URIForFile &MainFileUri) {
+  std::vector<DocumentSymbol> Symbols;
+  auto &SM = Ctx.getSourceManager();
+  for (const Decl *D : Scope.decls()) {
+    if (D->isImplicit())
+      continue;
+    if (!SM.isWrittenInMainFile(D->getLocation()))
+      continue;
+    // Add symbol to a list.
+    DocumentSymbol *S = nullptr;
+    if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
+      Symbols.push_back(declToSym(Ctx, *ND, findNameLoc(ND), MainFileUri));
+      S = &Symbols.back();
+    }
+    // Add childrens if decl is also a DeclContext.
+    // FIXME(ibiryukov): filtering only functions is probably not enough.
+    if (llvm::isa<FunctionDecl>(D))
+      continue; // don't add function locals.
+    // FIXME(ibiryukov): do we even need to handle "DeclContext, but not
+    //                   NamedDecl" case? Can this happen?
+    if (const DeclContext *ChildScope = llvm::dyn_cast<DeclContext>(D)) {
+      auto ChildSymbols = collectDocSymbols(Ctx, *ChildScope, MainFileUri);
+      if (S)
+        S->children = std::move(ChildSymbols);
+      else
+        Symbols.insert(Symbols.end(), ChildSymbols.begin(), ChildSymbols.end());
+    }
+  }
+  return Symbols;
 } // namespace
 
-llvm::Expected<std::vector<SymbolInformation>>
-getDocumentSymbols(ParsedAST &AST) {
-  DocumentSymbolsConsumer DocumentSymbolsCons(AST.getASTContext());
-
-  index::IndexingOptions IndexOpts;
-  IndexOpts.SystemSymbolFilter =
-      index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
-  IndexOpts.IndexFunctionLocals = false;
-  indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
-                     DocumentSymbolsCons, IndexOpts);
-
-  return DocumentSymbolsCons.takeSymbols();
+llvm::Expected<std::vector<DocumentSymbol>> getDocumentSymbols(ParsedAST &AST) {
+  // Compute the absolute path of the main file which we will use for all
+  // results.
+  const SourceManager &SM = AST.getASTContext().getSourceManager();
+  const FileEntry *F = SM.getFileEntryForID(SM.getMainFileID());
+  if (!F)
+    return llvm::createStringError(
+        llvm::errc::invalid_argument,
+        "Could not retrieve FileEntry for main file.");
+  auto FilePath = getRealPath(F, SM);
+  // No point in continuing the index consumer if we could not get the
+  // absolute path of the main file.
+  if (!FilePath)
+    return llvm::createStringError(
+        llvm::errc::invalid_argument,
+        "Could not retrieve absolute path of the main file.");
+  URIForFile MainFileUri = URIForFile(*FilePath);
+
+  // Collect all local symbols in the AST, along with start locations of their
+  // names.
+  ASTContext &Ctx = AST.getASTContext();
+  // Convert into the hierarchical tree of symbols.
+  return collectDocSymbols(Ctx, *Ctx.getTranslationUnitDecl(), MainFileUri);
 }
 
 } // namespace clangd
 } // namespace clang

clangd/Protocol.h

 namespace clangd {
 
 enum class ErrorCode {
   // Defined by JSON RPC.
   ParseError = -32700,
   InvalidRequest = -32600,
   MethodNotFound = -32601,
   InvalidParams = -32602,
+
   InternalError = -32603,
 
   ServerNotInitialized = -32002,
   UnknownErrorCode = -32001,
 
   // Defined by the protocol.
   RequestCancelled = -32800,
 };
struct Range {
   friend bool operator!=(const Range &LHS, const Range &RHS) {
     return !(LHS == RHS);
   }
   friend bool operator<(const Range &LHS, const Range &RHS) {
     return std::tie(LHS.start, LHS.end) < std::tie(RHS.start, RHS.end);
   }
 
   bool contains(Position Pos) const { return start <= Pos && Pos < end; }
 };
 bool fromJSON(const llvm::json::Value &, Range &);
 llvm::json::Value toJSON(const Range &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Range &);
 
 struct Location {
   /// The text document's URI.
   URIForFile uri;
   Range range;

 bool fromJSON(const llvm::json::Value &, ExecuteCommandParams &);
 
 struct Command : public ExecuteCommandParams {
   std::string title;
 };
 
 llvm::json::Value toJSON(const Command &C);
 
+/// Represents programming constructs like variables, classes, interfaces etc.
+/// that appear in a document. Document symbols can be hierarchical and they
+/// have two ranges: one that encloses its definition and one that points to its
+/// most interesting range, e.g. the range of an identifier.
+struct DocumentSymbol {
+  /// The name of this symbol.
+  std::string name;
+
+  /// More detail for this symbol, e.g the signature of a function.
+  std::string detail;
+
+  /// The kind of this symbol.
+  SymbolKind kind;
+
+  /// Indicates if this symbol is deprecated.
+  bool deprecated;
+
+  /// The range enclosing this symbol not including leading/trailing whitespace
+  /// but everything else like comments. This information is typically used to
+  /// determine if the clients cursor is inside the symbol to reveal in the
+  /// symbol in the UI.
+  Range range;
+
+  /// The range that should be selected and revealed when this symbol is being
+  /// picked, e.g the name of a function. Must be contained by the `range`.
+  Range selectionRange;
+
+  /// Children of this symbol, e.g. properties of a class.
+  std::vector<DocumentSymbol> children;
+};
+llvm::json::Value toJSON(const DocumentSymbol &S);
+
 /// Represents information about programming constructs like variables, classes,
 /// interfaces etc.
 struct SymbolInformation {
   /// The name of this symbol.
   std::string name;
 
   /// The kind of this symbol.
   SymbolKind kind;

clangd/Protocol.cpp

 
 #include "Protocol.h"
 #include "Logger.h"
 #include "URI.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
 using namespace llvm;
 
 URIForFile::URIForFile(std::string AbsPath) {

 
 json::Value toJSON(const Command &C) {
   auto Cmd = json::Object{{"title", C.title}, {"command", C.command}};
   if (C.workspaceEdit)
     Cmd["arguments"] = {*C.workspaceEdit};
   return std::move(Cmd);
 }
 
+llvm::json::Value toJSON(const DocumentSymbol &S) {
+  return json::Object{{"name", S.name},
+                      {"detail", S.detail},
+                      {"kind", static_cast<int>(S.kind)},
+                      {"deprecated", S.deprecated},
+                      {"children", json::Array{S.children}},
+                      {"range", S.range},
+                      {"selectionRange", S.selectionRange}};
+}
+
 json::Value toJSON(const WorkspaceEdit &WE) {
   if (!WE.changes)
     return json::Object{};
   json::Object FileChanges;
   for (auto &Change : *WE.changes)
     FileChanges[Change.first] = json::Array(Change.second);
   return json::Object{{"changes", std::move(FileChanges)}};
 }

clangd/clients/clangd-vscode/package.json

 {
     "name": "vscode-clangd",
     "displayName": "vscode-clangd",
     "description": "Clang Language Server",
     "version": "0.0.6",
     "publisher": "llvm-vs-code-extensions",
     "homepage": "https://clang.llvm.org/extra/clangd.html",
     "engines": {
-        "vscode": "^1.18.0"
+        "vscode": "^1.27.0"
     },
     "categories": [
         "Programming Languages",
         "Linters",
         "Snippets"
     ],
     "keywords": [
         "C",

     "main": "./out/src/extension",
     "scripts": {
         "vscode:prepublish": "tsc -p ./",
         "compile": "tsc -watch -p ./",
         "postinstall": "node ./node_modules/vscode/bin/install",
         "test": "node ./node_modules/vscode/bin/test"
     },
     "dependencies": {
-        "vscode-languageclient": "^4.0.0",
-        "vscode-languageserver": "^4.0.0"
+        "vscode-languageclient": "^5.1.0",
+        "vscode-languageserver": "^5.1.0"
     },
     "devDependencies": {
         "typescript": "^2.0.3",
         "vscode": "^1.1.0",
         "mocha": "^2.3.3",
         "@types/node": "^6.0.40",
         "@types/mocha": "^2.2.32"
     },

unittests/clangd/SyncAPI.h

 llvm::Expected<std::vector<tooling::Replacement>>
 runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
 llvm::Expected<std::vector<SymbolInformation>>
 runWorkspaceSymbols(ClangdServer &Server, StringRef Query, int Limit);
 
-llvm::Expected<std::vector<SymbolInformation>>
+llvm::Expected<std::vector<DocumentSymbol>>
 runDocumentSymbols(ClangdServer &Server, PathRef File);
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H

unittests/clangd/SyncAPI.cpp

 
 llvm::Expected<std::vector<SymbolInformation>>
 runWorkspaceSymbols(ClangdServer &Server, StringRef Query, int Limit) {
   llvm::Optional<llvm::Expected<std::vector<SymbolInformation>>> Result;
   Server.workspaceSymbols(Query, Limit, capture(Result));
   return std::move(*Result);
 }
 
-llvm::Expected<std::vector<SymbolInformation>>
+llvm::Expected<std::vector<DocumentSymbol>>
 runDocumentSymbols(ClangdServer &Server, PathRef File) {
   llvm::Optional<llvm::Expected<std::vector<SymbolInformation>>> Result;
   Server.documentSymbols(File, capture(Result));
   return std::move(*Result);
 }
 
 } // namespace clangd
 } // namespace clang