kadircet updated this revision to Diff 226849.
kadircet added a comment.
- Handle unnamed parameters explicitly
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
@@ -1489,6 +1489,45 @@
EXPECT_EQ(apply(Test), Expected);
}
+TEST_F(DefineInlineTest, TransformParamNames) {
+ EXPECT_EQ(apply(R"cpp(
+ void foo(int, bool b, int T\
+est);
+
+ void ^foo(int f, bool x, int z) {})cpp"), R"cpp(
+ void foo(int f, bool x, int z){}
+
+ )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,97 @@
return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
}
+/// 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;
+
+ llvm::DenseMap<const Decl *, std::string> ParamToNewName;
+ llvm::DenseMap<const NamedDecl *, std::vector<SourceLocation>> RefLocs;
+ auto HandleParam = [&](const NamedDecl *DestParam,
+ const NamedDecl *SourceParam) {
+ // No need to rename if parameters already have the same name.
+ if (DestParam->getName() == SourceParam->getName())
+ return;
+ std::string NewName;
+ // Unnamed parameters won't be visited in findExplicitReferences. So add
+ // them here.
+ if (DestParam->getName().empty()) {
+ RefLocs[DestParam].push_back(DestParam->getLocation());
+ // 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).
+ NewName = " ";
+ }
+ NewName.append(SourceParam->getName());
+ ParamToNewName[DestParam->getCanonicalDecl()] = std::move(NewName);
+ };
+
+ // Populate mapping for template parameters.
+ 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)
+ HandleParam(DestTPL->getParam(I), SourceTPL->getParam(I));
+ }
+
+ // Populate mapping for function params.
+ for (size_t I = 0, E = Dest->param_size(); I != E; ++I)
+ HandleParam(Dest->getParamDecl(I), Source->getParamDecl(I));
+
+ llvm::Error RenamingErrors = llvm::Error::success();
+ const SourceManager &SM = Dest->getASTContext().getSourceManager();
+ const LangOptions &LangOpts = Dest->getASTContext().getLangOpts();
+ // Collect other references in function signautre, i.e parameter types and
+ // default arguments.
+ findExplicitReferences(
+ // Use function template in case of templated functions to visit template
+ // parameters.
+ DestTempl ? llvm::dyn_cast<Decl>(DestTempl) : llvm::dyn_cast<Decl>(Dest),
+ [&](ReferenceLoc Ref) {
+ if (Ref.Targets.size() != 1)
+ return;
+ const auto *Target =
+ llvm::cast<NamedDecl>(Ref.Targets.front()->getCanonicalDecl());
+ auto It = ParamToNewName.find(Target);
+ if (It == ParamToNewName.end())
+ return;
+ RefLocs[Target].push_back(Ref.NameLoc);
+ });
+
+ for (auto &Entry : RefLocs) {
+ const auto *OldDecl = Entry.first;
+ llvm::StringRef OldName = OldDecl->getName();
+ llvm::StringRef NewName = ParamToNewName[OldDecl];
+ for (const SourceLocation &RefLoc : Entry.second) {
+ // Can't rely on OldName.size() because of multi-line identifiers, e.g.
+ // int T\
+ // est
+ // Also we can't lex unnamed parameters, since the RefLoc will still point
+ // to a token, e.g. vod foo(int) here for the first parameter, RefLoc will
+ // point at ')'.
+ const size_t OldNameLen =
+ OldName.empty() ? 0 : Lexer::MeasureTokenLength(RefLoc, SM, LangOpts);
+ if (auto Err = Replacements.add(
+ tooling::Replacement(SM, RefLoc, OldNameLen, NewName))) {
+ elog("define inline: Couldn't replace parameter name for {0} to {1}: "
+ "{2}",
+ OldName, NewName, 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 +423,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 +449,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
