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:
> MaskRay wrote:
> > sammccall wrote:
> > > MaskRay wrote:
> > > > 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)
> > > Ah, I see - the case where we match only part of the word is handled up 
> > > here now.
> > > (I think you mean this is not a behavior change? The result is the same 
> > > AFAICS)
> > > 
> > > That does make more sense, but it's pretty subtle.
> > > Can you add a comment like
> > >  `// The pattern doesn't have to match the whole word (but the whole 
> > > pattern must match).`
> > Added
> > ```
> >   // Find the optimal prefix of Word to match Pattern.
> > ```
> > 
> > I meant this is a behavior change but it makes the first row and the rest 
> > rows of the score table more consistent.
> That comment really doesn't capture what's significant about this line - it's 
> the *policy*, rather than the mechanism, that needs highlighting here.
> 
> (Re: behavior change - I *think* there's no inputs for which we produce a 
> different match result/score because of this patch, right?)
Added your comment.

The behavior change is regarding the values of `Scores` (there is no longer 
different interpretation for the last character of Pattern) how the final value 
is chosen (Scores[P][W][*] -> Scores[P][*][Match]) . There is no noticeable 
change in the user viewpoint.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to