[PATCH] D36397: [clangd] Fixed a data race.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: clangd/ClangdUnit.cpp:714
 
-void CppFile::cancelRebuilds() {
+void CppFile::cancelRebuild() { deferCancelRebuild().get(); }
+
----------------
Somewhat orthogonal question - why CppFile? We do support other languages in Clangd, right? (C / Obj-C)
I'd have expected this to be called ClangdUnit, especially given the file name.


================
Comment at: clangd/ClangdUnit.cpp:735
+
+  std::shared_ptr<CppFile> That = shared_from_this();
+  return std::async(std::launch::deferred, [That, RequestRebuildCounter]() {
----------------
Now I know why I don't like the shard_ptr, this is all pretty awkward.


================
Comment at: clangd/ClangdUnit.cpp:738
+    std::unique_lock<std::mutex> Lock(That->Mutex);
+    CppFile *This = &*That;
+    This->RebuildCond.wait(Lock, [This, RequestRebuildCounter]() {
----------------
Why do we need that instead of just calling the bound shared_ptr This?


https://reviews.llvm.org/D36397