[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#1390283 <https://reviews.llvm.org/D56370#1390283>, @kadircet wrote:

> Implementation LG, but I am not sure about adding a functionality on a proposal that might change. WDYT @sammccall ?


We discussed this a bit on the thread, I think we should follow the proposed spec, but with some useful hedges:

- only implement enough bits to be useful
- I'm not sure the internal APIs/implementation should follow the "shape" of the spec so deeply.

So on a concrete but still high-level:

- I don't think we should implement the `resolve` operation, or resolution bounds, at this point - this patch doesn't do anything slow. Returning complete ancestors and never returning any children seems simplest.
- in 'XRefs.h', I think the API to introduce is `findTypeAt(Position) -> Decl*` +  `typeAncestors(Decl*)` and later `typeDescendants(Decl*)`, rather than a single more complex `typeHierarchy` call. These two operations have little in common implementation-wise, and it's easy to imagine editors preferring to expose them separately. In clangdserver of course we need to expose a single operation because of transactionality. The stitching together could go in clangdserver, or a higher-level function exposed by xrefs - but I think the separate functions are what we should be unit-testing.



================
Comment at: clang-tools-extra/unittests/clangd/Matchers.h:130
 
+// Implements the HasValue(m) matcher for matching an Optional whose
+// value matches matcher m.
----------------
nridge wrote:
> Should I split this out into a separate patch? It's used by the tests being added for this functionality.
This is nice! I think it should probably go in llvm/Testing/Support/..., but it's OK here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370