[PATCH] D47935: [clangd] Boost completion score according to file proximity.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Oops, couple more comments.
But the big things I think are:

- what's the performance impact of doing all this work (including the URI stuff) inside the scoring loop?
- what's the most useful formula for the proximity score



================
Comment at: clangd/Quality.cpp:171
+/// "uri:///a/b/c" will be treated as /a/b/c
+static float uriProximity(StringRef UX, StringRef UY) {
+  auto SchemeSplitX = UX.split(':');
----------------
This doesn't look quite right to me.
We can tune the details later, but in practice this seems like it's *very* hard to get zero proximity, which is our neutral score - you need to be 18 directories away?

FWIW, fozzie appears to give an additive boost proportional to 5-up, where up is the number of directories from the context you have to traverse up from the context to get to a parent of the symbol. (There's no penalty for down-traversals probably for implementation reasons, this should be smaller than the up-traversal penalty I think)


================
Comment at: clangd/Quality.h:74
+  // If set, this is used to compute proximity from symbol's declaring file.
+  llvm::StringRef ProximityPath;
   /// Proximity between best declaration and the query. [0-1], 1 is closest.
----------------
sammccall wrote:
> It seems OK to have ProximityPath or ProximityScore, but we shouldn't have both: drop proximityscore and calculate it during evaluate()?
what's the plan for associated-header? should this be a smallvector<2>?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47935