kadircet updated this revision to Diff 226619.
kadircet added a comment.
- Don't rename if parameters have the same name
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68937/new/
https://reviews.llvm.org/D68937
Files:
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1494,6 +1494,44 @@
EXPECT_EQ(apply(Test), Expected);
}
+TEST_F(DefineInlineTest, TransformParamNames) {
+ EXPECT_EQ(apply(R"cpp(
+ void foo(int, bool b);
+
+ void ^foo(int f, bool x) {})cpp"), R"cpp(
+ void foo(int f, bool x){}
+
+ )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+ EXPECT_EQ(apply(R"cpp(
+ struct Foo {
+ struct Bar {
+ template <class, class X,
+ template<typename> class, template<typename> class Y,
+ int, int Z>
+ void foo(X, Y<X>, int W = 5 * Z + 2);
+ };
+ };
+
+ template <class T, class U,
+ template<typename> class V, template<typename> class W,
+ int X, int Y>
+ void Foo::Bar::f^oo(U, W<U>, int Q) {})cpp"),
+ R"cpp(
+ struct Foo {
+ struct Bar {
+ template <class T, class U,
+ template<typename> class V, template<typename> class W,
+ int X, int Y>
+ void foo(U, W<U>, int Q = 5 * Y + 2){}
+ };
+ };
+
+ )cpp");
+}
+
TEST_F(DefineInlineTest, TransformInlineNamespaces) {
auto Test = R"cpp(
namespace a { inline namespace b { namespace { struct Foo{}; } } }
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,80 @@
return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
}
+/// Returns true if Dest is templated function. Fills in \p ParamToNewName with
+/// a mapping from template decls in \p Dest to theire respective names in \p
+/// Source.
+bool fillTemplateParameterMapping(
+ const FunctionDecl *Dest, const FunctionDecl *Source,
+ llvm::DenseMap<const Decl *, std::string> &ParamToNewName) {
+ auto *DestTempl = Dest->getDescribedFunctionTemplate();
+ auto *SourceTempl = Source->getDescribedFunctionTemplate();
+ assert(bool(DestTempl) == bool(SourceTempl));
+ if (!DestTempl)
+ return false;
+ const auto *DestTPL = DestTempl->getTemplateParameters();
+ const auto *SourceTPL = SourceTempl->getTemplateParameters();
+ assert(DestTPL->size() == SourceTPL->size());
+
+ for (size_t I = 0, EP = DestTPL->size(); I != EP; ++I)
+ ParamToNewName[DestTPL->getParam(I)->getCanonicalDecl()] =
+ SourceTPL->getParam(I)->getName();
+ return true;
+}
+
+/// Generates Replacements for changing template and function parameter names in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected<tooling::Replacements>
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+ llvm::DenseMap<const Decl *, std::string> ParamToNewName;
+ bool HasTemplateParams =
+ fillTemplateParameterMapping(Dest, Source, ParamToNewName);
+ tooling::Replacements Replacements;
+
+ // Populate mapping for function params.
+ for (size_t I = 0, E = Dest->param_size(); I != E; ++I) {
+ auto *DestParam = Dest->getParamDecl(I);
+ auto *SourceParam = Source->getParamDecl(I);
+ ParamToNewName[DestParam] = SourceParam->getName();
+ }
+
+ llvm::Error RenamingErrors = llvm::Error::success();
+ const SourceManager &SM = Dest->getASTContext().getSourceManager();
+ findExplicitReferences(
+ // Use function template in case of templated functions to visit template
+ // parameters.
+ HasTemplateParams
+ ? llvm::dyn_cast<Decl>(Dest->getDescribedFunctionTemplate())
+ : llvm::dyn_cast<Decl>(Dest),
+ [&](ReferenceLoc Ref) {
+ if (Ref.Targets.size() != 1)
+ return;
+ const auto *Target =
+ llvm::dyn_cast<NamedDecl>(Ref.Targets.front()->getCanonicalDecl());
+ auto It = ParamToNewName.find(Target);
+ if (It == ParamToNewName.end())
+ return;
+ // No need to rename if parameters already have the same name.
+ if (Target->getName() == It->second)
+ return;
+ const size_t OldNameLen = Target->getName().size();
+ // If decl is unnamed in destination we pad the new name to avoid gluing
+ // with previous token, e.g. foo(int^) shouldn't turn into foo(intx).
+ auto ReplacementText = (OldNameLen == 0 ? " " : "") + It->second;
+ if (auto Err = Replacements.add(tooling::Replacement(
+ SM, Ref.NameLoc, OldNameLen, ReplacementText))) {
+ elog("define inline: Couldn't replace parameter name for {0} to {1}: "
+ "{2}",
+ Target->getName(), ReplacementText, Err);
+ RenamingErrors =
+ llvm::joinErrors(std::move(RenamingErrors), std::move(Err));
+ }
+ });
+ if (RenamingErrors)
+ return std::move(RenamingErrors);
+ return Replacements;
+}
+
// Returns the canonical declaration for the given FunctionDecl. This will
// usually be the first declaration in current translation unit with the
// exception of template specialization.
@@ -332,6 +406,10 @@
"Couldn't find semicolon for target declaration.");
}
+ auto ParamReplacements = renameParameters(Target, Source);
+ if (!ParamReplacements)
+ return ParamReplacements.takeError();
+
auto QualifiedBody = qualifyAllDecls(Source);
if (!QualifiedBody)
return QualifiedBody.takeError();
@@ -354,8 +432,9 @@
llvm::SmallVector<std::pair<std::string, Edit>, 2> Edits;
// Edit for Target.
- auto FE = Effect::fileEdit(SM, SM.getFileID(*Semicolon),
- tooling::Replacements(SemicolonToFuncBody));
+ auto FE = Effect::fileEdit(
+ SM, SM.getFileID(*Semicolon),
+ tooling::Replacements(SemicolonToFuncBody).merge(*ParamReplacements));
if (!FE)
return FE.takeError();
Edits.push_back(std::move(*FE));
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
