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,107 @@ 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) { + 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. + assert(Dest->param_size() == Source->param_size()); + for (size_t I = 0, E = Dest->param_size(); I != E; ++I) + HandleParam(Dest->getParamDecl(I), Source->getParamDecl(I)); + + const SourceManager &SM = Dest->getASTContext().getSourceManager(); + const LangOptions &LangOpts = Dest->getASTContext().getLangOpts(); + // Collect other references in function signature, 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); + }); + + // Now try to generate edits for all the refs. + tooling::Replacements Replacements; + for (auto &Entry : RefLocs) { + const auto *OldDecl = Entry.first; + llvm::StringRef OldName = OldDecl->getName(); + llvm::StringRef NewName = ParamToNewName[OldDecl]; + for (SourceLocation RefLoc : Entry.second) { + CharSourceRange ReplaceRange; + // In case of unnamed parameters, we have an empty char range, whereas we + // have a tokenrange at RefLoc with named parameters. + if (OldName.empty()) + ReplaceRange = CharSourceRange::getCharRange(RefLoc, RefLoc); + else + ReplaceRange = CharSourceRange::getTokenRange(RefLoc, RefLoc); + // If occurence is coming from a macro expansion, try to get back to the + // file range. + if (RefLoc.isMacroID()) { + ReplaceRange = Lexer::makeFileCharRange(ReplaceRange, SM, LangOpts); + // Bail out if we need to replace macro bodies. + if (ReplaceRange.isInvalid()) { + auto Err = llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Cant rename parameter inside macro body."); + elog("define inline: {0}", Err); + return std::move(Err); + } + } + + if (auto Err = Replacements.add( + tooling::Replacement(SM, ReplaceRange, NewName))) { + elog("define inline: Couldn't replace parameter name for {0} to {1}: " + "{2}", + OldName, NewName, Err); + 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. @@ -332,6 +433,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 +459,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 @@ -40,9 +40,9 @@ #include using ::testing::AllOf; +using ::testing::ElementsAre; using ::testing::HasSubstr; using ::testing::StartsWith; -using ::testing::ElementsAre; namespace clang { namespace clangd { @@ -1386,7 +1386,7 @@ // Template body is not parsed until instantiation time on windows, which // results in arbitrary failures as function body becomes NULL. ExtraArgs.push_back("-fno-delayed-template-parsing"); - for(const auto &Case : Cases) + for (const auto &Case : Cases) EXPECT_EQ(apply(Case.first), Case.second) << Case.first; } @@ -1429,7 +1429,7 @@ } TEST_F(DefineInlineTest, TransformDeclRefs) { - auto Test =R"cpp( + auto Test = R"cpp( namespace a { template class Bar { public: @@ -1498,6 +1498,70 @@ EXPECT_EQ(apply(Test), Expected); } +TEST_F(DefineInlineTest, TransformParamNames) { + std::pair Cases[] = { + {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"}, + {R"cpp( + #define PARAM int Z + void foo(PARAM); + + void ^foo(int X) {})cpp", + "fail: Cant rename parameter inside macro body."}, + {R"cpp( + #define TYPE int + #define PARAM TYPE Z + #define BODY(x) 5 * (x) + 2 + template + void foo(PARAM, TYPE Q, TYPE, TYPE W = BODY(P)); + template + void ^foo(int Z, int b, int c, int d) {})cpp", + R"cpp( + #define TYPE int + #define PARAM TYPE Z + #define BODY(x) 5 * (x) + 2 + template + void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){} + )cpp"}, + }; + for (const auto &Case : Cases) + EXPECT_EQ(apply(Case.first), Case.second) << Case.first; +} + +TEST_F(DefineInlineTest, TransformTemplParamNames) { + auto Test = 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"; + auto Expected = 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"; + EXPECT_EQ(apply(Test), Expected); +} + TEST_F(DefineInlineTest, TransformInlineNamespaces) { auto Test = R"cpp( namespace a { inline namespace b { namespace { struct Foo{}; } } } @@ -1536,7 +1600,7 @@ void fo^o() { return ; })cpp", "fail: Couldn't find semicolon for target declaration."}, }; - for(const auto& Case: Cases) + for (const auto &Case : Cases) EXPECT_EQ(apply(Case.first), Case.second) << Case.first; } @@ -1594,7 +1658,7 @@ void TARGET(){ return; } )cpp"}, }; - for(const auto& Case: Cases) + for (const auto &Case : Cases) EXPECT_EQ(apply(Case.first), Case.second) << Case.first; }