[PATCH] D59407: [clangd] Add RelationSlab

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/index/Index.h:59
+    return sizeof(*this) + Arena.getTotalMemory() +
+           sizeof(value_type) * Relations.size();
+  }
----------------
nridge wrote:
> 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?
I'd say yes -- in a separate patch though.  Thanks for catching it!


================
Comment at: clang-tools-extra/clangd/index/Relation.h:1
+//===--- Ref.h ---------------------------------------------------*- C++-*-===//
+//
----------------
"--- Relation.h --------------------"


================
Comment at: clang-tools-extra/clangd/index/Relation.h:41
+public:
+  // Key.Symbol is Key.Kind of every symbol in Value.
+  // For example, if Key.Kind == SymbolRole::RelationChildOf,
----------------
Three slashes for doc comments, please.


================
Comment at: clang-tools-extra/clangd/index/Relation.h:45
+  // in Value are the base classes of Key.Symbol).
+  struct Relation {
+    RelationKey Key;
----------------
Lift it up into the `clang::clangd` namespace?  (like `Symbol` and `Ref`)


================
Comment at: clang-tools-extra/clangd/index/Relation.h:68
+
+  // RelationSlab::Builder is a mutable container that can 'freeze' to
+  // RelationSlab.
----------------
No need to repeat the type name being documented.  "A mutable container that can ..."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407