[PATCH] D82642: [clangd] Run formatting operations asynchronously.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 30 15:46:17 PDT 2020


sammccall marked 4 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:900
+          llvm::Expected<tooling::Replacements> Result) mutable {
+        if (Result)
+          Reply(replacementsToEdits(Code, Result.get()));
----------------
kbobyrev wrote:
> `Reply(Result ? replacementsToEdits(...) : Result.takeError())` may be shorter, but up to you, maybe this looks more readable.
Sadly it doesn't compile... the usual thing about not being able to determine a common type for the two expressions, and C++ doesn't infer it from the type that's required in the context.


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:326
 private:
-  /// FIXME: This stats several files to find a .clang-format file. I/O can be
-  /// slow. Think of a way to cache this.
----------------
kbobyrev wrote:
> Is this no longer true?
It's still true that we do IO and it might be nice to cache it.
However this same IO (to get the formatstyle) also now happens in lots of other places too (without comments), and the only reason this one was especially bad is that it happened on the main thread (which is no longer true).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82642/new/

https://reviews.llvm.org/D82642





More information about the cfe-commits mailing list