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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm not entirely sure how to get the spelledTokens working in a good macro safe way?



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:220
+        assert(Attr->getLocation().isValid());
+        if (Attr->getLocation().isMacroID()) {
+          Errors = llvm::joinErrors(
----------------
kadircet wrote:
> can you add some test cases for this branch ? In theory it should be ok to drop this if full expansion is just the attr, i.e.
> 
> ```
> #define FNL final
> struct A { virtual void foo() FNL {} };
> ```
> 
> but should possibly fail in :
> ```
> #define MACRO foo() final
> struct A { virtual void MACRO {} };
> ```
it's not a great idea refactoring functions with MACRO attributes, we can never know if there are side effects based on different definitions due to things like build configs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75429