[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
Fri Mar 15 02:48:06 PDT 2019


ilya-biryukov 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);
+
----------------
nridge wrote:
> 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.
LG, thanks for the clarification. I haven't been following the review closely.
Having methods that return pointers in `XRefs.h` seems like the wrong layering to me, but in any case it's a minor detail.


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