hokein added a comment.
In D73088#1830960 <https://reviews.llvm.org/D73088#1830960>, @kbobyrev wrote:
> It would also be great to have the original rename test from the linked issue
> since something might potentially go wrong in-between
> `findExplicitReferences`.
Done.
> and rename action (and also to expand the testset because the existing one is
> not really extensive :().
yeap, there is some room to improve the test code here (e.g. using an idea
similar to `CodeContext` in `TweakTesting`), previously we put the test code to
a wrapper `foo` function, it is sufficient for most cases, but not for this
patch as defining a template class in a function body is forbidden in C++. I
think we can adress this afterwards.
================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:736
+ template <typename $0^T>
+ class $1^$2^Bar {
+ ~$3^Bar();
----------------
kadircet wrote:
> kadircet wrote:
> > looks like we are getting duplicates here
> looking at this again, this is actually not caused by this change, it was
> always there we just didn't notice it before since no one tested class
> templates.
>
> could you put a fixme on it?
Done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73088/new/
https://reviews.llvm.org/D73088
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
