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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
