[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-04 Thread Kirill Bobyrev 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 rG5eec9a380a24: [clangd] Detect rename conflicits within enclosing scope (authored by kbobyrev). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:411 + if (const auto *EnclosingFor = Parent->get()) +return CheckCompoundStmt(EnclosingFor->getBody(), NewName); + another case we could add is FunctionDecl, if we rename

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Looks good, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925 __

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-04 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321336. kbobyrev marked an inline comment as done. kbobyrev added a comment. Pass const ref to DynTypedNode in helper. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-04 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:334 + DynTypedNodeList Parents(DynTypedNode::create(RenamedDecl)); + auto GetSingleParent = [&](DynTypedNode Node) -> const DynTypedNode * { +Parents = Ctx.getParents(Node); ---

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-04 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321334. kbobyrev marked 5 inline comments as done and an inline comment as not done. kbobyrev added a comment. Fix a couple problems pointed out during the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:333 + // not invalidated. + DynTypedNodeList Parents(DynTypedNode::create(RenamedDecl)); + auto GetSingleParent = [&](DynTypedNode Node) -> const DynTypedNode * { If the int

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321095. kbobyrev marked an inline comment as done. kbobyrev added a comment. Add comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925 Files: clang-tools-extra/c

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:355 + // This helper checks if there is a condition variable has NewName. + auto CheckConditionVariable = [&](const auto *Scope) -> const NamedDecl * { +if (!Scope) hok

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321086. kbobyrev marked 6 inline comments as done. kbobyrev added a comment. Resolve most comments but one: some comments & examples in the code incoming in the next diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. thanks, looks better now, just some nits to improve the code readability. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:332 + DynTypedNodeList Parents = Ctx.getParents(RenamedDecl); + if (Parents.size() != 1 || !Parents.begin()->get()) +

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321055. kbobyrev added a comment. Revert last change: leads to incomplete types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925 Files: clang-tools-extra/clangd/refa

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321053. kbobyrev added a comment. Don't spell out DynTypedNodeList and don't include ParentMapContext.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925 Files: clang-

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321052. kbobyrev added a comment. Save few LOCs by checking for nullptr in CheckDeclStmt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925 Files: clang-tools-extra/cl

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:339 + if (const auto *If = ParentNode->get()) +if (const auto *Then = dyn_cast(If->getThen())) + EnclosingScope = Then; hokein wrote: > thinking more about the `if`

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 321050. kbobyrev marked 3 inline comments as done. kbobyrev added a comment. Resolve review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95925/new/ https://reviews.llvm.org/D95925 Files: clang-to

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Thanks, this looks right to me. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:338 + const CompoundStmt *EnclosingScope = ParentNode->get(); + if (const auto *If = ParentNode->get()) +if (const auto *Then = dyn_cast(If->getThen())) --

[PATCH] D95925: [clangd] Detect rename conflicits within enclosing scope

2021-02-02 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev created this revision. kbobyrev added a reviewer: hokein. Herald added subscribers: usaxena95, kadircet, arphaman. kbobyrev requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. This patch allows detecting conflict