[PATCH] D132797: [clangd] Support renaming virtual methods

tom-anders added a comment. In D132797#3774138 , @sammccall wrote: > LG > > IIRC Tom doesn't have commit access, so I'll apply the last trivial changes > and commit to avoid another round trip. > (Please LMK if i'm wrong about this!) I actually have com

[PATCH] D132797: [clangd] Support renaming virtual methods

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8019b46bc70b: [clangd] Support renaming virtual methods (authored by tom-anders, committed by sammccall). Changed prior to commit: https://reviews

[PATCH] D132797: [clangd] Support renaming virtual methods

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG IIRC Tom doesn't have commit access, so I'll apply the last trivial changes and commit to avoid another round trip. (Please LMK if i'm wrong about this!) Comment a

[PATCH] D132797: [clangd] Support renaming virtual methods

ilya-biryukov added a comment. A few more NITs from me, but please wait for @sammccall for another round of reviews. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:550 +// method. +void recursivelyInsertOverrides(SymbolID Base, llvm::DenseSet &IDs, +

[PATCH] D132797: [clangd] Support renaming virtual methods

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:554 + Req.Subjects = {Base}; + Index.relations(Req, [&](const SymbolID &, const Symbol &Override) { +IDs.insert(Override.ID); tom-anders wrote: > ilya-biryukov wrote:

[PATCH] D132797: [clangd] Support renaming virtual methods

tom-anders updated this revision to Diff 458270. tom-anders marked an inline comment as done. tom-anders added a comment. Fix recursivelyInsertRenames() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132797/new/ https://reviews.llvm.org/D132797 Fil

[PATCH] D132797: [clangd] Support renaming virtual methods

tom-anders marked 6 inline comments as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:245 trace::Span Tracer("FindOccurrencesWithinFile"); assert(canonicalRenameDecl(&ND) == &ND && "ND should be already canonicaliz

[PATCH] D132797: [clangd] Support renaming virtual methods

tom-anders updated this revision to Diff 458221. tom-anders added a comment. Add test case that triggers assertion, limit recursion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132797/new/ https://reviews.llvm.org/D132797 Files: clang-tools-ext

[PATCH] D132797: [clangd] Support renaming virtual methods

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:554 + Req.Subjects = {Base}; + Index.relations(Req, [&](const SymbolID &, const Symbol &Override) { +IDs.insert(Override.ID); sammccall wrote: > ilya-biryukov wrot

[PATCH] D132797: [clangd] Support renaming virtual methods

sammccall added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:548 +namespace { +void recursivelyInsertOverrides(const SymbolID &Base, +llvm::DenseSet &IDs, sammccall wrote: > The fact that we only need t

[PATCH] D132797: [clangd] Support renaming virtual methods

ilya-biryukov added a comment. Thanks for the patch! A drive-by comment from me, hopefully useful. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:554 + Req.Subjects = {Base}; + Index.relations(Req, [&](const SymbolID &, const Symbol &Override) { +IDs.insert(Over

[PATCH] D132797: [clangd] Support renaming virtual methods

sammccall added a comment. Sorry for the delay, this patch is really neat! The only serious thing is it uncovers an existing bug which asserts. As a potential new crash I think we should fix that - LMK whether you feel comfortable adding that fix in here or you'd like me to do it as a separate

[PATCH] D132797: [clangd] Support renaming virtual methods

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. Fixes https://github.com/clangd/clang