hokein added a comment.
didn't finish all parts (looked at the FunctionDecl and RecordDecl), I think we
need more documentation/comments in the code to specify the canonical behavior
(templates are tricky).
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:239
+ if (const auto *Template = D->getPrimaryTemplate())
+ return canonicalRenameDecl(Template);
+ return D;
----------------
the `auto` + varies `canonicalRenameDecl` overrides make the code hard to
follow.
since these functions are small, I think we can inline them into the main
`canonicalRenameDecl(const NamedDecl *D)`
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:246
+ if (const auto *Primary = TemplatedDecl->getPrimaryTemplate())
+ return Primary;
+ return TemplatedDecl;
----------------
didn't quite follow the code here, the code looks like we get the
FunctionTemplateDecl back via the code path (FunctionTemplateDecl ->
FunctionDecl -> FunctionTemplateDecl), and it looks like we're not using the
specialized declaration as the canonical decl.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:255
+const NamedDecl *canonicalRenameDecl(const ClassTemplateSpecializationDecl *D)
{
+ return D->getSpecializedTemplate()->getTemplatedDecl();
+}
----------------
want to confirm my understanding:
given the example below:
```
template<typename T>
class Foo {};
template<>
class Foo<int> {};
```
the AST is like:
```
ClassTemplateDecl
|-CXXRecordDecl (Foo definition) -> (1)
|-ClassTemplateSpecialization.
ClassTemplateSpecializationDecl -> call canonicalRenameDecl on it.
|-Template Argument int
|-CXXRecordDecl -> (2)
```
if we pass `ClassTemplateSpecializationDecl` to this function, this function
will return (2)?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:261
+// CXXRecordDecl
+// - Specializations should point to the specialized declaration.
+// - Instantiations should point to instantiated declaration.
----------------
I think the motivation is for merely renaming the explicit template
specialization, but not the primary template?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:610
const auto &RenameDecl =
llvm::cast<NamedDecl>(*(*DeclsUnderCursor.begin())->getCanonicalDecl());
if (RenameDecl.getName() == RInputs.NewName)
----------------
`getCanonicalDecl` is not needed now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71880/new/
https://reviews.llvm.org/D71880
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
