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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 13 03:29:45 PDT 2019


ilya-biryukov added a comment.

A few small NITs



================
Comment at: clang-tools-extra/clangd/ClangdServer.h:188
+  /// Get information about type hierarchy for a given position.
+  void findTypeHierarchy(PathRef File, Position Pos, int Resolve,
+                         TypeHierarchyDirection Direction,
----------------
Could you rename it to `typeHierarchy`?
We try to keep these method names aligned with the LSP methods.


================
Comment at: clang-tools-extra/clangd/XRefs.h:62
+/// Find the record type references at \p Pos.
+const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos);
+
----------------
This method looks like an implementation detail and does not align with other methods in `XRefs.h` which are high-level methods that implement LSP functionality.

It would be more appropriate to move it to `AST.h` or directly into the `XRefs.cpp`. WDYT?


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