[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

This revision was automatically updated to reflect the committed changes. Closed by commit rGb6429cdd65ff: Refactor getDeclAtPosition() to use SelectionTree + targetDecl() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69237/new/

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge updated this revision to Diff 227352. nridge marked 12 inline comments as done. nridge added a comment. Address latest comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69237/new/ https://reviews.llvm.org/D69237 Files: clang-tools-ex

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1133 + DeclRelationSet Relations = + DeclRelation::TemplatePattern | DeclRelation::Alias; + auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations); sammccall wrote: > A

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG, just some style nits, thanks! Comment at: clang-tools-extra/clangd/XRefs.cpp:226 - // Emit all symbol locations (declaration or definition) from AST. - for (con

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge added a comment. In D69237#1722399 , @nridge wrote: > So, to summarize my understanding of the way forward here: > > 1. Land the refactoring without any constructor targeting (i.e. the current > patch, which is ready to review in this state). > 2.

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge updated this revision to Diff 226561. nridge added a comment. Updated not to use the beginning-of-identifier location as input to SelectionTree Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69237/new/ https://reviews.llvm.org/D69237 Files:

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge marked an inline comment as done. nridge added a comment. So, to summarize my understanding of the way forward here: 1. Land the refactoring without any constructor targeting (i.e. the current patch, which is ready to review in this state). 2. In a follow-up patch: target constructor in c

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructo

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructor(const Select

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:147 } + while ((N = N->Parent)) { +if (N->ASTNode.get() || N->ASTNode.get() || sammccall wrote: > nridge wrote: > > This part is a super

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge updated this revision to Diff 226372. nridge marked 4 inline comments as done. nridge added a comment. Removed constructor handling, deferring that to a follow-up Addressed other comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69237/n

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) { --

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructor(const Select

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) { --

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

sammccall added a comment. Thanks! I'd been meaning to do this before my vacation (and had a draft I'll dig up in case there were other issues). Generally I think we should be willing to change behavior in some marginal cases, and I do think it's important to care about the layering of seltree

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge marked 3 inline comments as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructor(const Select

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgrang, jkorous. Herald added a project: clang. This fixes issue #163, among other improvements to go-to-definition. Repository: rG LLVM Github Monorepo htt