nridge marked 7 inline comments as done. nridge added inline comments.
================ Comment at: clang-tools-extra/clangd/index/Index.h:43 +public: + using value_type = std::pair<RelationKey, llvm::ArrayRef<SymbolID>>; + using const_iterator = std::vector<value_type>::const_iterator; ---------------- kadircet wrote: > gribozavr wrote: > > `struct Relation`? And in the comments for it, please explain which way > > the relationship is directed (is the SymbolID in the key the subtype? or > > is the SymbolID in the ArrayRef the subtype?). > Ah exactly my thoughts, forget to mention this. > > I believe current usage is the counter-intuitive one. For example, we will > most likely query with something like: > `getRelations(SymbolID, baseOf)` to get all relations where `SymbolID` is > `baseOf` something else(which says get children of `SymbolID`) > > So that this valueType can be read like, > ``` > `SymbolID` is `RelationKind` every `SymbolID inside array` > ``` > WDYT? The way I was thinking of it is that `getRelations(SymbolID, baseOf)` would return all the bases of `SymbolID`. However, the opposite interpretation is also reasonable, we just need to pick one and document it. I'm happy to go with your suggested one. ================ Comment at: clang-tools-extra/clangd/index/Index.h:59 + return sizeof(*this) + Arena.getTotalMemory() + + sizeof(value_type) * Relations.size(); + } ---------------- kadircet wrote: > use capacity instead of size Note, `RefSlab::bytes()` (which I where I copied this from) uses `size()` as well. Should I change that too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59407/new/ https://reviews.llvm.org/D59407 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
