ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land.
LGTM with a few nits. And comment requests for things that were unclear when reading the code. ================ Comment at: clangd/FuzzyMatch.cpp:269 + : AwfulScore, + MissMatchScore = allowMatch(P, W, Miss) + ? PreMatch[Miss].Score + matchBonus(P, W, Miss) ---------------- NIT: maybe split into two declarations? ``` auto MatchMatchScore = ...; auto MissMatchScore = ....; ``` ================ Comment at: clangd/FuzzyMatch.h:56 // - GCC 4.8 complains not all values fit if the type is unsigned using Action = bool; constexpr static Action Miss = false, Match = true; ---------------- Maybe add a small doc comment for `Action`? Found myself figuring out what it is while reading this patch. ================ Comment at: clangd/FuzzyMatch.h:79 CharRole WordRole[MaxWord]; // Word segmentation info int WordTypeSet; // Bitmask of 1<<CharType bool WordContainsPattern; // Simple substring check ---------------- Maybe add a doc comment that this is the union of `CharType` of all chars in the word? Seems obvious now, but was also a bit confusing when reading the code. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47950 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
