arphaman added inline comments.
================
Comment at: clangd/ASTManager.cpp:69
+ // our one-element queue is empty.
+ if (!RequestIsPending && !Done)
+ ClangRequestCV.wait(Lock);
----------------
This is just a suggestion based on personal opinion: `RequestIsPending` and
`Done` can be merged into one enum value that uses an enum like this:
```
enum ASTManagerState {
Waiting, ReceivedRequest, Done
}
```
Then this condition could be rewritten as `if (state == Waiting)`.
================
Comment at: clangd/ASTManager.cpp:70
+ if (!RequestIsPending && !Done)
+ ClangRequestCV.wait(Lock);
+
----------------
I believe `std::condition_variable` may also be unblocked spuriously when
`wait` is called without a predicate, which would lead to an empty `File` down
below.
================
Comment at: clangd/ASTManager.h:67
+ /// Setting Done to true will make the worker thread terminate.
+ std::atomic<bool> Done;
+};
----------------
It looks like `Done` is always accessed in a scope where `RequestLock` is
locked, so `atomic` doesn't seem needed here.
https://reviews.llvm.org/D29886
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
