[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

just a few drive-by comments ;)



================
Comment at: clang-tools-extra/clangd/FindSymbols.h:28
+// https://github.com/Microsoft/language-server-protocol/issues/344
+SymbolKind indexSymbolKindToSymbolKind(index::SymbolKind Kind);
+
----------------
This could probably live in `Protocol.h`.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:957
+      getTypeAncestors(*CXXRD, AST.getASTContext());
+  // TODO: Resolve type descendants if direction is Children or Both,
+  //       and ResolveLevels > 0.
----------------
nit: s/TODO/FIXME(owner)/

same elsewhere.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.h:154
+// the SourceManager.
+std::string toURI(const SourceManager &SM, llvm::StringRef Path,
+                  llvm::StringRef FallbackDir);
----------------
why are we pulling this into the header? This still seems to be only used in SymbolCollector.cpp.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56370/new/

https://reviews.llvm.org/D56370