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)), ---------------- kadircet wrote: > 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. Oops, you're right - I forgot `FileSymbols` was threadsafe. Then I think FileHash itself is indeed the only issue and this patch (DigestsMu) fixes it already. Carry on! 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
