[PATCH] D92749: [clangd] go-to-implementation on a base class jumps to all subclasses.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

How does this handle Dependent base classes (including CRTP). Can tests be added to demonstrate that behaviour if its supported.



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1200
+      }
+    if (const auto *D = dyn_cast<CXXRecordDecl>(ND)) {
+      IDs.insert(getSymbolID(D));
----------------
nit: Make this `else if` or add a continue statement to the the previous if. We know ND can't be a `CXXRecordDecl` its a `CXXMethodDecl`.
super-nit: Maybe call this `CXXRD` or just `RD`, `D` implies we only know its a `Decl` but nothing more.


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1513
   auto AST = TU.build();
-  for (const std::string &Label : {"1", "2", "3", "4"}) {
+  for (const std::string &Label : {"0", "1", "2", "3", "4"}) {
     for (const auto &Point : Code.points(Label)) {
----------------
If you're changing this line, may as well change use `StringRef` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92749