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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D56370#1391924 <https://reviews.llvm.org/D56370#1391924>, @nridge wrote:

> As far as reworking the tests to use these functions, I've thought about that a bit:
>
> - These functions return AST nodes. It's not clear to me how I would come up with "expected" AST nodes to test the return values against.


See FindDecl

> - Even if we find a way to get "expected" AST nodes, we would be losing test coverage of functions like `declToTypeHierarchyItem()` (though I suppose we could write separate tests for that).
> - We could instead test against the //properties// of the AST nodes, such as their names and source ranges, but then the test code to query those properties would basically be duplicating code in `declToTypeHierarchyItem()`. It would seem to make more sense to just test the resulting `TypeHierarchyItem` instead (as the tests are doing now).





================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:543
+      return CB(InpAST.takeError());
+    CB(clangd::getTypeHierarchy(InpAST->AST, Item.range.start, Resolve,
+                                Direction));
----------------
relying on the item range to resolve the actual symbol isn't reliable:
 - the source code may have changed
 - the range may not be within the TU, and might be e.g. in an indexed header we don't have a compile command for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370