[PATCH] D59407: [clangd] Add RelationSlab
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 25 08:51:19 PDT 2019
sammccall added a subscriber: hokein.
sammccall added a comment.
Hi Nathan, sorry for the stall here, and for repeatedly going over the same issues.
The design space here is pretty complicated.
I think the conclusion of recent offline discussions is:
- refs and relations can be the same thing-ish when seen through a certain lens.
- Unifying them probably isn't space-prohibitive *if* we implement callgraph: it's something like +15% to overall memory usage, and storing callgraph in another way is likely to be similar.
- however unifying the concepts seems likely to be awkward in practice. LSP features seem to want one or the other but not both - symbolID is generally better than location if it's precise enough, and useless if not. We're storing redundant information (the location for the method override ref is the same as the declaration of the related symbol). We risk tying ourselves in knots maintaining a model that doesn't map well onto our problems.
- In terms of storage size, relation-major (`map<SymbolRole, vector<pair<SymbolID, SymbolID>>>` or so) seems like a quick win. But it's a small enough one that we should try to live without it first.
So if you can stomach it, I think
> **Approach 2: Add a RelationSlab storing (subject, predicate, object) triples, intended for sparse relations***
is certainly fine with us (having spoken with @kadircet @ilya-biryukov @sammccall @gribozavr - @hokein is on vacation but not nearly as stubborn as I am!)
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits