[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
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))
----------------
sammccall wrote:
> this looks like a behavior change - why?
This is a behavior change. Instead of choosing between `Match/Miss` in the last position, we enumerate the last matching position in `Word`.

This saves `if (P < PatN - 1) {` check in the main loop at the cost of a for loop here (use sites of ending values)


================
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);
----------------
sammccall wrote:
> adding the penalty unconditionally seems like a behavior change, why?
Because now we use a different method to calculate the final value. I believe this makes the loop simpler.

This was not regular because 

Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score + missScore(W, Miss),
                              Miss};

This unconditionally added a trailing penalty but the main loop did not.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720