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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
