[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 1 04:04:36 PDT 2018
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
More information about the cfe-commits
mailing list