[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D133664#3791694 , @tom-anders wrote: > Hmm, Github noticed that I referenced the issue with this commit, but didn't > close it. > According to > https://github.blog/2013-03-18-closing-issues-across-repositories/ closing >

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-15 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders added a comment. Hmm, Github noticed that I referenced the issue with this commit, but didn't close it. According to https://github.blog/2013-03-18-closing-issues-across-repositories/ closing issues across repos should work, but only if you have push permissions in the repo that has

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-15 Thread Tom Praschan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG220d85082349: [clangd] Fix hover on symbol introduced by using declaration (authored by tom-anders). Changed prior to commit: https://reviews.llvm.org/D133664?vs=460058&id=460328#toc Repository: rG L

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D133664#3790144 , @tom-anders wrote: > Yeah I do have commit access now, so I'll land this by myself. > > I don't think I have the permissions to close the corresponding issue on > Github though, so someone else would need to

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders added a comment. Yeah I do have commit access now, so I'll land this by myself. I don't think I have the permissions to close the corresponding issue on Github though, so someone else would need to do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! i think you have commit access now, but let me know if I should land this for you (preferably with an email address for commit attribution) Comment at: cla

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1011 +const NamedDecl * +pickDeclToUse(const llvm::SmallVector &Candidates) { + if (Candidates.empty()) kadircet wrote: > you can just pass in ArrayRef instead. Ah of course, forgot

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders updated this revision to Diff 460058. tom-anders marked 5 inline comments as done. tom-anders added a comment. Add additional test, tidy up logic in pickDeclToUse() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133664/new/ https://review

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1011 +const NamedDecl * +pickDeclToUse(const llvm::SmallVector &Candidates) { + if (Candidates.empty()) you can just pass in ArrayRef instead. Repository: rG LLVM Github Monorepo

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1023 + // which in this example would be the actual declaration of foo. + if (Candidates.size() == 2) { +if (llvm::isa(Candidates.front())) nit: you can change this to `Candidate

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-13 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders updated this revision to Diff 459864. tom-anders added a comment. Move logic to pickDeclToUse Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133664/new/ https://reviews.llvm.org/D133664 Files: clang-tools-extra/clangd/Hover.cpp clang

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1091 +// better, which in this example would be the actual declaration of foo. +auto *DeclToUse = Decls.begin(); +while (llvm::isa(*DeclToUse) && we actually w

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-11 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. tom-anders requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This fixes https://github.com/clangd/