llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-tools-extra Author: Christian Kandeler (ckandeler) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/112345.diff 2 Files Affected: - (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+50-19) - (modified) clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp (+44-7) ``````````diff diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp index 591a8b245260ea..14d9858b702a1d 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -109,7 +109,8 @@ findContextForNS(llvm::StringRef TargetNS, const DeclContext *CurContext) { // afterwards it can be shared with define-inline code action. llvm::Expected<std::string> getFunctionSourceAfterReplacements(const FunctionDecl *FD, - const tooling::Replacements &Replacements) { + const tooling::Replacements &Replacements, + bool TargetFileIsHeader) { const auto &SM = FD->getASTContext().getSourceManager(); auto OrigFuncRange = toHalfOpenFileRange( SM, FD->getASTContext().getLangOpts(), FD->getSourceRange()); @@ -130,23 +131,41 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD, return QualifiedFunc.takeError(); std::string TemplatePrefix; + auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) { + const TemplateParameterList *Params = D->getDescribedTemplateParams(); + if (!Params) + return; + for (Decl *P : *Params) { + if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(P)) { + TTP->removeDefaultArgument(); + } else if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P)) { + NTTP->removeDefaultArgument(); + } else if (auto * TTPD = dyn_cast<TemplateTemplateParmDecl>(P)) { + TTPD->removeDefaultArgument(); + } + } + std::string S; + llvm::raw_string_ostream Stream(S); + Params->print(Stream, FD->getASTContext()); + if (!S.empty()) + *S.rbegin() = '\n'; // Replace space with newline + if (append) + TemplatePrefix.append(S); + else + TemplatePrefix.insert(0, S); + }; if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(FD)) { for (const CXXRecordDecl *Parent = MD->getParent(); Parent; Parent = llvm::dyn_cast_or_null<const CXXRecordDecl>(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); - } + AddToTemplatePrefixIfApplicable(Parent, false); } } - + + AddToTemplatePrefixIfApplicable(FD, true); auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1); + if (TargetFileIsHeader) + Source.insert(0, "inline "); if (!TemplatePrefix.empty()) Source.insert(0, TemplatePrefix); return Source; @@ -202,7 +221,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind, llvm::Expected<std::string> getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext, const syntax::TokenBuffer &TokBuf, - const HeuristicResolver *Resolver) { + const HeuristicResolver *Resolver, + bool targetFileIsHeader) { auto &AST = FD->getASTContext(); auto &SM = AST.getSourceManager(); @@ -337,7 +357,7 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext, if (Errors) return std::move(Errors); - return getFunctionSourceAfterReplacements(FD, DeclarationCleanups); + return getFunctionSourceAfterReplacements(FD, DeclarationCleanups, targetFileIsHeader); } struct InsertionPoint { @@ -419,15 +439,15 @@ class DefineOutline : public Tweak { Source->isOutOfLine()) return false; - // Bail out if this is a function template or specialization, as their + // Bail out if this is a function template specialization, as their // definitions need to be visible in all including translation units. - if (Source->getDescribedFunctionTemplate()) - return false; if (Source->getTemplateSpecializationInfo()) return false; auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source); if (!MD) { + if (Source->getDescribedFunctionTemplate()) + return false; // Can't outline free-standing functions in the same file. return !SameFile; } @@ -443,13 +463,24 @@ class DefineOutline : public Tweak { SameFile = true; // Bail out if the template parameter is unnamed. + // FIXME: Is this really needed? It inhibits application on + // e.g. std::enable_if. for (NamedDecl *P : *Params) { if (!P->getIdentifier()) return false; } } } - + + // For function templates, the same limitations as for class templates apply. + if (const TemplateParameterList *Params = MD->getDescribedTemplateParams()) { + for (NamedDecl *P : *Params) { + if (!P->getIdentifier()) + return false; + } + SameFile = true; + } + // The refactoring is meaningless for unnamed classes and namespaces, // unless we're outlining in the same file for (const DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) { @@ -459,7 +490,7 @@ class DefineOutline : public Tweak { return false; } } - + // Note that we don't check whether an implementation file exists or not in // the prepare, since performing disk IO on each prepare request might be // expensive. @@ -485,7 +516,7 @@ class DefineOutline : public Tweak { auto FuncDef = getFunctionSourceCode( Source, InsertionPoint->EnclosingNamespace, Sel.AST->getTokens(), - Sel.AST->getHeuristicResolver()); + Sel.AST->getHeuristicResolver(), SameFile && isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts())); if (!FuncDef) return FuncDef.takeError(); diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp index 6a9e90c3bfa70f..7e2b863733477d 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp @@ -111,13 +111,17 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) { template <typename> struct Foo { void fo^o(){} }; )cpp"); - // Not available on function templates and specializations, as definition must - // be visible to all translation units. + // Not available on function template specializations and free function templates. EXPECT_UNAVAILABLE(R"cpp( - template <typename> void fo^o() {}; - template <> void fo^o<int>() {}; + template <typename T> void fo^o() {} + template <> void fo^o<int>() {} )cpp"); - + + // Not available on member function templates with unnamed template parameters. + EXPECT_UNAVAILABLE(R"cpp( + struct Foo { template <typename> void ba^r() {} }; + )cpp"); + // Not available on methods of unnamed classes. EXPECT_UNAVAILABLE(R"cpp( struct Foo { @@ -237,7 +241,7 @@ TEST_F(DefineOutlineTest, ApplyTest) { Foo(T z) __attribute__((weak)) ; int bar; };template <typename T> -Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){} +inline Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){} )cpp", ""}, // Virt specifiers. @@ -390,7 +394,7 @@ Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){} }; };template <typename T, typename ...U> template <class V, int A> -typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::foo(T, U..., V, E) { return E1; } +inline typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::foo(T, U..., V, E) { return E1; } )cpp", ""}, // Destructors @@ -399,6 +403,39 @@ typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::fo "class A { ~A(); };", "A::~A(){} ", }, + + // Member template + { + R"cpp( + struct Foo { + template <typename T, bool B = true> + void ^bar() {} + };)cpp", + R"cpp( + struct Foo { + template <typename T, bool B = true> + void bar() ; + };template <typename T, bool B> +inline void Foo::bar() {} +)cpp", + "" + }, + + // Class template with member template + { + R"cpp( + template <typename T> struct Foo { + template <typename U> void ^bar(const T& t, const U& u) {} + };)cpp", + R"cpp( + template <typename T> struct Foo { + template <typename U> void bar(const T& t, const U& u) ; + };template <typename T> +template <typename U> +inline void Foo<T>::bar(const T& t, const U& u) {} +)cpp", + "" + }, }; for (const auto &Case : Cases) { SCOPED_TRACE(Case.Test); `````````` </details> https://github.com/llvm/llvm-project/pull/112345 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
