sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:92
+ DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+ const auto *ND = llvm::cast<NamedDecl>(D);
+ // Get to CXXRecordDecl from constructor or destructor.
----------------
OK, this can definitely fail though - just select 'friend X' in a class body or
so. Need to check for ND and bail out.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:223
- ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
- // getUSRsForDeclaration will find other related symbols, e.g. virtual and
its
- // overriddens, primary template and all explicit specializations.
----------------
you removed this comment and the fixme, but it still applies?
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:732
+ {
+ // Constructor.
+ R"cpp(
----------------
nit: put these after "class methods" rather than at the top?
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:750
+ {
+ // Destructor (selecting before the identifier).
+ R"cpp(
----------------
4 is too many cases for constructor/destructor.
What about 2: Constructor at start, destructor at end?
Renaming in the middle of the token is covered by existing cases.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71247/new/
https://reviews.llvm.org/D71247
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
