kadircet updated this revision to Diff 225209.
kadircet marked an inline comment as done.
kadircet added a comment.
- Accept StringRef for Source in rewriteParameterName instead of a NamedDecl
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
@@ -1290,6 +1290,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,
+ llvm::StringRef SourceName) {
+ auto &SM = Dest->getASTContext().getSourceManager();
+ llvm::StringRef DestName = Dest->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)->getName()))
+ 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->getName()))
+ 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
