sammccall added a comment.
Can you elaborate on what this patch is improving, and how?
There are some stylistic changes, and also what look like subtle logic changes,
and some rearrangement of the algorithm - to what end?
Canonical way to run all tests is `ninja check-clang-tools`, the way you
suggested is the right thing for rapid iteration with unit tests.
Personally, I don't use a debugger.
================
Comment at: clangd/FuzzyMatch.cpp:93
+ for (int I = PatN; I <= WordN; I++)
+ Best = std::max(Best, Scores[PatN][I][Match].Score);
if (isAwful(Best))
----------------
this looks like a behavior change - why?
================
Comment at: clangd/FuzzyMatch.cpp:241
- auto MatchMissScore = PreMiss[Match].Score;
- auto MissMissScore = PreMiss[Miss].Score;
- if (P < PatN - 1) { // Skipping trailing characters is always free.
- MatchMissScore -= skipPenalty(W, Match);
- MissMissScore -= skipPenalty(W, Miss);
- }
+ auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+ auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);
----------------
adding the penalty unconditionally seems like a behavior change, why?
================
Comment at: clangd/FuzzyMatch.h:61
bool allowMatch(int P, int W) const;
- int skipPenalty(int W, Action Last) const;
- int matchBonus(int P, int W, Action Last) const;
+ int missScore(int W, Action Last) const;
+ int matchScore(int P, int W, Action Last) const;
----------------
FWIW, I don't think this is an improvement - I think the clarity of purpose in
names is more useful than having consistent signs in this case.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44720
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
