[PATCH] D71090: [clangd] Navigation from definition of template specialization to primary templateFixes https://github.com/clangd/clangd/issues/212.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice!



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:194
   // As a consequence, there's no need to look them up in the index either.
-  SourceLocation MaybeMacroLocation = SM.getMacroArgExpandedLocation(
+  SourceLocation IdentStartLoc = SM.getMacroArgExpandedLocation(
       getBeginningOfIdentifier(Pos, AST.getSourceManager(), AST.getLangOpts()));
----------------
Not thrilled about reusing this after previous patches disentangled it - it works well for macros but if (for example) you have a templated `operator<<` I think the feature you're adding probably won't work.

That said, I don't see a simple alternative, and it's unlikely to be a big deal, so I think this is OK.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:240
+    // it's more useful to navigate to the template declaration.
+    if (Preferred->getLocation() == IdentStartLoc) {
+      if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(Preferred)) {
----------------
Be aware that this is going to break "go to definition" toggling between def and decl. I think that's fine.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:240
+    // it's more useful to navigate to the template declaration.
+    if (Preferred->getLocation() == IdentStartLoc) {
+      if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(Preferred)) {
----------------
sammccall wrote:
> Be aware that this is going to break "go to definition" toggling between def and decl. I think that's fine.
(I think you probably want the macro-arg-expanded location here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71090