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<CompoundStmt>();
+ if (const auto *If = ParentNode->get<IfStmt>())
+ if (const auto *Then = dyn_cast<CompoundStmt>(If->getThen()))
----------------
I found that the current structure (we have two places of this `If` cases, and
they are separated) is a bit hard to follow, I'd suggest union them, just like
the code below, it seems straight-forward to me, and easy to extend.
```
if (const auto* CompoundStmtEnclosing = Parents.begin()->get<CompoundStmt>()) {
CheckCompoundStmt(CompoundStmtEnclosing);
// below we detect conflicts in conditions or others (if the CompoundStmt is
a function body, we could detect the parameter decls as well)
getParents(CompoundStmtEnclosing);
if ( ...<IfStmt>()) {
CheckConditionVariable();
} else if (...) {
}
} else if (const auto* IfEnclosing = ... <IfStmt>()) {
CheckCompoundStmt(If->getThen());
// possibly CheckCompoundStmt(If->getElse)
} else {
...
}
```
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:339
+ if (const auto *If = ParentNode->get<IfStmt>())
+ if (const auto *Then = dyn_cast<CompoundStmt>(If->getThen()))
+ EnclosingScope = Then;
----------------
thinking more about the `if` case, I think else should be included as well? no
need to address in this patch.
like
```
if (int a = 0) {
} else {
int s; // rename s=>a will cause a compiling error.
}
```
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:352
+ for (const auto &Child : DS->getDeclGroup())
+ if (const auto *VD = dyn_cast<VarDecl>(Child))
+ if (VD != &RenamedDecl && VD->getName() == NewName)
----------------
nit: VarDecl => NamedDecl
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
