[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

High level comments based on offline discussion:

I think we want to define/formalize the concept of a near miss, to make precise the tradeoffs between false positives, false negatives, and implementability.
Not just at the individual level (an index occurrence vs a lexed occurrence) but at a whole-document level.
A possible definition, intuitively finding the locations where the indexed occurrences are now spelled:

- a near miss maps all of the **name** occurrences in the index onto a **subset** of the lexed occurrences. (names may refer to more than one thing)
- indexed occurrences must all be mapped. Result must be distinct, and preserve order. (Support only simple edits to ensure our mapping is robust)
- each indexed->lexed correspondence may change row or column but not both (increases chance our mapping is robust)

Then we can use a greedy algorithm to find a match (or memoized DFS to enumerate/compare all matches)

A good metric for comparing competing mappings might be the sum of the implied edit sizes between successive entries.
i.e. if the first three mappings are offset by 1 column down, and the last is offset by 2 columns down, then this implies an insertion of 1 line at the top of the file and 1 line between edits 3 and 4, for a total of 2.

subset is fine/good to handle as a special case - if the subset is small we don't want to find it by search.



================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:385
+  assert(std::is_sorted(Superset.begin(), Superset.end()));
+  auto ItSubset = Subset.begin();
+  auto ItSuperset = Superset.begin();
----------------
I think this is just 
return std::includes(Superset.begin, Superset.end(), Subset.begin(), Subset.end());
(probably clear enough to just inline to the callsite)


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:623
+  if (M == MatchType::Subset)
+    return std::move(IndexOccurrences); // RVO is disallowed for parameters.
+  if (M == MatchType::NearMiss)
----------------
That doesn't mean you need std::move to get a move, it just means you can't avoid calling the move constructor.
https://godbolt.org/z/Rh-CvT


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70594