[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 2 07:18:19 PST 2019
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.h:95
+ /// \p getBest, exposing for testing only.
+ static MatchType match(llvm::ArrayRef<Range> LHS, llvm::ArrayRef<Range> RHS);
+
----------------
Oops, forgot this...
I think the public API isn't quite right here - exposing parts for testing is fine, but the classification itself isn't fine grained enough I think. (Too easy to write a test that "passes" but the actual mapping found isn't the right one).
And the class structure wrapping a LangOpts ref seems like a detail that can be hidden.
I'd like to see:
- a function that returns the lexed ranges from a StringRef/LangOpts
- a function that constructs the mapping given two sequences of ranges (like `getMappedRanges(ArrayRef<Range>, ArrayRef<Range>) -> vector<Range>`
- a function that ties these together to the data structures we care about (i.e. taking Code + identifier + LangOpts + ArrayRef<Ref> or so)
then you can unit test the first and second and smoke test the third.
Tests like
```
Indexed = "int [[x]] = 0; void foo(int x);";
Draft = "double [[x]] = 0; void foo(double x);";
verifyRenameMatches(Indexed, Draft);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70594/new/
https://reviews.llvm.org/D70594
More information about the cfe-commits
mailing list