kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:496
+ std::condition_variable CV;
+ std::atomic<bool> ShouldStop = {false}; // Must notify CV after writing.
+ std::deque<CDBLookupResult> Queue;
----------------
nit: we already hold the lock within the loop while reading. it is nice that we
no longer need to acquire the lock during destructor, but is it worth the extra
mental load that comes with memory orderings? (also IIRC, default load/store
behaviors are already acquire-release)
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:541
+ });
+ Queue.push_back(std::move(Task));
+ }
----------------
don't we need some context propagation here? previously broadcasting was
running with the context of the astworker that discovered the cdb. i suppose it
is not needed at the moment, as most of the stuff we do in ::process isn't
context-aware, but I can't be sure if we don't have any users that make use of
some context-aware FS.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94606/new/
https://reviews.llvm.org/D94606
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
