[clang-tools-extra] a209a80 - [clangd] Delete ctor initializers while moving functions out-of-line

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-09T18:52:57+01:00
New Revision: a209a8000e17ef3560598a44825747aab2f7914d

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

LOG: [clangd] Delete ctor initializers while moving functions out-of-line

Summary:
Currently we only delete function body from declaration, in addition to
that we should also drop ctor initializers.

Unfortunately CXXConstructorDecl doesn't store the location of `:` before
initializers, therefore we make use of token buffer to figure out where to start
deletion.

Fixes https://github.com/clangd/clangd/issues/220

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

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 4935aa005153..353da26a0f91 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/SourceLocation.h"
@@ -141,6 +142,7 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
 // Contains function signature, except defaulted parameter arguments, body and
 // template parameters if applicable. No need to qualify parameters, as they are
 // looked up in the context containing the function/method.
+// FIXME: Drop attributes in function signature.
 llvm::Expected<std::string>
 getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
                       const syntax::TokenBuffer &TokBuf) {
@@ -238,6 +240,45 @@ getInsertionPoint(llvm::StringRef Contents, llvm::StringRef QualifiedName,
   return InsertionPoint{Region.EnclosingNamespace, *Offset};
 }
 
+// Returns the range that should be deleted from declaration, which always
+// contains function body. In addition to that it might contain constructor
+// initializers.
+SourceRange getDeletionRange(const FunctionDecl *FD,
+                             const syntax::TokenBuffer &TokBuf) {
+  auto DeletionRange = FD->getBody()->getSourceRange();
+  if (auto *CD = llvm::dyn_cast<CXXConstructorDecl>(FD)) {
+    const auto &SM = TokBuf.sourceManager();
+    // AST doesn't contain the location for ":" in ctor initializers. Therefore
+    // we find it by finding the first ":" before the first ctor initializer.
+    SourceLocation InitStart;
+    // Find the first initializer.
+    for (const auto *CInit : CD->inits()) {
+      // We don't care about in-class initializers.
+      if (CInit->isInClassMemberInitializer())
+        continue;
+      if (InitStart.isInvalid() ||
+          SM.isBeforeInTranslationUnit(CInit->getSourceLocation(), InitStart))
+        InitStart = CInit->getSourceLocation();
+    }
+    if (InitStart.isValid()) {
+      auto Toks = TokBuf.expandedTokens(CD->getSourceRange());
+      // Drop any tokens after the initializer.
+      Toks = Toks.take_while([&TokBuf, &InitStart](const syntax::Token &Tok) {
+        return TokBuf.sourceManager().isBeforeInTranslationUnit(Tok.location(),
+                                                                InitStart);
+      });
+      // Look for the first colon.
+      auto Tok =
+          llvm::find_if(llvm::reverse(Toks), [](const syntax::Token &Tok) {
+            return Tok.kind() == tok::colon;
+          });
+      assert(Tok != Toks.rend());
+      DeletionRange.setBegin(Tok->location());
+    }
+  }
+  return DeletionRange;
+}
+
 /// Moves definition of a function/method to an appropriate implementation file.
 ///
 /// Before:
@@ -338,7 +379,8 @@ class DefineOutline : public Tweak {
     const tooling::Replacement DeleteFuncBody(
         Sel.AST.getSourceManager(),
         CharSourceRange::getTokenRange(*toHalfOpenFileRange(
-            SM, Sel.AST.getLangOpts(), Source->getBody()->getSourceRange())),
+            SM, Sel.AST.getLangOpts(),
+            getDeletionRange(Source, Sel.AST.getTokens()))),
         ";");
     auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(),
                                      tooling::Replacements(DeleteFuncBody));

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 0bc11a898d7e..d50d27afb1c8 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1979,6 +1979,24 @@ TEST_F(DefineOutlineTest, ApplyTest) {
           "void foo(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) ;",
           "void foo(int x, int y , int , int (*foo)(int) ) {}",
       },
+      // Ctor initializers.
+      {
+          R"cpp(
+              class Foo {
+                int y = 2;
+                F^oo(int z) __attribute__((weak)) : bar(2){}
+                int bar;
+                int z = 2;
+              };)cpp",
+          R"cpp(
+              class Foo {
+                int y = 2;
+                Foo(int z) __attribute__((weak)) ;
+                int bar;
+                int z = 2;
+              };)cpp",
+          "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
+      },
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Test);