[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

- It would be useful for the C++ API (CodeCompletion struct) to provide the includes/edits in ranked order, if possible. Frontends could experiment with showing all the options.
- Still not sure that adding more complicated behavior to Code Complete (vs just improving ranking) is the right thing for now (bigger comment below)



================
Comment at: clangd/CodeComplete.cpp:303
+
+    // Pick the most popular include, only if it has way more references than
+    // the rest of includes; otherwise, we just give up and don't insert
----------------
I'm not sure about this:
 - we're adding new user-visible behavior which is potentially confusing (not inserting even though insertion is turned on and we're offering a completion that requires insertion)
 - it's not clear that first > 10 * second is the right heuristic or tuning
 - this probably isn't the right place for this heuristic - e.g. it causes us not to show the location of the symbol in the detail field. (IIRC, the fact that this is only shown when header insertion is active is a limitation we wanted to fix, not furither entrench).

Can we start with the simplest behavior, closest to what we do today: just choose the most popular include?

Probably the most forward-looking way to do this is to pick the winner when we construct the CompletionCandidate and stash it in a field there. (The scoring function is going to become more complex as we add proximity, and this function gets called twice. Also this function won't have access to the info needed for proximity).


================
Comment at: clangd/CodeComplete.cpp:316
+              });
+    if (Includes.begin()->References <
+        10 * std::next(Includes.begin())->References)
----------------
nit: Includes[0] and Includes[1]?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291