[PATCH] D52276: [clangd] Add type boosting in code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added inline comments.


================
Comment at: clangd/Quality.h:98
+  /// Whether the item matches the type expected in the completion context.
+  bool TypeMatchesPreferred = false;
   /// FIXME: unify with index proximity score - signals should be
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > you've inserted in the middle of the file proximity stuff :-)
> > > Generally we'd put both context/symbol types as the signal here, rather than just whether they match, unless it's prohibitive. They'd get populated manually, and by merge() overloads, respectively.
> > I did it to avoid inefficiencies of:
> > 1. copying the context type for each completion item,
> > 2. copying the symbol type for each of the indexed items.
> > 
> > My understanding is that (1) can be avoided by storing a reference to a type, since it outlives the signals.
> > But we'll still have to do a copy for (2), right?
> > 
> > That's probably not a bottleneck anyway, but keeping it a bool flag for now and happy to change it  to your liking.
> Agree with your analysis.
> 
> I think our goal here should be to have a model with small # of per-symbol types, and small size of each type, so the per-symbol copy would be cheap. Sequencing is tricky - we don't have that optimization yet. Can we replay a session with some code completion and see if it shows up in a profiler?
> 
> (Perfect would be one type per symbol, types are 8-byte hashes. Unfortunately I think pointer->base conversions mean symbols must admit multiple tokens. But I have some ideas...)
After discussing offline, we ended up with a bunch of flags to indicate whether:
1. a symbol or a sema decl had a type,
2. a context had a type,
3. those types were the same.

Let me know if that look ok. The bit that I don't like is that they're computed ad-hoc outside the merge function, which is a bit ugly.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D52276