[PATCH] D71188: [clangd] Delete ctor initializers while moving functions out-of-line

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:269
+      }
+      DeletionRange.setBegin(InitStart);
+    }
----------------
kadircet wrote:
> hokein wrote:
> > There is an alternative here, I think. The end loc of `FunctionProtoTypeLoc` should point the location after the `)`, e.g. the `Foo()^ : `,  would it be easier to use it as as the `InitStart`?
> > 
> > it can be retrieved by `CD->getTypeSourceInfo()->getTypeLoc().getEndLoc()`.
> yes, that would be great if attributes didn't exist. e.g.:
> 
> ```cpp
> class Foo {
>   Foo() __attribute__((weak)) : z(2) {}
>   int z;
> };
> ```
> 
> for the example given above, typeloc.endloc would not include trailing attributes, updated test case according to that.(And added a fixme, since we should not propagate those attributes into definition)
> 
> also the approach proposed in this patch would result in minimal changes, by keeping anything before `:` in declaration site.
> if we were to make use of endloc we could end up deleting more tokens in between, even if it would be rare to have tokens in between.
ah, I see, that makes sense. 


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:271
+      // Look for the first colon.
+      for (auto &Tok : llvm::reverse(Toks)) {
+        if (Tok.kind() == tok::colon) {
----------------
nit: the same, `llvm::find_if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71188