[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

Thu Aug 6 08:24:32 PDT 2020

hokein added a comment.

In D84811#2199829 <https://reviews.llvm.org/D84811#2199829>, @kbobyrev wrote:

> In D84811#2199820 <https://reviews.llvm.org/D84811#2199820>, @hokein wrote:
>
>> In D84811#2199510 <https://reviews.llvm.org/D84811#2199510>, @kbobyrev wrote:
>>
>>> Even despite `FileFilter` not being fully implemented yet (an issue for a separate patch) I think this change should still be correct and is probably OK to land, WDYT @hokein?
>>
>> Yes, personally no objection on landing this, but landing it doesn't seem to help much for your remote-index case (STL symbols are not filtered out)?
>
> True, it doesn't filter _all_ of them, but it partially filters some symbols which is still a win I guess :) My rationale is probably that having this blocked on more implementation details would be more reasonable if there was no implementation at all but since it still filters out some symbols this should probably be fine.

It just reduces symptoms, not solving the problem. I think remote-index is also blocked the FileFiltering stuff (we can't launch remote-index without fixing this issue entirely).

As you may have noticed implementing FileFiltering is tricky, I think the indexer here is a good opportunity to test/verify your implementation (comparing the index data before vs after), so my preference would be to implement the FileFilter, test it with this patch together, then finally land this patch after making sure everything works.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84811/new/

https://reviews.llvm.org/D84811