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
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
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
__
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
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);
---
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
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
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
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
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
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())
+
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
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-
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
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`
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
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()))
--
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
18 matches
Mail list logo