This is an archive of the discontinued LLVM Phabricator instance.

[include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.
ClosedPublic

Authored by sammccall on Mar 7 2017, 2:14 PM.

Details

Summary

Add fuzzy SymbolIndex, where identifier needn't match exactly.

The purpose for this is global autocomplete in clangd. The query will be a
partial identifier up to the cursor, and the results will be suggestions.

It's in include-fixer because:

  • it handles SymbolInfos, actually SymbolIndex is exactly the right interface
  • it's a good harness for lit testing the fuzzy YAML index
  • (Laziness: we can't unit test clangd until reorganizing with a tool/ dir)

Other questionable choices:

  • FuzzySymbolIndex, which just refines the contract of SymbolIndex. This is an interface to allow extension to large monorepos (*cough*)
  • an always-true safety check that Identifier == Name is removed from SymbolIndexManager, as it's not true for fuzzy matching
  • exposing -db=fuzzyYaml from include-fixer is not a very useful feature, and a non-orthogonal ui (fuzziness vs data source). -db=fixed is similar though.

Diff Detail

Repository
rL LLVM

Event Timeline

sammccall created this revision.Mar 7 2017, 2:14 PM
hokein added a subscriber: hokein.Mar 8 2017, 9:01 AM
hokein added inline comments.
include-fixer/FuzzySymbolIndex.cpp
138 ↗(On Diff #90939)

nit: llvm::make_unique.

include-fixer/FuzzySymbolIndex.h
54 ↗(On Diff #90939)

nit: an empty line before #endif and trailing // LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_FUZZY_SYMBOL_INDEX_H.

include-fixer/SymbolIndexManager.cpp
106 ↗(On Diff #90939)

Just want to verify: this judgement is not needed as it is guaranteed in Line 96 DB.get()->search(Names.back()) right?

156 ↗(On Diff #90939)

An off-topic catch: std::move should not work for const auto &, remove the const.

include-fixer/tool/ClangIncludeFixer.cpp
233 ↗(On Diff #90939)

I'd prefer to add a break; statement although this case is the last one.

sammccall updated this revision to Diff 91042.Mar 8 2017, 9:58 AM
sammccall marked 4 inline comments as done.

Address review comments.

Thanks for the comments!

include-fixer/SymbolIndexManager.cpp
106 ↗(On Diff #90939)

Yes. The existing non-fuzzy implementations all make this guarantee, and we decide that we'll accept results from fuzzy implementations now too.

hokein accepted this revision.Mar 13 2017, 2:19 AM

Looks good from my side.

This revision is now accepted and ready to land.Mar 13 2017, 2:19 AM

bkramer: ping

bkramer accepted this revision.Mar 13 2017, 9:00 AM

lg as a prototype.

This revision was automatically updated to reflect the committed changes.