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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:842
+
+  StringRef Filename = SM.getFilename(BeginLoc);
+  std::string FileURI = toURI(SM, Filename, {});
----------------
sammccall wrote:
> This is more conversions than necessary.
> I think we just need:
> 
> ```auto FilePath = getCanonicalPath(SM.getFileEntryForID(SM.getFileID(BeginLoc)), SM);
> auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()));
> if (!FilePath || !TUPath)
>   return;
> THI.uri = URIForFile::Canonicalize(*FilePath, *TUPath);
> ```
> 
> (This is still more lines than I'd like, maybe we should have some helper for going from (File ID, SourceManager) --> URI)
(I looked at adding such a helper, and I don't think it's a good idea - the few existing callsites that could use it currently benefit significantly from hoisting the getCanonicalPath from the TUPath outside of a loop. It may or may not be worth doing that here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370