[PATCH] D42073: [clangd] Query all visible scopes based on all visible using-namespace declarationsand containing namespace for global qualified code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sounds like implementation is in flux, but a few comments that might be relevant.



================
Comment at: clangd/CodeComplete.cpp:264
+/// (e.g. "ns::ab?").
+struct QueryScopes {
+  /// All scopes being queried in indexes. Each scope should meet the
----------------
this could also be just a using, rather than a struct - do you see more members here?


================
Comment at: clangd/CodeComplete.cpp:270
+  /// namespace scopes which are visible to the qualified-id completion token.
+  std::vector<std::string> Scopes;
+};
----------------
Just to check, if the user types:
"vec" --> None
"::vec" --> {""}
"std::vec" --> {"std"}
"using namespace std; vec" --> None
"using namespace std; ::vec" --> {"", "std"}

is this right?


================
Comment at: clangd/CodeComplete.cpp:300
   /// This is set if the completion is for qualified IDs, e.g. "abc::x^".
-  llvm::Optional<SpecifiedScope> SSInfo;
+  llvm::Optional<QueryScopes> QScopes;
 };
----------------
Probably not for this patch, but I just realized...

Currently if the completion is not qualified, we don't track what namespaces are in scope.
This is really relevant for ranking though.
e.g. "using namespace std; namespace foo { v^ }"
The index should apply namespace-based bonuses: foo::value > std::vector > boost::variant.
At the moment we don't provide enough information to do this. But this optional<QueryScopes> probably wants to be a QueryScopes + bool Qualified, or two QueryScopes (one for restrictions, and one for scoring)

So I'd suggest either here or in queryscopes:
    // TODO: for unqualified completions, capture scopes and use for scoring.


================
Comment at: clangd/CodeComplete.cpp:658
+    VisibleNamespaceFinder Finder;
+    S.LookupVisibleDecls(QCC.ClosestScope,
+                         clang::Sema::LookupNameKind::LookupOrdinaryName,
----------------
you need to set loadexternal to false to avoid deserializing the whole preamble.
Ilya says this is going to get merged with lookup during code-completion, which would be nice :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42073