diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ b/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 +renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) { + tooling::Replacements Replacements; + + llvm::DenseMap ParamToNewName; + llvm::DenseMap> 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(DestTempl) : llvm::dyn_cast(Dest), + [&](ReferenceLoc Ref) { + if (Ref.Targets.size() != 1) + return; + const auto *Target = + llvm::cast(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, 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)); diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/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, template class Y, + int, int Z> + void foo(X, Y, int W = 5 * Z + 2); + }; + }; + + template class V, template class W, + int X, int Y> + void Foo::Bar::f^oo(U, W, int Q) {})cpp"), + R"cpp( + struct Foo { + struct Bar { + template class V, template class W, + int X, int Y> + void foo(U, W, int Q = 5 * Y + 2){} + }; + }; + + )cpp"); +} + TEST_F(DefineInlineTest, TransformInlineNamespaces) { auto Test = R"cpp( namespace a { inline namespace b { namespace { struct Foo{}; } } }