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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
