[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

> Concretely, can you give an example of which node and what you'd want it to point to? If it's just the single unqualified-name token, I agree and would like to add it (as a separate patch). If it's the full range, I think that's getSourceRange(). Is it something else?

Yes I was talking about for the single unqualified name.



================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:49
+    EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
+    llvm::Annotations::Range R = A.llvm::Annotations::range();
+    SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,
----------------
sammccall wrote:
> kadircet wrote:
> > can't you just call `A.range()` ? :D
> > I hope it wasn't clangd that somehow code-completed this.
> A.range() is an LSP Location (line/col pairs), offsets seem slightly more convenient here.
> 
> This is ugly though, using Annotations with SourceLocations seems unneccesarily clunky. Ideas?
> 
> (this reminds me, these test helpers need comments, added some)
ah sorry somehow thought this was already an `llvm::Annotations`. You seem to be only using `Code` and `Base::range` and they are both coming from `llvm::Annotations` already. Any reason for not using it directly instead of `clangd::Annotations` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751