[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-09 Thread Tom Praschan via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGac21938fbdfa: [clangd] Fix rename for symbol introduced by UsingDecl (authored by tom-anders). Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-09 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders marked 2 inline comments as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:170 +void filterUsingDecl(llvm::DenseSet& Decls) { + // There should never be more than one UsingDecl here, + // otherwise the rename would b

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:169 +// For renaming, we're only interested in foo's declaration, so drop the other one +void filterUs

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-08 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders updated this revision to Diff 466292. tom-anders added a comment. Add additional tests, only handle UsingDecl instead of BaseUsingDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135489/new/ https://reviews.llvm.org/D135489 Files: c

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-08 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders marked 4 inline comments as done. tom-anders added a comment. In D135489#3844426 , @sammccall wrote: > namespace ns { int foo(int); char foo(char); } > using ns::foo; > int x = foo(42); > char y = foo('x'); > > What should happen when w

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:169 +// For renaming, we're only interested in foo's declaration, so drop the other one +void filterBaseUsingDecl(llvm::DenseSet& Decls) { + if (Decls.size() == 2) { sammc

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. (Thanks for fixing this BTW!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135489/new/ https://reviews.llvm.org/D135489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Want to probe a bit at the behavior we're aiming for here. TL;DR is I think the handling is basically right but considering the general case tells us how to simplify filterBaseUsingDecl. --- The testcase is great to illustrate the common case. The thing we need to gen

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-07 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:169 +// For renaming, we're only interested in foo's declaration, so drop the other one +void filterBaseUsingDecl(llvm::DenseSet& Decls) { + if (Decls.size() == 2) { I'm

[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

2022-10-07 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders created this revision. tom-anders added a reviewer: kadircet. Herald added a subscriber: 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 h