[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
Sun Mar 3 11:27:20 PST 2019
nridge marked 19 inline comments as done.
nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/Protocol.cpp:830
+ return false;
+ if (auto *Resolve = Params.getAsObject()->get("resolve")) {
+ if (!fromJSON(*Resolve, R.resolve)) {
----------------
sammccall wrote:
> use ObjectMapper here? It's more idiomatic and avoids the issues below.
The reason I didn't use ObjectMapper is that I didn't know how to use it while also reusing the `fromJSON()` method for the base class, `TextDocumentPositionParams`.
I suppose I could just not reuse it (i.e. mention the fields of `TextDocumentPositionParams` in this function directly).
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:885
+static Optional<TypeHierarchyItem>
+getTypeHierarchy(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, int Levels,
+ TypeHierarchyDirection Direction) {
----------------
sammccall wrote:
> The scope and complexity of this function seems unneccesarily large:
> - it's (in future?) capable of resolving in both directions
> - it calls itself recursively, modifying TypeHierarchyDirection to control the search
> - it handles levels, even though this optimization is only important for derived types
>
> Can we restrict this to retrieving (all) recursive base types?
> i.e. `Optional<TypeHierarchyItem> getTypeAncestors(const CXXRecordDecl &CXXRD, ASTContext &Ctx)`
> Then later, subtype resolution can be the job of another function: `resolveTypeDescendants(TypeHierarchyItem& Item, int depth)`
>
> That way the recursion of getTypeAncestors is simpler to understand, as it has much smaller scope. And code sharing between the two LSP calls is clearer: fetching type hierarchy is a call to `getTypeAncestors` and a call to `resolveTypeDescendants` (if direction is children or both, and resolve is > 0), and resolving a hierarchy is a call to `resolveTypeDescendants`.
If we remove "levels" here, should we introduce some kind of guard for infinite recursion, in case the user writes something like:
```
template <int N>
struct S : S<N + 1> {};
S<0> s;
```
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