From 0cfa6e2092846f11a1534af4c928df3c61d73eb0 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 14 Oct 2024 11:00:02 +0200 Subject: [PATCH] [clangd] Let DefineOutline tweak handle member functions (#95235) ... of class templates. --- clang-tools-extra/clangd/AST.cpp | 7 ++- .../clangd/refactor/tweaks/DefineOutline.cpp | 50 +++++++++++++++-- .../unittests/tweaks/DefineOutlineTests.cpp | 55 ++++++++++++++----- clang-tools-extra/docs/ReleaseNotes.rst | 2 + 4 files changed, 94 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 333fc10f17d7b4..f3eee1c6335f98 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -144,8 +144,13 @@ getQualification(ASTContext &Context, const DeclContext *DestContext, // since we stored inner-most parent first. std::string Result; llvm::raw_string_ostream OS(Result); - for (const auto *Parent : llvm::reverse(Parents)) + for (const auto *Parent : llvm::reverse(Parents)) { + if (Parent != *Parents.rbegin() && Parent->isDependent() && + Parent->getAsRecordDecl() && + Parent->getAsRecordDecl()->getDescribedClassTemplate()) + OS << "template "; Parent->print(OS, Context.getPrintingPolicy()); + } return OS.str(); } diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp index f43f2417df8fce..591a8b245260ea 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -128,7 +128,28 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD, SM.getBufferData(SM.getMainFileID()), Replacements); if (!QualifiedFunc) return QualifiedFunc.takeError(); - return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1); + + std::string TemplatePrefix; + if (auto *MD = llvm::dyn_cast(FD)) { + for (const CXXRecordDecl *Parent = MD->getParent(); Parent; + Parent = + llvm::dyn_cast_or_null(Parent->getParent())) { + if (const TemplateParameterList *Params = + Parent->getDescribedTemplateParams()) { + std::string S; + llvm::raw_string_ostream Stream(S); + Params->print(Stream, FD->getASTContext()); + if (!S.empty()) + *S.rbegin() = '\n'; // Replace space with newline + TemplatePrefix.insert(0, S); + } + } + } + + auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1); + if (!TemplatePrefix.empty()) + Source.insert(0, TemplatePrefix); + return Source; } // Returns replacements to delete tokens with kind `Kind` in the range @@ -212,9 +233,13 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext, } } const NamedDecl *ND = Ref.Targets.front(); - const std::string Qualifier = + std::string Qualifier = getQualification(AST, TargetContext, SM.getLocForStartOfFile(SM.getMainFileID()), ND); + if (ND->getDeclContext()->isDependentContext() && + llvm::isa(ND)) { + Qualifier.insert(0, "typename "); + } if (auto Err = DeclarationCleanups.add( tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) Errors = llvm::joinErrors(std::move(Errors), std::move(Err)); @@ -407,10 +432,23 @@ class DefineOutline : public Tweak { return !SameFile; } - // Bail out in templated classes, as it is hard to spell the class name, - // i.e if the template parameter is unnamed. - if (MD->getParent()->isTemplated()) - return false; + for (const CXXRecordDecl *Parent = MD->getParent(); Parent; + Parent = + llvm::dyn_cast_or_null(Parent->getParent())) { + if (const TemplateParameterList *Params = + Parent->getDescribedTemplateParams()) { + + // Class template member functions must be defined in the + // same file. + SameFile = true; + + // Bail out if the template parameter is unnamed. + for (NamedDecl *P : *Params) { + if (!P->getIdentifier()) + return false; + } + } + } // The refactoring is meaningless for unnamed classes and namespaces, // unless we're outlining in the same file diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp index 906ff33db87344..6a9e90c3bfa70f 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp @@ -105,8 +105,8 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) { F^oo(const Foo&) = delete; };)cpp"); - // Not available within templated classes, as it is hard to spell class name - // out-of-line in such cases. + // Not available within templated classes with unnamed parameters, as it is + // hard to spell class name out-of-line in such cases. EXPECT_UNAVAILABLE(R"cpp( template struct Foo { void fo^o(){} }; )cpp"); @@ -154,7 +154,6 @@ TEST_F(DefineOutlineTest, FailsWithoutSource) { } TEST_F(DefineOutlineTest, ApplyTest) { - llvm::StringMap EditedFiles; ExtraFiles["Test.cpp"] = ""; FileName = "Test.hpp"; @@ -229,17 +228,18 @@ TEST_F(DefineOutlineTest, ApplyTest) { // Ctor initializer with attribute. { R"cpp( - class Foo { - F^oo(int z) __attribute__((weak)) : bar(2){} + template class Foo { + F^oo(T z) __attribute__((weak)) : bar(2){} int bar; };)cpp", R"cpp( - class Foo { - Foo(int z) __attribute__((weak)) ; + template class Foo { + Foo(T z) __attribute__((weak)) ; int bar; - };)cpp", - "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n", - }, + };template +Foo::Foo(T z) __attribute__((weak)) : bar(2){} +)cpp", + ""}, // Virt specifiers. { R"cpp( @@ -369,7 +369,31 @@ TEST_F(DefineOutlineTest, ApplyTest) { };)cpp", " void A::foo(int) {}\n", }, - // Destrctors + // Complex class template + { + R"cpp( + template struct O1 { + template struct O2 { + enum E { E1, E2 }; + struct I { + E f^oo(T, U..., V, E) { return E1; } + }; + }; + };)cpp", + R"cpp( + template struct O1 { + template struct O2 { + enum E { E1, E2 }; + struct I { + E foo(T, U..., V, E) ; + }; + }; + };template +template +typename O1::template O2::E O1::template O2::I::foo(T, U..., V, E) { return E1; } +)cpp", + ""}, + // Destructors { "class A { ~A^(){} };", "class A { ~A(); };", @@ -378,9 +402,14 @@ TEST_F(DefineOutlineTest, ApplyTest) { }; for (const auto &Case : Cases) { SCOPED_TRACE(Case.Test); + llvm::StringMap EditedFiles; EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader); - EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( - testPath("Test.cpp"), Case.ExpectedSource))); + if (Case.ExpectedSource.empty()) { + EXPECT_TRUE(EditedFiles.empty()); + } else { + EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( + testPath("Test.cpp"), Case.ExpectedSource))); + } } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8655331dfc00e4..39b600f361e8c5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -88,6 +88,8 @@ Objective-C Miscellaneous ^^^^^^^^^^^^^ +- The DefineOutline tweak now handles member functions of class templates. + Improvements to clang-doc -------------------------