kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.
When moving a function definition to declaration location we also need
to handle renaming of the both function and template parameters.
This patch achives that by making sure every parameter name and dependent type
in destination is renamed to their respective name in the source.
Repository:
rG LLVM Github Monorepo
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
@@ -1285,6 +1285,44 @@
)cpp");
}
+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) {
EXPECT_EQ(apply(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,176 @@
return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
}
+/// Generates a replacement that renames \p Dest to \p Source.
+llvm::Error rewriteParameterName(tooling::Replacements &Replacements,
+ const NamedDecl *Dest,
+ const NamedDecl *Source) {
+ auto &SM = Dest->getASTContext().getSourceManager();
+ llvm::StringRef DestName = Dest->getName();
+ llvm::StringRef SourceName = Source->getName();
+ if (DestName == SourceName)
+ return llvm::Error::success();
+ if (auto Err = Replacements.add(tooling::Replacement(
+ SM, Dest->getLocation(), DestName.size(),
+ // If \p Dest is unnamed, we need to insert a space before current
+ // name.
+ ((DestName.empty() ? " " : "") + SourceName).str()))) {
+ return Err;
+ }
+ return llvm::Error::success();
+}
+
+/// Returns a mapping from template names in \p Dest to \p Source, so that they
+/// can be updated in other places like parameter types and default arguments.
+llvm::Expected<llvm::DenseMap</*DestName*/ llvm::StringRef,
+ /*SourceName*/ llvm::StringRef>>
+renameTemplateParameters(tooling::Replacements &Replacements,
+ const FunctionDecl *Dest, const FunctionDecl *Source) {
+ llvm::DenseMap<llvm::StringRef, llvm::StringRef> TemplParamNameDestToSource;
+
+ auto *DestTempl = Dest->getDescribedFunctionTemplate();
+ auto *SourceTempl = Source->getDescribedFunctionTemplate();
+ assert(bool(DestTempl) == bool(SourceTempl));
+ if (DestTempl) {
+ 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) {
+ if (auto Err = rewriteParameterName(Replacements, DestTPL->getParam(I),
+ SourceTPL->getParam(I)))
+ return std::move(Err);
+ TemplParamNameDestToSource[DestTPL->getParam(I)->getName()] =
+ SourceTPL->getParam(I)->getName();
+ }
+ }
+
+ return TemplParamNameDestToSource;
+}
+
+/// Generates replacements for rewriting dependent types in \p DestParam to the
+/// matching template name in \p TemplParamNameDestToSource.
+llvm::Error
+rewriteParameterType(tooling::Replacements &Replacements,
+ const ParmVarDecl *DestParam,
+ const llvm::DenseMap<llvm::StringRef, llvm::StringRef>
+ &TemplParamNameDestToSource) {
+ auto *DestTSI = DestParam->getTypeSourceInfo();
+ if (!DestTSI)
+ return llvm::Error::success();
+ const SourceManager &SM = DestParam->getASTContext().getSourceManager();
+
+ // Update paramater types if they are template template or type template.
+ auto DestTL = DestTSI->getTypeLoc();
+ if (auto DestTPTL = DestTL.getAs<TemplateTypeParmTypeLoc>()) {
+ if (auto Err = Replacements.add(tooling::Replacement(
+ SM, CharSourceRange(DestTPTL.getSourceRange(), /*ITR=*/true),
+ TemplParamNameDestToSource.lookup(
+ DestTPTL.getDecl()->getDeclName().getAsString())))) {
+ return Err;
+ }
+ } else if (auto DestTTSL = DestTL.getAs<TemplateSpecializationTypeLoc>()) {
+ std::string TemplName;
+ llvm::raw_string_ostream OS(TemplName);
+ DestTTSL.getTypePtr()->getTemplateName().print(
+ OS, DestParam->getASTContext().getPrintingPolicy());
+ OS.flush();
+ if (auto Err = Replacements.add(tooling::Replacement(
+ SM, CharSourceRange(DestTTSL.getTemplateNameLoc(), /*ITR=*/true),
+ TemplParamNameDestToSource.lookup(TemplName)))) {
+ return Err;
+ }
+
+ for (size_t I = 0, E = DestTTSL.getNumArgs(); I != E; ++I) {
+ const auto &TAL = DestTTSL.getArgLoc(I);
+ std::string TemplName;
+ llvm::raw_string_ostream OS(TemplName);
+ TAL.getArgument().print(DestParam->getASTContext().getPrintingPolicy(),
+ OS);
+ OS.flush();
+ if (auto Err = Replacements.add(tooling::Replacement(
+ SM, CharSourceRange(TAL.getLocation(), /*ITR=*/true),
+ TemplParamNameDestToSource.lookup(TemplName)))) {
+ return Err;
+ }
+ }
+ }
+
+ return llvm::Error::success();
+}
+
+/// Renames any occurences of template names in default argument of \p DestParam
+/// to the matching template name in \p TemplParamNameDestToSource.
+llvm::Error
+rewriteDefaultArg(tooling::Replacements &Replacements,
+ const ParmVarDecl *DestParam,
+ const llvm::DenseMap<llvm::StringRef, llvm::StringRef>
+ &TemplParamNameDestToSource) {
+ llvm::Error Err = llvm::Error::success();
+
+ auto *DefArg = DestParam->getDefaultArg();
+ if (!DefArg)
+ return Err;
+ const auto &SM = DestParam->getASTContext().getSourceManager();
+
+ findExplicitReferences(DefArg, [&](ReferenceLoc RefLoc) {
+ if (Err)
+ return;
+ for (const NamedDecl *ND : RefLoc.Targets) {
+ llvm::StringRef SourceName =
+ TemplParamNameDestToSource.lookup(ND->getName());
+ if (SourceName.empty())
+ continue;
+ if (auto Error = Replacements.add(tooling::Replacement(
+ SM, CharSourceRange(RefLoc.NameLoc, /*ITR=*/true), SourceName))) {
+ Err = std::move(Error);
+ break;
+ }
+ }
+ });
+
+ return Err;
+}
+
+/// 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) {
+ tooling::Replacements Replacements;
+
+ // To be used later on when mapping dependent types in function parameters.
+ auto TemplParamNameDestToSource =
+ renameTemplateParameters(Replacements, Dest, Source);
+ if (!TemplParamNameDestToSource)
+ return TemplParamNameDestToSource.takeError();
+
+ // Fix function parameters.
+ assert(Dest->param_size() == Source->param_size());
+ for (size_t I = 0, E = Dest->param_size(); I != E; ++I) {
+ auto *DestParam = Dest->getParamDecl(I);
+ auto *SourceParam = Source->getParamDecl(I);
+
+ if (auto Err = rewriteParameterName(Replacements, DestParam, SourceParam))
+ return std::move(Err);
+
+ // If function is not templated, only updating the parameter name is enough.
+ // Otherwise we need to update dependent types in parameter types and
+ // dependent values in default arguments.
+ if (TemplParamNameDestToSource->empty())
+ continue;
+
+ if (auto Err = rewriteParameterType(Replacements, DestParam,
+ *TemplParamNameDestToSource))
+ return std::move(Err);
+
+ if (auto Err = rewriteDefaultArg(Replacements, DestParam,
+ *TemplParamNameDestToSource))
+ return std::move(Err);
+ }
+
+ 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.
@@ -327,6 +497,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();
@@ -340,8 +514,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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
