[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

Wed Oct 16 06:46:07 PDT 2019

kadircet added a comment.

In D68937#1710634 <https://reviews.llvm.org/D68937#1710634>, @ilya-biryukov wrote:

> An alternative approach I'm thinking of:
>  After D68977 <https://reviews.llvm.org/D68977> lands, we could try using `findExplicitReferences` to produce all of these edits:
>
> 1. we collect locations of all references and declaration of relevant parameters and template parameters.
> 2. find the ones where the name differs and produce the changes.
>
>   That would handle not only references in the default argument, but also all references in the body and should be less code overall. IMO it's worth exploring this approach right away, e.g. you could layer your patch on top of the current version of D68977 <https://reviews.llvm.org/D68977>

I totally agree that the solution you proposed would also work, but don't think it would be any less code. Since one needs to correlate
parameters between two different declarations, and `findExplicitReferences` doesn't really provide a nice way to achieve that. One
would have to rely on `SourceLocation` ordering or in the order callback was issued(which implicitly relies on AST traversal order).

It would be nice to unify the rewrite parameter name/type/defaultarg logics, but not sure if it is worth.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937