[PATCH] D59407: [clangd] Add RelationSlab

[Sam McCall via Phabricator via cfe-commits]

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!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59407/new/

https://reviews.llvm.org/D59407