[PATCH] D75429: [clangd] DefineOutline removes `override` specified from overridden methods.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.
njames93 edited the summary of this revision.
njames93 added a project: clang-tools-extra.

The define out of line refactor tool previously would copy the override specifier into the out of line definition. 
This results in malformed code as the specifiers aren't allowed outside the class definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75429

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
@@ -2068,6 +2068,24 @@
               };)cpp",
           "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
       },
+      // Overridden Methods
+      {
+          R"cpp(
+            struct A {
+              virtual void foo() = 0;
+            };
+            struct B : A {
+              void fo^o() override {}
+            };)cpp",
+          R"cpp(
+            struct A {
+              virtual void foo() = 0;
+            };
+            struct B : A {
+              void foo() override ;
+            };)cpp",
+          "void B::foo()  {}\n",
+      },
   };
   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
@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -211,6 +212,18 @@
     }
   }
 
+  // Removes the override specifier if it exists as it doesn't belong in the
+  // function out-of-line definition.
+  if (FD->hasAttr<OverrideAttr>()) {
+    const auto *Override = FD->getAttr<OverrideAttr>();
+    assert(Override);
+    CharSourceRange DelRange =
+        CharSourceRange::getTokenRange(Override->getLocation());
+    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: D75429.247523.patch
Type: text/x-patch
Size: 2031 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200301/41a1f9d7/attachment.bin>