[clangd] Run formatting operations asynchronously.

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

Reviewers: kbobyrev

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82642
This commit is contained in:
Sam McCall 2020-06-26 12:57:29 +02:00
parent b210c9899b
commit ffa63dde8e
6 changed files with 84 additions and 60 deletions

View File

@ -879,7 +879,8 @@ void ClangdLSPServer::onDocumentOnTypeFormatting(
"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(
@ -892,12 +893,15 @@ void ClangdLSPServer::onDocumentRangeFormatting(
"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()));
else
Reply(ReplacementsOrError.takeError());
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(Result.takeError());
});
}
void ClangdLSPServer::onDocumentFormatting(
@ -910,11 +914,14 @@ void ClangdLSPServer::onDocumentFormatting(
"onDocumentFormatting called for non-added file",
ErrorCode::InvalidParams));
auto ReplacementsOrError = Server->formatFile(Code->Contents, File);
if (ReplacementsOrError)
Reply(replacementsToEdits(Code->Contents, ReplacementsOrError.get()));
else
Reply(ReplacementsOrError.takeError());
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(Result.takeError());
});
}
/// The functions constructs a flattened view of the DocumentSymbol hierarchy.

View File

@ -296,40 +296,46 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
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();
auto Style = format::getStyle(format::DefaultFormatStyle, File,
format::DefaultFallbackStyle, Code,
TFS.view(/*CWD=*/llvm::None).get());
if (!Style)
return Style.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 CB(Style.takeError());
std::vector<TextEdit> Result;
for (const tooling::Replacement &R :
formatIncremental(Code, *CursorPos, TriggerText, *Style))
Result.push_back(replacementToEdit(Code, R));
return Result;
std::vector<TextEdit> Result;
for (const tooling::Replacement &R :
formatIncremental(Code, CursorPos, TriggerText, *Style))
Result.push_back(replacementToEdit(Code, R));
return CB(Result);
};
WorkScheduler.run("FormatOnType", std::move(Action));
}
void ClangdServer::prepareRename(PathRef File, Position Pos,
@ -561,21 +567,25 @@ void ClangdServer::switchSourceHeader(
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.
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();
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 CB(Changed.takeError());
return IncludeReplaces.merge(format::reformat(
Style, *Changed,
tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
File));
CB(IncludeReplaces.merge(format::reformat(
Style, *Changed,
tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
File)));
};
WorkScheduler.run("Format", std::move(Action));
}
void ClangdServer::findDocumentHighlights(

View File

@ -247,18 +247,17 @@ public:
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,
@ -323,11 +322,9 @@ public:
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;

View File

@ -878,7 +878,7 @@ void f() {}
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));

View File

@ -105,6 +105,13 @@ llvm::Expected<FileEdits> runRename(ClangdServer &Server, PathRef File,
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));

View File

@ -44,6 +44,9 @@ 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>>