[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clangd/CodeComplete.cpp:1251
+  // from any scope.
+  std::pair<std::vector<std::string>, bool> QueryScopes;
   // Include-insertion and proximity scoring rely on the include structure.
----------------
consider splitting out AllScopes as a separate variable, and assign `std::tie(QueryScopes, AllScopes) = getQueryScopes(...)`, for readability


================
Comment at: clangd/index/Index.h:432
+  /// wildcard.
+  /// FIXME: support assigning different weight to each scope.
   std::vector<std::string> Scopes;
----------------
ioeric wrote:
> sammccall wrote:
> > May not want a heavyweight API with explicit weights...
> > 
> > I think what we really **need** here is the ability to weight `clang::clangd::` > `clang::` > `` when you're in the scope of namespace clangd.
> > 
> > We could just say something like "the primary scope should come first" and do some FileDistance-like penalizing of the others...
> Changed the wording of `FIXME`.
> 
> > I think what we really need here is the ability to weight clang::clangd:: > clang:: > `` when you're in the scope of namespace clangd.
> It's unclear what this would mean for scopes from `using-namespace` directives. But we can revisit when we get there.
Yeah, my hypothesis is that it doesn't matter much, as long as the using-namespaces are ranked lower than the enclosing namespaces, and above "any" namespaces.


================
Comment at: clangd/index/dex/Dex.cpp:171
   }
+  if (Req.AnyScope)
+    ScopeIterators.push_back(createBoost(createTrue(Symbols.size()), 0.2));
----------------
kbobyrev wrote:
> Probably also check `!ScopeIterators.empty()`: otherwise the latency might increase for queries without any scope/any scope known to `Dex`.
if there are no other scopes, the "naive" query tree will end up looking like and(..., or(boost(true)).

The or() will already be optimized away today.
I'd suggest you just remove the boost() here if req.scopes is empty (or better: make the boost 1) and we make "generic optimizations" take care of eliminating boosts with value 1, and dropping true when it's an argument to and()


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52364