[PATCH] D41506: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 22 04:19:39 PST 2017

hokein added inline comments.

Comment at: clangd/index/Index.cpp:76
+SymbolSlab SymbolSlab::Builder::build() && {
+  Symbols = {Symbols.begin(), Symbols.end()}; // Force shrink-to-fit.
+  // Sort symbols so the slab can binary search over them.
 use `Symbols.shrink_to_fit()`?

Comment at: clangd/index/Index.cpp:81
+  // We may have unused strings from overwritten symbols. Build a new arena.
+  BumpPtrAllocator Arena;
+  DenseSet<StringRef> Strings;
Maybe rename it to `NewArena`, we already have a member called `Arena`.

Comment at: clangd/index/Index.h:174
+     std::vector<Symbol> Symbols;
+     llvm::DenseMap<SymbolID, size_t> SymbolIndex;
+  };
worth a comment saying the value of the map is the vector index of `Symbols`, it took me a while to understand.

Comment at: clangd/index/Index.h:182
+  llvm::BumpPtrAllocator Arena;
+  std::vector<Symbol> Symbols;
Mention the Symbols is required to be sorted.

Comment at: clangd/index/SymbolCollector.h:36
   // All Symbols collected from the AST.
-  SymbolSlab Symbols;
+  SymbolSlab::Builder Symbols;
Maybe name it `SymbolBuilder` to avoid confusion -- `Symbols` indicates its type is `SymbolSlab`. 

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list