[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
Thu Mar 14 16:55:56 PDT 2019
nridge marked 7 inline comments as done.
nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.h:62
+/// Find the record type references at \p Pos.
+const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos);
+
----------------
ilya-biryukov wrote:
> 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?
@sammccall asked for this method to be exposed in the header and some of the tests written in terms of it.
================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.h:154
+// the SourceManager.
+std::string toURI(const SourceManager &SM, llvm::StringRef Path,
+ llvm::StringRef FallbackDir);
----------------
ioeric wrote:
> why are we pulling this into the header? This still seems to be only used in SymbolCollector.cpp.
You're right. This is a leftover from an earlier version of the patch.
================
Comment at: clang-tools-extra/test/clangd/type-hierarchy.test:1
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
----------------
kadircet wrote:
> why strict-whitespace?
That's what the other lit tests seem to do, I just copied it :)
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