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 intention is for storage, maybe call it Storage.
================
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);
----------------
`const DynTypedNode &`
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:362
+ for (const auto *Node : CS->children())
+ return CheckDeclStmt(dyn_cast<DeclStmt>(Node), Name);
+ return nullptr;
----------------
I think we need to iterate *all* children, rather than the first one. looks
like our testcase doesn't cover this, maybe add one.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:372
+
+ // CompoundStmt is the most common enclosing scope. In the simplest case we
+ // just iterate through sibling DeclStmts and check for collisions.
----------------
CompoundStmt is the most common enclosing scope for function-local symbols.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:378
+ const auto *ScopeParent =
+ GetSingleParent(DynTypedNode::create(*EnclosingCS));
+ // CompoundStmt may be found within if/while/for. In these cases, rename
can
----------------
maybe just `GetSingleParent(Parent)`, rather than creating a new DynTypeNode.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:390
+ if (const auto *Result =
+ CheckDeclStmt(dyn_cast<DeclStmt>(For->getInit()), NewName))
+ return Result;
----------------
be careful about nullptr here, the init-statement could be a nullptr, which
will trigger a crash in dyn_cast, thinking about the case below:
```
for (; ; ;) {
int var;
}
```
use `dyn_cast_or_null`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95925/new/
https://reviews.llvm.org/D95925
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
