[clangd] Remove inline Specifier for DefineOutline Tweak

`inline` specifiers should be removed from from the function declaration and
the newly-created implementation.

For example, take the following (working) code:
```cpp
// foo.hpp
struct A {
  inline void foo() { std::cout << "hello world\n" << std::flush; }
};

// foo.cpp
#include "foo.hpp"

// main.cpp
#include "foo.hpp"

int main() {
  A a;
  a.foo();
  return 0;
}

// compile: clang++ -std=c++20 main.cpp foo.cpp -o main
```

After applying the tweak:
```
// foo.hpp
struct A {
  inline void foo();
};

// foo.cpp
#include "foo.hpp"

inline void A::foo() { std::cout << "hello world\n" << std::flush; }

// main.cpp
#include "foo.hpp"

int main() {
  A a;
  a.foo();
  return 0;
}

// compile: clang++ -std=c++20 main.cpp foo.cpp -o main
```

We get a link error, as expected:
```
/usr/bin/ld: /tmp/main-4c5d99.o: in function `main':
main.cpp:(.text+0x14): undefined reference to `A::foo()'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```

This revision removes these specifiers from both the header and the source file. This was identified in Github issue llvm/llvm-project#61295.

Reviewed By: kadircet

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

diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index f883397..b84ae04 100644 (file)
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -131,6 +131,47 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
   return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
 }
 
+// Returns replacements to delete tokens with kind `Kind` in the range
+// `FromRange`. Removes matching instances of given token preceeding the
+// function defition.
+llvm::Expected<tooling::Replacements>
+deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind,
+                     SourceRange FromRange) {
+  tooling::Replacements DelKeywordCleanups;
+  llvm::Error Errors = llvm::Error::success();
+  bool FoundAny = false;
+  for (const auto &Tok : TokBuf.expandedTokens(FromRange)) {
+    if (Tok.kind() != Kind)
+      continue;
+    FoundAny = true;
+    auto Spelling = TokBuf.spelledForExpanded(llvm::ArrayRef(Tok));
+    if (!Spelling) {
+      Errors = llvm::joinErrors(
+          std::move(Errors),
+          error("define outline: couldn't remove `{0}` keyword.",
+                tok::getKeywordSpelling(Kind)));
+      break;
+    }
+    auto &SM = TokBuf.sourceManager();
+    CharSourceRange DelRange =
+        syntax::Token::range(SM, Spelling->front(), Spelling->back())
+            .toCharRange(SM);
+    if (auto Err =
+            DelKeywordCleanups.add(tooling::Replacement(SM, DelRange, "")))
+      Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+  if (!FoundAny) {
+    Errors = llvm::joinErrors(
+        std::move(Errors),
+        error("define outline: couldn't find `{0}` keyword to remove.",
+              tok::getKeywordSpelling(Kind)));
+  }
+
+  if (Errors)
+    return std::move(Errors);
+  return DelKeywordCleanups;
+}
+
 // 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, except defaulted parameter arguments, body and
@@ -251,34 +292,16 @@ getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
   DelAttr(FD->getAttr<FinalAttr>());
 
   auto DelKeyword = [&](tok::TokenKind Kind, SourceRange FromRange) {
-    bool FoundAny = false;
-    for (const auto &Tok : TokBuf.expandedTokens(FromRange)) {
-      if (Tok.kind() != Kind)
-        continue;
-      FoundAny = true;
-      auto Spelling = TokBuf.spelledForExpanded(llvm::ArrayRef(Tok));
-      if (!Spelling) {
-        Errors = llvm::joinErrors(
-            std::move(Errors),
-            error("define outline: couldn't remove `{0}` keyword.",
-                  tok::getKeywordSpelling(Kind)));
-        break;
-      }
-      CharSourceRange DelRange =
-          syntax::Token::range(SM, Spelling->front(), Spelling->back())
-              .toCharRange(SM);
-      if (auto Err =
-              DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
-        Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
-    }
-    if (!FoundAny) {
-      Errors = llvm::joinErrors(
-          std::move(Errors),
-          error("define outline: couldn't find `{0}` keyword to remove.",
-                tok::getKeywordSpelling(Kind)));
+    auto DelKeywords = deleteTokensWithKind(TokBuf, Kind, FromRange);
+    if (!DelKeywords) {
+      Errors = llvm::joinErrors(std::move(Errors), DelKeywords.takeError());
+      return;
     }
+    DeclarationCleanups = DeclarationCleanups.merge(*DelKeywords);
   };
 
+  if (FD->isInlineSpecified())
+    DelKeyword(tok::kw_inline, {FD->getBeginLoc(), FD->getLocation()});
   if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
     if (MD->isVirtualAsWritten())
       DelKeyword(tok::kw_virtual, {FD->getBeginLoc(), FD->getLocation()});
@@ -460,15 +483,23 @@ public:
     if (!Effect)
       return Effect.takeError();
 
-    // FIXME: We should also get rid of inline qualifier.
-    const tooling::Replacement DeleteFuncBody(
+    tooling::Replacements HeaderUpdates(tooling::Replacement(
         Sel.AST->getSourceManager(),
         CharSourceRange::getTokenRange(*toHalfOpenFileRange(
             SM, Sel.AST->getLangOpts(),
             getDeletionRange(Source, Sel.AST->getTokens()))),
-        ";");
-    auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(),
-                                     tooling::Replacements(DeleteFuncBody));
+        ";"));
+
+    if (Source->isInlineSpecified()) {
+      auto DelInline =
+          deleteTokensWithKind(Sel.AST->getTokens(), tok::kw_inline,
+                               {Source->getBeginLoc(), Source->getLocation()});
+      if (!DelInline)
+        return DelInline.takeError();
+      HeaderUpdates = HeaderUpdates.merge(*DelInline);
+    }
+
+    auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(), HeaderUpdates);
     if (!HeaderFE)
       return HeaderFE.takeError();
 

diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index fd6d4d7..576377a 100644 (file)
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -136,6 +136,12 @@ TEST_F(DefineOutlineTest, ApplyTest) {
           "void foo() ;",
           "void foo() { return; }",
       },
+      // Inline specifier.
+      {
+          "inline void fo^o() { return; }",
+          " void foo() ;",
+          " void foo() { return; }",
+      },
       // Default args.
       {
           "void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",
@@ -319,6 +325,17 @@ TEST_F(DefineOutlineTest, ApplyTest) {
             };)cpp",
           "  Foo::Foo(int) {}\n",
       },
+      {
+          R"cpp(
+            struct A {
+              inline void f^oo(int) {}
+            };)cpp",
+          R"cpp(
+            struct A {
+               void foo(int) ;
+            };)cpp",
+          " void A::foo(int) {}\n",
+      },
       // Destrctors
       {
           "class A { ~A^(){} };",