[PATCH] D71187: [clangd] Delete default arguments while moving functions out-of-line

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Only function declarations should have the default arguments.

This patch makes sure we don't propogate those arguments to out-of-line
definitions.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71187

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


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1935,6 +1935,12 @@
             template <> void foo<int>() ;)cpp",
           "template <> void foo<int>() { return; }",
       },
+      // Default args
+      {
+            "void fo^o(int x, int y = 5, int = 2) {}",
+            "void foo(int x, int y = 5, int = 2) ;",
+            "void foo(int x, int y, int ) {}",
+      },
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -10,6 +10,7 @@
 #include "FindTarget.h"
 #include "HeaderSourceSwitch.h"
 #include "Logger.h"
+#include "ParsedAST.h"
 #include "Path.h"
 #include "Selection.h"
 #include "SourceCode.h"
@@ -23,6 +24,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Driver/Types.h"
 #include "clang/Format/Format.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
@@ -133,12 +135,13 @@
 
 // Creates a modified version of function definition that can be inserted at a
 // different location, qualifies return value and function name to achieve that.
-// Contains function signature, body and template parameters if applicable.
-// No need to qualify parameters, as they are looked up in the context
-// containing the function/method.
+// 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.
 llvm::Expected<std::string>
 getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace) {
-  auto &SM = FD->getASTContext().getSourceManager();
+  auto &AST = FD->getASTContext();
+  auto &SM = AST.getSourceManager();
   auto TargetContext = findContextForNS(TargetNamespace, FD->getDeclContext());
   if (!TargetContext)
     return llvm::createStringError(
@@ -169,14 +172,32 @@
       }
     }
     const NamedDecl *ND = Ref.Targets.front();
-    const std::string Qualifier =
-        getQualification(FD->getASTContext(), *TargetContext,
-                         SM.getLocForStartOfFile(SM.getMainFileID()), ND);
+    const std::string Qualifier = getQualification(
+        AST, *TargetContext, SM.getLocForStartOfFile(SM.getMainFileID()), ND);
     if (auto Err = QualifierInsertions.add(
             tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier)))
       Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
   });
 
+  // Get rid of default arguments, since they should not be specified in
+  // out-of-line definition.
+  for (const auto *PVD : FD->parameters()) {
+    if (PVD->hasDefaultArg()) {
+      auto DelRange = CharSourceRange::getTokenRange(PVD->getDefaultArgRange());
+      auto StartLoc = PVD->getLocation();
+      // If decl has a name, we want to keep it, so lex till end of token.
+      // Otherwise getLocation will be printing to '=', so we can directly use
+      // it.
+      if (!PVD->getDeclName().isEmpty())
+        StartLoc =
+            Lexer::getLocForEndOfToken(StartLoc, 0, SM, AST.getLangOpts());
+      DelRange.setBegin(StartLoc);
+      if (auto Err =
+              QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+        Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+    }
+  }
+
   if (Errors)
     return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, QualifierInsertions);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71187.232774.patch
Type: text/x-patch
Size: 3864 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191209/db434fde/attachment-0001.bin>