kbobyrev updated this revision to Diff 322382.
kbobyrev added a comment.
Rebase on top of D96247 <https://reviews.llvm.org/D96247>, check for conflicts
in specialization bodies.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96324/new/
https://reviews.llvm.org/D96324
Files:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -678,6 +678,61 @@
}
)cpp",
+ // Function argument.
+ R"cpp(
+ // Main.cpp
+ void foo(int [[Bar^]]);
+
+ void foo(int [[Bar^]]) {}
+
+ // Forward declarations somewhere else.
+ void foo(int [[Bar^]]);
+ void foo(int /*Bar*/);
+ )cpp",
+
+ // Template function argument.
+ R"cpp(
+ template <typename T>
+ void foo(T [[Bar^]]);
+
+ template <typename T>
+ void foo(T [[Bar^]]) {}
+ )cpp",
+ R"cpp(
+ template <typename T>
+ void foo(T [[Bar^]]);
+
+ template <typename T>
+ void foo(T [[Bar^]]) {}
+
+ template <>
+ void foo(int [[Bar^]]) {}
+ )cpp",
+
+ // Constructor argument.
+ R"cpp(
+ class Foo {
+ Foo(int [[Bar^]]);
+ };
+ Foo::Foo(int [[Bar^]]) {}
+ )cpp",
+ R"cpp(
+ class Foo {
+ template <typename T, typename U>
+ Foo(T Something, U [[Bar^]]);
+
+ Foo();
+ };
+
+ template <typename T, typename U>
+ Foo::Foo(T Something, U [[Bar^]]) {}
+
+ template <>
+ Foo::Foo(int Something, int [[Bar^]]) {}
+ template <>
+ Foo::Foo(bool Something, bool [[Bar^]]) {}
+ )cpp",
+
// Namespace alias.
R"cpp(
namespace a { namespace b { void foo(); } }
@@ -1091,14 +1146,21 @@
)cpp",
"conflict", !HeaderFile, nullptr, "Conflict"},
- {R"cpp(// No conflict: only forward declaration's argument is renamed.
- void func(int [[V^ar]]);
+ {R"cpp(
+ template <typename T>
+ void func(T Var);
+
+ template <>
+ void func(bool V^ar) {}
+ template <>
void func(int Var) {
- bool Conflict;
+ // We are renaming from the primary template but the parameter name
+ // will also be changed here introducing a new conflict.
+ int Conflict;
}
)cpp",
- nullptr, !HeaderFile, nullptr, "Conflict"},
+ "conflict", !HeaderFile, nullptr, "Conflict"},
{R"cpp(
void func(int V^ar, int Conflict) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -100,7 +100,8 @@
//
// Here, both partial (2) and full (3) specializations are canonicalized to (1)
// which ensures all three of them are renamed.
-const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D,
+ bool CanonicalizeCtorToType = true) {
if (const auto *VarTemplate = dyn_cast<VarTemplateSpecializationDecl>(D))
return canonicalRenameDecl(
VarTemplate->getSpecializedTemplate()->getTemplatedDecl());
@@ -113,9 +114,17 @@
ClassTemplateSpecialization->getSpecializedTemplate()
->getTemplatedDecl());
if (const auto *Method = dyn_cast<CXXMethodDecl>(D)) {
- if (Method->getDeclKind() == Decl::Kind::CXXConstructor ||
+ // By default ctors and dtors are canonicalized to the returned type but
+ // when CanonicalizeCtorToType is not set, we should still retrieve the
+ // canonical method.
+ if ((Method->getDeclKind() == Decl::Kind::CXXConstructor &&
+ CanonicalizeCtorToType) ||
Method->getDeclKind() == Decl::Kind::CXXDestructor)
return canonicalRenameDecl(Method->getParent());
+ if (const FunctionTemplateDecl *Template = Method->getPrimaryTemplate())
+ if (const auto *TemplatedDecl =
+ dyn_cast<CXXMethodDecl>(Template->getTemplatedDecl()))
+ Method = TemplatedDecl;
if (const FunctionDecl *InstantiatedMethod =
Method->getInstantiatedFromMemberFunction())
Method = cast<CXXMethodDecl>(InstantiatedMethod);
@@ -151,6 +160,19 @@
if (const auto *VD = dyn_cast<VarDecl>(D)) {
if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember())
VD = OriginalVD;
+ // Arguments are canonicalized to their canonical function's arguments.
+ if (const auto *Argument = dyn_cast<ParmVarDecl>(VD)) {
+ if (const auto *Function =
+ dyn_cast<FunctionDecl>(Argument->getDeclContext())) {
+ const auto *Canonical = dyn_cast<FunctionDecl>(
+ canonicalRenameDecl(Function, /*CanonicalizeCtorToType=*/false));
+ assert(Canonical &&
+ "canonicalRenameDecl should return the declaration "
+ "of the same type with CanonicalizeCtorToType=true.");
+ return Canonical->getParamDecl(Argument->getFunctionScopeIndex())
+ ->getCanonicalDecl();
+ }
+ }
return VD->getCanonicalDecl();
}
return dyn_cast<NamedDecl>(D->getCanonicalDecl());
@@ -421,11 +443,18 @@
for (const auto *Parameter : EnclosingFunction->parameters())
if (Parameter != &RenamedDecl && Parameter->getName() == NewName)
return Parameter;
- // FIXME: We don't modify all references to function parameters when
- // renaming from forward declaration now, so using a name colliding with
- // something in the definition's body is a valid transformation.
- if (!EnclosingFunction->doesThisDeclarationHaveABody())
- return nullptr;
+ EnclosingFunction = dyn_cast<FunctionDecl>(canonicalRenameDecl(
+ EnclosingFunction, /*CanonicalizeCtorToType=*/false));
+ assert(EnclosingFunction &&
+ "canonicalRenameDecl should return the declaration "
+ "of the same type with CanonicalizeCtorToType=false.");
+ // Because we will be renaming parameter in all specializations, check
+ // their bodies for conflict, too.
+ if (const auto *Template =
+ EnclosingFunction->getDescribedFunctionTemplate())
+ for (const auto *Spec : Template->specializations())
+ if (const auto *Result = CheckCompoundStmt(Spec->getBody(), NewName))
+ return Result;
return CheckCompoundStmt(EnclosingFunction->getBody(), NewName);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
