[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.
Eric Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 21 01:20:46 PDT 2018
ioeric added inline comments.
================
Comment at: clangd/Cancellation.h:96
+/// checks using it to avoid extra lookups in the Context.
+class CancellationToken {
+public:
----------------
ilya-biryukov wrote:
> ioeric wrote:
> > As chatted offline, I have questions about the motivation of the `CancellationToken` class. Intuitively, it seems to me that this can be merged into `TaskHandle`, and we can simply stash the `TaskHandle` instead of a token into the context. There would be fewer states to maintain, and the design would be a bit more straightforward. I might be missing context/concerns here, so I'd like to hear what you and Ilya think. @ilya-biryukov
> I am for splitting cancellation checks from cancellation triggers.
> The processing code only needs to check if it was cancelled and exposing the `cancel()` does not add any useful functionality, merely ways to misuse it.
Couldn't we prevent this by passing only const handle references to users who are not expect to call `cancel()`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50502
More information about the cfe-commits
mailing list