https://github.com/ckandeler updated 
https://github.com/llvm/llvm-project/pull/112345

>From 42ea89f38b599796b15d50d078cba1cb0e0f2b02 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kande...@qt.io>
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
 templates

---
 .../clangd/refactor/tweaks/DefineOutline.cpp  | 68 ++++++++++++++-----
 .../unittests/tweaks/DefineOutlineTests.cpp   | 49 +++++++++++--
 2 files changed, 93 insertions(+), 24 deletions(-)

diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..56f3ccdf493419 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,14 +109,13 @@ 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());
   if (!OrigFuncRange)
     return error("Couldn't get range for function.");
-  assert(!FD->getDescribedFunctionTemplate() &&
-         "Define out-of-line doesn't apply to function templates.");
 
   // Get new begin and end positions for the qualified function definition.
   unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
@@ -130,23 +129,40 @@ 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 +218,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 +354,8 @@ 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 +437,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,6 +461,8 @@ 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;
@@ -450,6 +470,17 @@ class DefineOutline : public Tweak {
       }
     }
 
+    // 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()) {
@@ -485,7 +516,8 @@ 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..1af1bc31bf6486 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -111,11 +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.
@@ -237,7 +243,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 +396,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 +405,37 @@ 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);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to