[PATCH] D79950: [clangd] Avoid wasteful data structures in RefSlab::Builder

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 4 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/index/Ref.cpp:11
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/StringSaver.h"
----------------
kbobyrev wrote:
> This defines `BumpPtrAllocator`, right? I think it should be included in `clang-tools-extra/clangd/index/Ref.h`, not here if the intent is to explicitly include used headers. Same for `StringSaver.h`, this source file does not use any additional variables of these types.
Yup, that's right, thanks.
(An intermediate version of the code used the overloaded new from allocator.h)


================
Comment at: clang-tools-extra/clangd/index/Ref.h:140
+      SymbolID Symbol;
+      Ref R;
+    };
----------------
kbobyrev wrote:
> nit: I know you like the short names, but with `R` I had to get back to `Entry` definition each time I saw it (I was thinking "wait, `R`? is there an `L` somewhere here?". Maybe it's just me, but one-letter fields seem scary. Maybe `Reference`?
Oops, I agree but forgot to do this, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79950