kbobyrev added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:591
// Details:
-// - lex the draft code to get all rename candidates, this yields a superset
-// of candidates.
+// - lex the draft code to get all rename candidates, this yields a set of
+// candidates. It may be a superset of candidates returned from the index
----------------
kadircet wrote:
> i believe lexing might yield a superset even if index is up-to-date,
>
> ```
> void foo() { int bar; }
> void baz() { int ba^r; }
>
> ```
Good point. Any local variable (IIRC we don't store local variables in index,
so it's not there) having the same identifier as the renamed symbol might cause
the indexed ranges to be a subset of lexed ranges.
However,
> i believe lexing might yield a superset even if index is up-to-date
> exactly for the same reason above, it might not be a subset even in those
> circumstances.
I tried to describe the case when lexer will return a _subset_ of indexed
results, which happens in practice. Is the suggestion to change the comments
you think are misleading?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:597
+// references to the renamed entity have been removed.
+// - if the lexed ranges are subset of index candidates, try to filter the
+// results from index by exactly matching existing ranges from lexer.
----------------
kadircet wrote:
> i believe we should do that even if it is not a subset.
Maybe my understanding of `getMappedRanges` is incorrect, but I suppose this
will happen in case indexed ranges are subset of lexed ranges in some form
(i.e. no need to find exact matches explicitly). Am I missing something?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71598/new/
https://reviews.llvm.org/D71598
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
