varjujan added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:51
+        {true, 40, 50}, // Substring.
+        {true, 50, 66}, // Levenshtein.
+        {true, 75, 85}, // Jaro-Winkler.
----------------
whisperity wrote:
> aaron.ballman wrote:
> > We should probably document where all these numbers come from, but `66` 
> > definitely jumps out at me as being a bit strange. :-D
> Unfortunately, I have absolutely no idea. All these values are percentages 
> between 0 and 100 (`-1` is just saying that //"This heuristic doesn't accept 
> percentages"//), and this is written in the documentation now. However, the 
> answer to //"**why** 66%?"//, unless @varjujan can say something, I think is 
> lost to history...
> 
> I'll read over his thesis once again, maybe I can find anything with regards 
> to this.
> 
> Either way, I've detailed from both the code and the thesis how the 
> percentages are meant. In some cases, the % is calculated as //"% of the 
> longer string's length"//. In the Leventhstein's case, it's actually inverted:
> 
> ```
> Dist = (1 - Dist / LongerLength) * 100;
> ```
> 
> So what this says is that if the current arg1-param1 arg2-param2 pairing has 
> less than the inverse of 50% (which is more than 50%) of the longer string's 
> edit distance, but the arg2-param1 and arg1-param2 (the suggested swapped 
> order) has more than the inverse of 66% (which is less than 33%), then the 
> swap will be suggested.
> 
> Originally these values were called `LowerBound` and `UpperBound`, 
> respectively, which was saying **even less** about what they mean...
Sadly, I think there isn't any scientific reason behind these numbers. They 
just looked ok after a couple of test runs. (Maybe they make sense for shorter 
arg names, like the 66 for 3 char long names.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20689

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

Reply via email to