hokein added a comment. In https://reviews.llvm.org/D50385#1191914, @ioeric wrote:
> 2 high-level questions: > > 1. What's the reason for having a separate `SymbolOccurrenceSlab`? Could > store occurrences as extra payload of `Symbol`? Storing occurrences in `Symbol` structure is easy to misuse by users IMO -- if we go through this way, we will end up having a `getOccurrences`-like method in `Symbol` structure. Once users get the `Symbol` instance, it is natural for them to call `getOccurrences` to get all occurrences of the symbol. However this `getOccurrences` method doesn't do what users expected (just returning an incomplete set of results or empty). To query the symbol occurrences, we should always use index interface. Therefore, I think we should try to avoid these confusions in the design. > 2. Could we merge `SymbolOccurrenceCollector` into the existing > `SymbolCollector`? They look a lot alike. Having another index data consumer > seems like more overhead on the user side. The `SymbolOccurrenceCollector` has many responsibilities (collecting declaration, definition, code completion information etc), and the code is growing complex now. Merging the `SymbolOccurrenceCollector` to it will make it more complicated -- we will introduce more option flags like `collect-symbol-only`, `collect-occurrence-only` to configure it for our different use cases (we need to the implementation detail clearly in order to make a correct option for `SymbolCollector`). And I can foresee these two collectors might be run at different point (runWithPreamble vs runWithAST) in dynamic index. They might use same facilities, but we could always share them. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
