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

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/index/Index.h:430
   ///
-  /// The global scope is "", a top level scope is "foo::", etc.
+  /// The global scope is "", a top level scope is "foo::", etc. "*" is
+  /// wildcard.
----------------
sammccall wrote:
> I'm not a big fan of this magic value, it seems too easy to forget to handle.
> Especially since we have a lot of existing code that touches this interface, and we may forget to check some of it.
> 
> I suggest making this a separate boolean field `AnyScope`, with a comment that scopes explicitly listed will be ranked higher.
> We can probably also remove the "If this is empty" special case from `Scopes` now too, as the old behavior can be achieved by setting `Scopes = {}; AnyScope = true;`
sounds good.

> We can probably also remove the "If this is empty" special case from Scopes now too, as the old behavior can be achieved by setting Scopes = {}; AnyScope = true;
I think this is a good idea. Unfortunately, there seem to be many tests that rely on the old behavior. I'll add a FIXME and do it in followup patch.


================
Comment at: clangd/index/Index.h:432
+  /// wildcard.
+  /// FIXME: support assigning different weight to each scope.
   std::vector<std::string> Scopes;
----------------
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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52364