[PATCH] D111039: [clangd] Include refs of base method in refs for derived method.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 4 01:46:19 PDT 2021


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1395
           if (CMD->isVirtual())
-            if (IdentifierAtCursor && SM.getSpellingLoc(CMD->getLocation()) ==
-                                          IdentifierAtCursor->location()) {
----------------
it's important to note that this doesn't only suppress xrefs behaviour outside of the declaration, but it also suppresses xrefs on parts of the declaration apart from the function's name (like override/virtual keywords). it's unfortunate that we don't have tests to explicitly point out that fact.
I am not sure why the decision was to exclude them in the first place, I am definitely in favor of the new behaviour (we already provide go-to-def on `override` keyword for example, not having xrefs on it seems surprising). I suppose @hokein can tell if there are any other concerns I am missing.


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1993
         public:
           void $decl[[fu^nc]]() override;
         };
----------------
can we instead turn this test into a multi point check? (e.g. check that references in `virt^ual void fu^nc() over^ride;` ... `D->fu^nc();` are all the same?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111039



More information about the cfe-commits mailing list