================
@@ -94,6 +106,9 @@ class MemIndex : public SymbolIndex {
static_assert(sizeof(RelationKind) == sizeof(uint8_t),
"RelationKind should be of same size as a uint8_t");
llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>>
Relations;
+ // Reverse relations, currently only for OverridenBy
+ llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>>
----------------
timon-ul wrote:
I am a bit torn, I absolutely get that saving some memory here is a good idea,
but also it makes me question a bit that all the changes have been kept
generic. What I mean is:
- `reverseRelations` is a generic name, even though it will be very specific
for `OverriddenBy`
- `reverseRelations` gets a RelationsRequest, but again, we only accept
`OverriddenBy` anyway
- easily extendible
Now of course there is a difference, these things are exposed to the outside
while the map itself is an implementation detail, but I think the idea of
removing the RelationKind also kinda puts in question why we keep
`reverseRelations` generic. We could instead also just create a map called
something like `OverriddenByMap`, call the function something related too and
be very specific what we are doing here. It kinda comes down to if we expect
this to be expanded in the future or not.
Now let me finish this by saying I am not against this change, I just have some
mixed feelings and am myself unsure, these are just some thoughts that popped
into my mind.
https://github.com/llvm/llvm-project/pull/163024
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
