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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 3 inline comments as done.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/Protocol.cpp:1228
+    Result["parents"] = RP.parents;
+  return std::move(Result);
+}
----------------
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.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1954-1964
       [&AST](const SelectionTree::Node *N) -> std::vector<LocatedSymbol> {
     std::vector<LocatedSymbol> LocatedSymbols;
 
     // NOTE: unwrapFindType might return duplicates for something like
-    // unique_ptr<unique_ptr<T>>. Let's *not* remove them, because it gives you some
-    // information about the type you may have not known before
-    // (since unique_ptr<unique_ptr<T>> != unique_ptr<T>).
-    for (const QualType& Type : unwrapFindType(typeForNode(N), AST.getHeuristicResolver()))
-        llvm::copy(locateSymbolForType(AST, Type), std::back_inserter(LocatedSymbols));
+    // unique_ptr<unique_ptr<T>>. Let's *not* remove them, because it gives you
+    // some information about the type you may have not known before (since
+    // unique_ptr<unique_ptr<T>> != unique_ptr<T>).
----------------
usaxena95 wrote:
> Can you revert the formatting changes in l:1894-1964
> 
oops, thanks for noticing.


================
Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:780
+                  withResolveParents(HasValue(UnorderedElementsAre(
+                      withResolveID(Result.front().data.symbolID.str())))))));
+}
----------------
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`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131385