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

Nathan Ridge via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 9 16:25:40 PST 2019


nridge added a comment.

In D56370#1390359 <https://reviews.llvm.org/D56370#1390359>, @sammccall wrote:

> 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.


A hypothetical client could always ask for one level at a time, and ignore any levels it didn't ask for; such a client would break if we did this.

I think the implementation of `resolve` is straightforward enough that we might as well keep it.

> - 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.

This sounds nice and clean, thanks for the suggestion! I will give it a try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370





More information about the cfe-commits mailing list