hokein added inline comments.
================
Comment at: clangd/CodeComplete.cpp:270
+ /// namespace scopes which are visible to the qualified-id completion token.
+ std::vector<std::string> Scopes;
+};
----------------
ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > 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?
> > > I think the idea was to have (I only highlight the differences):
> > > "vec" --> {""}
> > > "using namespace std; vec" --> {"", "std"}
> > >
> > > @hokein , or am I getting it wrong?
> > You're probably right, just want to be sure we're talking about the same
> > thing.
> >
> > There's two layers here: the context detected from sema, and what we're
> > going to send the index. The layering is more relevant now that more of
> > this moves out of clangd.
> >
> > for "vec", we should be sending {""} to the index for now, and later move
> > towards doing global completion.
> > But if possible the detection code should report None, and the query-index
> > code should translate it - there's no reason the detection code needs to be
> > wrong just because clangd can't do qualifier insertion/smart ranking/etc.
> >
> > That said, per our discussion this morning, the detected state shouldn't
> > really be Optional<vector<string>>, but rather struct { vector<string>
> > AccessibleScope, optional<string> UnresolvedQualifier } or something like
> > that...
> - Totally agree, this patch only fills the `vector<string> AccessibleScope`
> field, and `optional<string> UnresolvedQualifier` could be added in the
> follow-up patch when it's actually handled.
> - It still makes sense to have `optional<QueryScopes>` to distinguish cases
> when clang did not run the lookup during completion and the scopes were not
> set (i.e. completion inside macros, etc.)
@ilya-biryukov your are right.
Based on our discussion last morning, the accessible scopes:
// qualified completion
// "::vec" => {"::"}
// "using namespace std; ::vec^" => {"::", "std"}
// "namespace ns {using namespace std;} ns::^" => {"ns", "std"}
// "std::vec^" => {"::"} // unresolved "std::"
//
// unqualified completion
// "vec^" => {"::"}
// "using namespace std; vec^" => {"::", "std"}
// "using namespace std; namespace ns { vec^ }" => {"ns", "std", "::"}
With rL322945, now it is sensible to include all (qualified/unqualified
completion) in this patch.
================
Comment at: clangd/CodeComplete.cpp:658
+ VisibleNamespaceFinder Finder;
+ S.LookupVisibleDecls(QCC.ClosestScope,
+ clang::Sema::LookupNameKind::LookupOrdinaryName,
----------------
sammccall wrote:
> 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 :-)
not needed now as we don't do lookup manually in clangd.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42073
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
