[clangd] Run formatting operations asynchronously.

[sammccall]

Summary

They don't need ASTs or anything, so they should still run immediately.

These were sync for historical reasons (they predate clangd having a pervasive
threading model). This worked ok as they were "cheap".
Aside for consistency, there are a couple of reasons to make them async:

• they do IO (finding .clang-format) so aren't trivially cheap
• having TUScheduler involved in running these tasks means we can use it as an injection point for configuration. (TUScheduler::run will need to learn about which file is being operated on, but that's an easy change).
• adding the config system adds more potential IO, too

[kbobyrev]

Looks nice just a couple of nits.

clang-tools-extra/clangd/ClangdLSPServer.cpp

900

Reply(Result ? replacementsToEdits(...) : Result.takeError()) may be shorter, but up to you, maybe this looks more readable.

clang-tools-extra/clangd/ClangdServer.h

326

Is this no longer true?

[sammccall]

clang-tools-extra/clangd/ClangdLSPServer.cpp

900

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.

clang-tools-extra/clangd/ClangdServer.h

326

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).

clang-tools-extra/clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onDocumentOnTypeFormatting(
     Callback<std::vector<TextEdit>> Reply) {
   auto File = Params.textDocument.uri.file();
   auto Code = DraftMgr.getDraft(File);
   if (!Code)
     return Reply(llvm::make_error<LSPError>(
         "onDocumentOnTypeFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  Reply(Server->formatOnType(Code->Contents, File, Params.position, Params.ch));
+  Server->formatOnType(File, Code->Contents, Params.position, Params.ch,
+                       std::move(Reply));
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(
     const DocumentRangeFormattingParams &Params,
     Callback<std::vector<TextEdit>> Reply) {
   auto File = Params.textDocument.uri.file();
   auto Code = DraftMgr.getDraft(File);
   if (!Code)
     return Reply(llvm::make_error<LSPError>(
         "onDocumentRangeFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  auto ReplacementsOrError =
-      Server->formatRange(Code->Contents, File, Params.range);
-  if (ReplacementsOrError)
-    Reply(replacementsToEdits(Code->Contents, ReplacementsOrError.get()));
+  Server->formatRange(
+      File, Code->Contents, Params.range,
+      [Code = Code->Contents, Reply = std::move(Reply)](
+          llvm::Expected<tooling::Replacements> Result) mutable {
+        if (Result)
+          Reply(replacementsToEdits(Code, Result.get()));
         else
-    Reply(ReplacementsOrError.takeError());
+          Reply(Result.takeError());
+      });
 }
 
 void ClangdLSPServer::onDocumentFormatting(
     const DocumentFormattingParams &Params,
     Callback<std::vector<TextEdit>> Reply) {
   auto File = Params.textDocument.uri.file();
   auto Code = DraftMgr.getDraft(File);
   if (!Code)
     return Reply(llvm::make_error<LSPError>(
         "onDocumentFormatting called for non-added file",
         ErrorCode::InvalidParams));
 
-  auto ReplacementsOrError = Server->formatFile(Code->Contents, File);
-  if (ReplacementsOrError)
-    Reply(replacementsToEdits(Code->Contents, ReplacementsOrError.get()));
+  Server->formatFile(File, Code->Contents,
+                     [Code = Code->Contents, Reply = std::move(Reply)](
+                         llvm::Expected<tooling::Replacements> Result) mutable {
+                       if (Result)
+                         Reply(replacementsToEdits(Code, Result.get()));
                        else
-    Reply(ReplacementsOrError.takeError());
+                         Reply(Result.takeError());
+                     });
 }
 
 /// The functions constructs a flattened view of the DocumentSymbol hierarchy.
 /// Used by the clients that do not support the hierarchical view.
 static std::vector<SymbolInformation>
 flattenSymbolHierarchy(llvm::ArrayRef<DocumentSymbol> Symbols,
                        const URIForFile &FileURI) {
 

clang-tools-extra/clangd/ClangdServer.h

public:
   void documentSymbols(StringRef File,
                        Callback<std::vector<DocumentSymbol>> CB);
 
   /// Retrieve locations for symbol references.
   void findReferences(PathRef File, Position Pos, uint32_t Limit,
                       Callback<ReferencesResult> CB);
 
   /// Run formatting for \p Rng inside \p File with content \p Code.
-  llvm::Expected<tooling::Replacements> formatRange(StringRef Code,
-                                                    PathRef File, Range Rng);
+  void formatRange(PathRef File, StringRef Code, Range Rng,
+                   Callback<tooling::Replacements> CB);
 
   /// Run formatting for the whole \p File with content \p Code.
-  llvm::Expected<tooling::Replacements> formatFile(StringRef Code,
-                                                   PathRef File);
+  void formatFile(PathRef File, StringRef Code,
+                  Callback<tooling::Replacements> CB);
 
   /// Run formatting after \p TriggerText was typed at \p Pos in \p File with
   /// content \p Code.
-  llvm::Expected<std::vector<TextEdit>> formatOnType(StringRef Code,
-                                                     PathRef File, Position Pos,
-                                                     StringRef TriggerText);
+  void formatOnType(PathRef File, StringRef Code, Position Pos,
+                    StringRef TriggerText, Callback<std::vector<TextEdit>> CB);
 
   /// Test the validity of a rename operation.
   void prepareRename(PathRef File, Position Pos,
                      const RenameOptions &RenameOpts,
                      Callback<llvm::Optional<Range>> CB);
 
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
public:
   llvm::StringMap<TUScheduler::FileStats> fileStats() const;
 
   // Blocks the main thread until the server is idle. Only for use in tests.
   // Returns false if the timeout expires.
   LLVM_NODISCARD bool
   blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds = 10);
 
 private:
-  /// FIXME: This stats several files to find a .clang-format file. I/O can be
-  /// slow. Think of a way to cache this.
-  llvm::Expected<tooling::Replacements>
-  formatCode(llvm::StringRef Code, PathRef File,
-             ArrayRef<tooling::Range> Ranges);
+  void formatCode(PathRef File, llvm::StringRef Code,
+                  ArrayRef<tooling::Range> Ranges,
+                  Callback<tooling::Replacements> CB);
 
   const ThreadsafeFS &TFS;
 
   Path ResourceDir;
   // The index used to look up symbols. This could be:
   //   - null (all index functionality is optional)
   //   - the dynamic index owned by ClangdServer (DynamicIdx)
   //   - the static index passed to the constructor

clang-tools-extra/clangd/ClangdServer.cpp

auto Action = [Pos, File = File.str(), CB = std::move(CB),
     CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput));
   };
 
   // Unlike code completion, we wait for a preamble here.
   WorkScheduler.runWithPreamble("SignatureHelp", File, TUScheduler::Stale,
                                 std::move(Action));
 }
 
-llvm::Expected<tooling::Replacements>
-ClangdServer::formatRange(llvm::StringRef Code, PathRef File, Range Rng) {
+void ClangdServer::formatRange(PathRef File, llvm::StringRef Code, Range Rng,
+                               Callback<tooling::Replacements> CB) {
   llvm::Expected<size_t> Begin = positionToOffset(Code, Rng.start);
   if (!Begin)
-    return Begin.takeError();
+    return CB(Begin.takeError());
   llvm::Expected<size_t> End = positionToOffset(Code, Rng.end);
   if (!End)
-    return End.takeError();
-  return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)});
+    return CB(End.takeError());
+  formatCode(File, Code, {tooling::Range(*Begin, *End - *Begin)},
+             std::move(CB));
 }
 
-llvm::Expected<tooling::Replacements>
-ClangdServer::formatFile(llvm::StringRef Code, PathRef File) {
+void ClangdServer::formatFile(PathRef File, llvm::StringRef Code,
+                              Callback<tooling::Replacements> CB) {
   // Format everything.
-  return formatCode(Code, File, {tooling::Range(0, Code.size())});
+  formatCode(File, Code, {tooling::Range(0, Code.size())}, std::move(CB));
 }
 
-llvm::Expected<std::vector<TextEdit>>
-ClangdServer::formatOnType(llvm::StringRef Code, PathRef File, Position Pos,
-                           StringRef TriggerText) {
+void ClangdServer::formatOnType(PathRef File, llvm::StringRef Code,
+                                Position Pos, StringRef TriggerText,
+                                Callback<std::vector<TextEdit>> CB) {
   llvm::Expected<size_t> CursorPos = positionToOffset(Code, Pos);
   if (!CursorPos)
-    return CursorPos.takeError();
+    return CB(CursorPos.takeError());
+  auto Action = [File = File.str(), Code = Code.str(),
+                 TriggerText = TriggerText.str(), CursorPos = *CursorPos,
+                 CB = std::move(CB), this]() mutable {
     auto Style = format::getStyle(format::DefaultFormatStyle, File,
                                   format::DefaultFallbackStyle, Code,
                                   TFS.view(/*CWD=*/llvm::None).get());
     if (!Style)
-    return Style.takeError();
+      return CB(Style.takeError());
 
     std::vector<TextEdit> Result;
     for (const tooling::Replacement &R :
-       formatIncremental(Code, *CursorPos, TriggerText, *Style))
+         formatIncremental(Code, CursorPos, TriggerText, *Style))
       Result.push_back(replacementToEdit(Code, R));
-  return Result;
+    return CB(Result);
+  };
+  WorkScheduler.run("FormatOnType", std::move(Action));
 }
 
 void ClangdServer::prepareRename(PathRef File, Position Pos,
                                  const RenameOptions &RenameOpts,
                                  Callback<llvm::Optional<Range>> CB) {
   auto Action = [Pos, File = File.str(), CB = std::move(CB), RenameOpts,
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
auto Action = [Path = Path.str(), CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(getCorrespondingHeaderOrSource(Path, InpAST->AST, Index));
   };
   WorkScheduler.runWithAST("SwitchHeaderSource", Path, std::move(Action));
 }
 
-llvm::Expected<tooling::Replacements>
-ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
-                         llvm::ArrayRef<tooling::Range> Ranges) {
+void ClangdServer::formatCode(PathRef File, llvm::StringRef Code,
+                              llvm::ArrayRef<tooling::Range> Ranges,
+                              Callback<tooling::Replacements> CB) {
   // Call clang-format.
+  auto Action = [File = File.str(), Code = Code.str(), Ranges = Ranges.vec(),
+                 CB = std::move(CB), this]() mutable {
     format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS);
     tooling::Replacements IncludeReplaces =
         format::sortIncludes(Style, Code, Ranges, File);
     auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
     if (!Changed)
-    return Changed.takeError();
+      return CB(Changed.takeError());
 
-  return IncludeReplaces.merge(format::reformat(
+    CB(IncludeReplaces.merge(format::reformat(
         Style, *Changed,
         tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-      File));
+        File)));
+  };
+  WorkScheduler.run("Format", std::move(Action));
 }
 
 void ClangdServer::findDocumentHighlights(
     PathRef File, Position Pos, Callback<std::vector<DocumentHighlight>> CB) {
   auto Action =
       [Pos, CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
         if (!InpAST)
           return CB(InpAST.takeError());

clang-tools-extra/clangd/unittests/ClangdTests.cpp

 #include "x.h"
 #include "y.h"
 
 void f() {}
 )cpp";
   FS.Files[Path] = Code;
   runAddDocument(Server, Path, Code);
 
-  auto Replaces = Server.formatFile(Code, Path);
+  auto Replaces = runFormatFile(Server, Path, Code);
   EXPECT_TRUE(static_cast<bool>(Replaces));
   auto Changed = tooling::applyAllReplacements(Code, *Replaces);
   EXPECT_TRUE(static_cast<bool>(Changed));
   EXPECT_EQ(Expected, *Changed);
 }
 
 TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) {
   MockFS FS;

clang-tools-extra/clangd/unittests/SyncAPI.h

 
 llvm::Expected<std::vector<DocumentHighlight>>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
 llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
                                     Position Pos, StringRef NewName,
                                     const clangd::RenameOptions &RenameOpts);
 
+llvm::Expected<tooling::Replacements>
+runFormatFile(ClangdServer &Server, PathRef File, StringRef Code);
+
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
 llvm::Expected<std::vector<SymbolInformation>>
 runWorkspaceSymbols(ClangdServer &Server, StringRef Query, int Limit);
 
 Expected<std::vector<DocumentSymbol>> runDocumentSymbols(ClangdServer &Server,
                                                          PathRef File);
 

clang-tools-extra/clangd/unittests/SyncAPI.cpp

 llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
                                     Position Pos, llvm::StringRef NewName,
                                     const RenameOptions &RenameOpts) {
   llvm::Optional<llvm::Expected<FileEdits>> Result;
   Server.rename(File, Pos, NewName, RenameOpts, capture(Result));
   return std::move(*Result);
 }
 
+llvm::Expected<tooling::Replacements>
+runFormatFile(ClangdServer &Server, PathRef File, StringRef Code) {
+  llvm::Optional<llvm::Expected<tooling::Replacements>> Result;
+  Server.formatFile(File, Code, capture(Result));
+  return std::move(*Result);
+}
+
 std::string runDumpAST(ClangdServer &Server, PathRef File) {
   llvm::Optional<std::string> Result;
   Server.dumpAST(File, capture(Result));
   return std::move(*Result);
 }
 
 llvm::Expected<std::vector<SymbolInformation>>
 runWorkspaceSymbols(ClangdServer &Server, llvm::StringRef Query, int Limit) {