kadircet 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)),
----------------
sammccall wrote:
> 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.
Ah, sorry for missing on my side as well. But IIUC, the only unsafe part is 
access to `FileHash` since `FileSymbols::update` is already thread-safe ?

If need be I can send out a patch to fix those issues.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to