[PATCH] D59407: [clangd] Add RelationSlab
Nathan Ridge via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 17 12:21:40 PDT 2019
nridge marked 7 inline comments as done.
nridge added inline comments.
Comment at: clang-tools-extra/clangd/index/Index.h:43
+ using value_type = std::pair<RelationKey, llvm::ArrayRef<SymbolID>>;
+ using const_iterator = std::vector<value_type>::const_iterator;
> 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`
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();
> use capacity instead of size
Note, `RefSlab::bytes()` (which I where I copied this from) uses `size()` as well. Should I change that too?
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits