[clang-tools-extra] d3ac301 - [clangd] Disable define out-of-line code action on templates

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-08-05T20:57:28+02:00
New Revision: d3ac30188666b182daa87d23533543543b789e97

URL: https://github.com/llvm/llvm-project/commit/d3ac30188666b182daa87d23533543543b789e97
DIFF: https://github.com/llvm/llvm-project/commit/d3ac30188666b182daa87d23533543543b789e97.diff

LOG: [clangd] Disable define out-of-line code action on templates

Differential Revision: https://reviews.llvm.org/D85310

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
    clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index d37d37740799..9f68e248197e 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -122,9 +122,8 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
   if (!OrigFuncRange)
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "Couldn't get range for function.");
-  // Include template parameter list.
-  if (auto *FTD = FD->getDescribedFunctionTemplate())
-    OrigFuncRange->setBegin(FTD->getBeginLoc());
+  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());
@@ -387,6 +386,13 @@ class DefineOutline : public Tweak {
         Source->isOutOfLine())
       return false;
 
+    // Bail out if this is a function template or specialization, as their
+    // definitions need to be visible in all including translation units.
+    if (auto *PT = Source->getDescribedFunctionTemplate())
+      return false;
+    if (auto *TSI = Source->getTemplateSpecializationInfo())
+      return false;
+
     // Bail out in templated classes, as it is hard to spell the class name, i.e
     // if the template parameter is unnamed.
     if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) {

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 791965160055..200a53c690b6 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2008,6 +2008,13 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
   EXPECT_UNAVAILABLE(R"cpp(
     template <typename> struct Foo { void fo^o(){} };
     )cpp");
+
+  // Not available on function templates and specializations, as definition must
+  // be visible to all translation units.
+  EXPECT_UNAVAILABLE(R"cpp(
+    template <typename> void fo^o() {};
+    template <> void fo^o<int>() {};
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
@@ -2037,27 +2044,6 @@ TEST_F(DefineOutlineTest, ApplyTest) {
           "void foo() ;",
           "void foo() { return; }",
       },
-      // Templated function.
-      {
-          "template <typename T> void fo^o(T, T x) { return; }",
-          "template <typename T> void foo(T, T x) ;",
-          "template <typename T> void foo(T, T x) { return; }",
-      },
-      {
-          "template <typename> void fo^o() { return; }",
-          "template <typename> void foo() ;",
-          "template <typename> void foo() { return; }",
-      },
-      // Template specialization.
-      {
-          R"cpp(
-            template <typename> void foo();
-            template <> void fo^o<int>() { return; })cpp",
-          R"cpp(
-            template <typename> void foo();
-            template <> void foo<int>() ;)cpp",
-          "template <> void foo<int>() { return; }",
-      },
       // Default args.
       {
           "void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",