nridge marked an inline comment as done. nridge added inline comments.
================ Comment at: clang-tools-extra/clangd/XRefs.cpp:258 + + // For now, only consider exact name matches, including case. + // This is to avoid too many false positives. ---------------- sammccall wrote: > nridge wrote: > > sammccall wrote: > > > I wouldn't bother qualifying this as "for now". Any code is subject to > > > change in the future, but requiring an exact name match for index-based > > > results seems more like a design decision than a fixme. > > Do we want to rule out the possibility of handling typos in an identifier > > name in a comment (in cases where we have high confidence in the match, > > e.g. a long / unique name, small edit distance, only one potential match) > > in the future? > > > > This is also relevant to whether we want to keep the `FuzzyMatcher` or not. > No idea whether typo-correction is a good idea in principle - tradeoff > between current false negatives and false positives+compute. > > However neither FuzzyMatcher nor the existing index implementations > support/can easily support real typo correction, and it seems implausible to > me we'd add it for this feature. > > Compare to e.g: > - allowing case-insensitive match in some cases: `fooBar` vs `FooBar` is a > plausible "typo". This is easy to implement. > - correct the typo where we see the fixed version used as an identifier in > this file (and not the original). Excludes some cases, but drives > false-positives way down, and easy to implement. > > I don't think we need to rule things out, but I'm uncertain enough about the > approach to think that putting comments, fuzzymatcher etc here speculatively > isn't worth it. Perhaps I'm unclear on the distinction between fuzzy matching and typo correction. Are they not both a matter of comparing a candidate string against a test string, and considering it a match if the they are "close enough" according to some metric (with the metric potentially being a simple edit distance in the case of typo correction)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72874/new/ https://reviews.llvm.org/D72874 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
