sammccall added a comment. In D115243#3185393 <https://reviews.llvm.org/D115243#3185393>, @kadircet wrote:
> Yes and no. As mentioned SymbolSlabs are frozen once created and I totally > agree with the reasons there. But AFAICT in none of the indexes (apart from > monolithic FileIndex) we ever have a single SymbolSlab. > We always have multiple of those and periodically build a single slab out of > them, by copying/merging each symbol so that we can build a SymbolIndex on > top of the final merged SymbolSlab. I was suggesting to perform ultimate > Origin tweaking there (we can even respect the Origins of each individual > slabs too). Discussed this offline a bit. We could pass around pairs of (SymbolSlab, Origin) and make it the index's responsibility to set the origin on each symbol. But I don't think this communicates so clearly that the origin reflects where the symbol came from/passed through, rather than what's serving it. (Given that Symbol::Origin exists in the slabs too). > - Definitely less plumbing, we just let FileSymbol now what it should be > called during construction. Background/RemoteIndex already has this knowledge > hard coded. Origin for an index isn't monolithic. Even a slab can contain symbols that are the result of merging, but FileIndex will soon have symbols from stdlib index + preamble index too. Given this, I'm not sure it's much less plumbing - it's not just passing a constructor parameter to the index. > - In theory those indexes that operate on top of a multiple SymbolSlabs > should be inserting their own "origin" into the final merged slab as it > contains some extra logic, just like MergeIndex. This makes sense to me if it's useful, but I think this is *additional* to the origin reflecting the source of the slabs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115243/new/ https://reviews.llvm.org/D115243 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
