[PATCH] D131385: [clangd] Support for standard type hierarchy

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 accepted this revision.
usaxena95 added a comment.
This revision is now accepted and ready to land.

LG. Thanks!



================
Comment at: clang-tools-extra/clangd/Protocol.cpp:1228
+    Result["parents"] = RP.parents;
+  return std::move(Result);
+}
----------------
kadircet wrote:
> usaxena95 wrote:
> > Nit: Allow RVO.
> there's an issue with one of the compilers used by a build-bot which can't find the relevant constructor if we don't have std::move here (hence we've the same pattern across others). we should definitely check at some point to see if that compiler is gone, but i'd rather not do that in the scope of this patch.
Interesting. Makes sense to get to it later.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1713
+      fillSuperTypes(*ParentDecl, TUPath, *ParentSym, RPSet);
+      Item.data.parents->emplace_back(ParentSym->data);
+      Item.parents->emplace_back(std::move(*ParentSym));
----------------
nit: std::move.


================
Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:780
+                  withResolveParents(HasValue(UnorderedElementsAre(
+                      withResolveID(Result.front().data.symbolID.str())))))));
+}
----------------
kadircet wrote:
> usaxena95 wrote:
> > It would be clearer if we use `getSymbolID(&findDecl(AST, "Parent1"))` here and in `SuperTypes`.
> > 
> > 
> not sure what you mean by also doing it in `SuperTypes`
Nvm. I was referring to `SuperTypes` test but it does not check for the symbol id.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131385