[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

[Sam McCall via cfe-commits]

On Tue, Sep 18, 2018, 16:28 Eric Liu via Phabricator <
reviews at reviews.llvm.org> wrote:

> ioeric added a comment.
>
> In https://reviews.llvm.org/D52078#1238301, @sammccall wrote:
>
> > Something not listed in cons: because macros aren't namespaced and we
> don't have lots of signals, they can be really spammy.
> >  Potentially, offering macros that aren't in the TU could be a loss even
> if it's a win for other types of signals.
>
>
> Aren't they already spammy from Sema? Sema can provide thousands of macros
> in the TU.
>
Indeed, but Sema will at least implicitly restrict to the transitive
headers!

Agree with everything you say, looking forward to this change!

We penalize quality of macro symbols in the global index. Maybe we can do
> the same thing for dynamic index?
>
> > We could always e.g. postfilter index macro results using the include
> structure of the preamble, so no concern for this patch, just something to
> think about for the followup.
>
> Sounds good.
>
> > We also need to make sure that we're not indexing/serving header guards
> as code completions (e.g. if SemaCodeComplete is currently taking care of
> this)
>
> These symbols are already filtered out in `SymbolCollector`.
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D52078
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180918/2b924994/attachment.html>