[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.


================
Comment at: clangd/index/Background.cpp:194
-  // FIXME: partition the symbols by file rather than TU, to avoid duplication.
-  IndexedSymbols.update(AbsolutePath,
-                        llvm::make_unique<SymbolSlab>(std::move(Symbols)),
----------------
um, this (before your patch) doesn't look even slightly threadsafe. Sorry I didn't catch it in code review.

Now I'm not sure whether it makes sense for you to fix it or not. It seems fine to use two independent locks here, to me - we don't need actual consistency.


================
Comment at: unittests/clangd/DexTests.cpp:494
 
-TEST(DexTest, DexDeduplicate) {
-  std::vector<Symbol> Symbols = {symbol("1"), symbol("2"), symbol("3"),
----------------
at this point we should remove the corresponding test for MemIndex (in IndexTests.cpp I think) too


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433