ioeric added a comment.

In https://reviews.llvm.org/D40562#941570, @arphaman wrote:

> In https://reviews.llvm.org/D40562#940201, @ilya-biryukov wrote:
>
> > In https://reviews.llvm.org/D40562#939950, @arphaman wrote:
> >
> > > This change breaks cached completions for declarations in namespaces in 
> > > libclang. What exactly are you trying to achieve here? We could introduce 
> > > another flag maybe.
> >
> >
> > Am I right to assume this cache is there to reduce the amount of `Decl`s we 
> > need to deserialize from `Preamble`s? Maybe we could fix the cache to also 
> > include namespace-level `Decl`s? It should improve performance of the 
> > cached completions.
>
>
> I'm not actually 100% sure, but I would imagine that this one of the reasons, 
> yes. It would be nice to improve the cache to have things like 
> namespace-level `Decl`, although how will lookup work in that case? Btw, do 
> you think the cache can be reused in clangd as well?


I took a quick look at the completion cache and lookup code. I think the 
completion cache also assumes that top-level decls are only TU-level decls, and 
this assumption seems to be also built into the lookup code. At this point, I 
am inclined to add a separate completion option for what I want 
(`IgnoreDeclsInTUOrNamespaces`?). Regarding cache in clangd, I think it might 
be useful short-term, when we still use Sema's global code completion, but long 
term, we would use symbols from clangd's indexes, so the cache would not be 
useful anymore.


https://reviews.llvm.org/D40562



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to