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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits