[PATCH] D38583: [clangd] Added async API to run code completion.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: clangd/ClangdServer.cpp:225
+  // A task that will be run asynchronously.
+  PackagedTask Task([=]() mutable { // 'mutable' to reassign Preamble variable.
+    if (!Preamble) {
----------------
ilya-biryukov wrote:
> klimek wrote:
> > It doesn't seem like we use Preamble anywhere else but in the lambda - so why not get it in the lambda?
> TLDR; To use in async requests exactly the same preamble that was previously used for sync requests.
> 
> Those are two different time points. Our callers might change the file when we'll be executing this lambda. I assume that most of time we'll want the preamble that was built at the point where `codeComplete` is called, not after that.
> 
> On the other hand, after file was changed, code completion results will probably be irrelevant and will be discarded by clients anyway, so that might not matter. I've opted for not changing the behavior and using the same preamble that was previously used by the synchronous version. (Unless `Preamble` was null in the sync case, where we would only improve performance).
That makes sense, but needs a comment in the code :)


================
Comment at: clangd/ClangdServer.h:236-237
+  ///
+  /// Request is processed  asynchronously, you can use returned future to wait
+  /// for results of async request.
+  ///
----------------
Extra space before asynchronously.
Replace first ',' with ';'
... wait for the results of the async request...


https://reviews.llvm.org/D38583