[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:232
+    }
+    CharSourceRange DelRange = CharSourceRange::getTokenRange(
+        AttrTokens->front().location(), AttrTokens->back().location());
----------------
let's use `syntax::Token::range(SM, AttrTokens->front(), AttrTokens->back())...` instead


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:254
+      if (!Spelling) {
+        Errors =
+            llvm::joinErrors(std::move(Errors),
----------------
what about setting `Any` to true below this check and just breaking here instead? e.g.

```
bool Any = false;
for(...) {
  ....
  if (!Spelling) break;
  Any = true;
}
if (!Any) appendToErrors
```


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:262
+      }
+      assert(Spelling->size() == 1);
+      if (auto Err = DeclarationCleanups.add(tooling::Replacement(
----------------
why assert on this and not delete the whole Spelling that produced the virtual keyword ?

e.g.


```
#define STUPID_MACRO(X) virtual
struct F {
STUPID_MACRO(BLA BLA) void foo();
}
```




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75429/new/

https://reviews.llvm.org/D75429