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`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41506
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
