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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
