Skip to content

Commit bca3e24

Browse files
committed
[clangd] Move DraftStore from ClangdLSPServer into ClangdServer.
ClangdServer already gets notified of every change, so it makes sense for it to be the source of truth. This is a step towards having ClangdServer expose a FS that includes dirty buffers: D94554 Related changes: - version is now optional for ClangdServer, to preserve our existing fuzziness in this area (missing version ==> autoincrement) - ClangdServer::format{File,Range} are now more regular ClangdServer functions that don't need the code passed in. While here, combine into one function. - incremental content update logic is moved from DraftStore to ClangdLSPServer, with most of the implementation in SourceCode.cpp. DraftStore is now fairly trivial, and will probably ultimately be *replaced* by the dirty FS stuff. Differential Revision: https://reviews.llvm.org/D97738
1 parent 00c7d66 commit bca3e24

14 files changed

+471
-608
lines changed

clang-tools-extra/clangd/ClangdLSPServer.cpp

Lines changed: 48 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ constexpr trace::Metric LSPLatency("lsp_latency", trace::Metric::Distribution,
5959

6060
// LSP defines file versions as numbers that increase.
6161
// ClangdServer treats them as opaque and therefore uses strings instead.
62-
std::string encodeVersion(int64_t LSPVersion) {
63-
return llvm::to_string(LSPVersion);
62+
std::string encodeVersion(llvm::Optional<int64_t> LSPVersion) {
63+
return LSPVersion ? llvm::to_string(*LSPVersion) : "";
6464
}
6565
llvm::Optional<int64_t> decodeVersion(llvm::StringRef Encoded) {
6666
int64_t Result;
@@ -124,15 +124,15 @@ CompletionItemKindBitset defaultCompletionItemKinds() {
124124
// Makes sure edits in \p FE are applicable to latest file contents reported by
125125
// editor. If not generates an error message containing information about files
126126
// that needs to be saved.
127-
llvm::Error validateEdits(const DraftStore &DraftMgr, const FileEdits &FE) {
127+
llvm::Error validateEdits(const ClangdServer &Server, const FileEdits &FE) {
128128
size_t InvalidFileCount = 0;
129129
llvm::StringRef LastInvalidFile;
130130
for (const auto &It : FE) {
131-
if (auto Draft = DraftMgr.getDraft(It.first())) {
131+
if (auto Draft = Server.getDraft(It.first())) {
132132
// If the file is open in user's editor, make sure the version we
133133
// saw and current version are compatible as this is the text that
134134
// will be replaced by editors.
135-
if (!It.second.canApplyTo(Draft->Contents)) {
135+
if (!It.second.canApplyTo(*Draft)) {
136136
++InvalidFileCount;
137137
LastInvalidFile = It.first();
138138
}
@@ -648,8 +648,8 @@ void ClangdLSPServer::onDocumentDidOpen(
648648

649649
const std::string &Contents = Params.textDocument.text;
650650

651-
auto Version = DraftMgr.addDraft(File, Params.textDocument.version, Contents);
652-
Server->addDocument(File, Contents, encodeVersion(Version),
651+
Server->addDocument(File, Contents,
652+
encodeVersion(Params.textDocument.version),
653653
WantDiagnostics::Yes);
654654
}
655655

@@ -661,25 +661,28 @@ void ClangdLSPServer::onDocumentDidChange(
661661
: WantDiagnostics::No;
662662

663663
PathRef File = Params.textDocument.uri.file();
664-
llvm::Expected<DraftStore::Draft> Draft = DraftMgr.updateDraft(
665-
File, Params.textDocument.version, Params.contentChanges);
666-
if (!Draft) {
667-
// If this fails, we are most likely going to be not in sync anymore with
668-
// the client. It is better to remove the draft and let further operations
669-
// fail rather than giving wrong results.
670-
DraftMgr.removeDraft(File);
671-
Server->removeDocument(File);
672-
elog("Failed to update {0}: {1}", File, Draft.takeError());
664+
auto Code = Server->getDraft(File);
665+
if (!Code) {
666+
log("Trying to incrementally change non-added document: {0}", File);
673667
return;
674668
}
675-
676-
Server->addDocument(File, Draft->Contents, encodeVersion(Draft->Version),
669+
for (const auto &Change : Params.contentChanges) {
670+
if (auto Err = applyChange(*Code, Change)) {
671+
// If this fails, we are most likely going to be not in sync anymore with
672+
// the client. It is better to remove the draft and let further
673+
// operations fail rather than giving wrong results.
674+
Server->removeDocument(File);
675+
elog("Failed to update {0}: {1}", File, std::move(Err));
676+
return;
677+
}
678+
}
679+
Server->addDocument(File, *Code, encodeVersion(Params.textDocument.version),
677680
WantDiags, Params.forceRebuild);
678681
}
679682

680683
void ClangdLSPServer::onDocumentDidSave(
681684
const DidSaveTextDocumentParams &Params) {
682-
reparseOpenFilesIfNeeded([](llvm::StringRef) { return true; });
685+
Server->reparseOpenFilesIfNeeded([](llvm::StringRef) { return true; });
683686
}
684687

685688
void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
@@ -720,13 +723,8 @@ void ClangdLSPServer::onCommandApplyEdit(const WorkspaceEdit &WE,
720723

721724
void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args,
722725
Callback<llvm::json::Value> Reply) {
723-
auto Code = DraftMgr.getDraft(Args.file.file());
724-
if (!Code)
725-
return Reply(error("trying to apply a code action for a non-added file"));
726-
727-
auto Action = [this, Reply = std::move(Reply), File = Args.file,
728-
Code = std::move(*Code)](
729-
llvm::Expected<Tweak::Effect> R) mutable {
726+
auto Action = [this, Reply = std::move(Reply),
727+
File = Args.file](llvm::Expected<Tweak::Effect> R) mutable {
730728
if (!R)
731729
return Reply(R.takeError());
732730

@@ -742,7 +740,7 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args,
742740
if (R->ApplyEdits.empty())
743741
return Reply("Tweak applied.");
744742

745-
if (auto Err = validateEdits(DraftMgr, R->ApplyEdits))
743+
if (auto Err = validateEdits(*Server, R->ApplyEdits))
746744
return Reply(std::move(Err));
747745

748746
WorkspaceEdit WE;
@@ -808,7 +806,7 @@ void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
808806
void ClangdLSPServer::onRename(const RenameParams &Params,
809807
Callback<WorkspaceEdit> Reply) {
810808
Path File = std::string(Params.textDocument.uri.file());
811-
if (!DraftMgr.getDraft(File))
809+
if (!Server->getDraft(File))
812810
return Reply(llvm::make_error<LSPError>(
813811
"onRename called for non-added file", ErrorCode::InvalidParams));
814812
Server->rename(
@@ -817,7 +815,7 @@ void ClangdLSPServer::onRename(const RenameParams &Params,
817815
this](llvm::Expected<RenameResult> R) mutable {
818816
if (!R)
819817
return Reply(R.takeError());
820-
if (auto Err = validateEdits(DraftMgr, R->GlobalChanges))
818+
if (auto Err = validateEdits(*Server, R->GlobalChanges))
821819
return Reply(std::move(Err));
822820
WorkspaceEdit Result;
823821
Result.changes.emplace();
@@ -832,7 +830,6 @@ void ClangdLSPServer::onRename(const RenameParams &Params,
832830
void ClangdLSPServer::onDocumentDidClose(
833831
const DidCloseTextDocumentParams &Params) {
834832
PathRef File = Params.textDocument.uri.file();
835-
DraftMgr.removeDraft(File);
836833
Server->removeDocument(File);
837834

838835
{
@@ -857,52 +854,35 @@ void ClangdLSPServer::onDocumentOnTypeFormatting(
857854
const DocumentOnTypeFormattingParams &Params,
858855
Callback<std::vector<TextEdit>> Reply) {
859856
auto File = Params.textDocument.uri.file();
860-
auto Code = DraftMgr.getDraft(File);
861-
if (!Code)
862-
return Reply(llvm::make_error<LSPError>(
863-
"onDocumentOnTypeFormatting called for non-added file",
864-
ErrorCode::InvalidParams));
865-
866-
Server->formatOnType(File, Code->Contents, Params.position, Params.ch,
867-
std::move(Reply));
857+
Server->formatOnType(File, Params.position, Params.ch, std::move(Reply));
868858
}
869859

870860
void ClangdLSPServer::onDocumentRangeFormatting(
871861
const DocumentRangeFormattingParams &Params,
872862
Callback<std::vector<TextEdit>> Reply) {
873863
auto File = Params.textDocument.uri.file();
874-
auto Code = DraftMgr.getDraft(File);
875-
if (!Code)
876-
return Reply(llvm::make_error<LSPError>(
877-
"onDocumentRangeFormatting called for non-added file",
878-
ErrorCode::InvalidParams));
879-
880-
Server->formatRange(
881-
File, Code->Contents, Params.range,
882-
[Code = Code->Contents, Reply = std::move(Reply)](
883-
llvm::Expected<tooling::Replacements> Result) mutable {
884-
if (Result)
885-
Reply(replacementsToEdits(Code, Result.get()));
886-
else
887-
Reply(Result.takeError());
888-
});
864+
auto Code = Server->getDraft(File);
865+
Server->formatFile(File, Params.range,
866+
[Code = std::move(Code), Reply = std::move(Reply)](
867+
llvm::Expected<tooling::Replacements> Result) mutable {
868+
if (Result)
869+
Reply(replacementsToEdits(*Code, Result.get()));
870+
else
871+
Reply(Result.takeError());
872+
});
889873
}
890874

891875
void ClangdLSPServer::onDocumentFormatting(
892876
const DocumentFormattingParams &Params,
893877
Callback<std::vector<TextEdit>> Reply) {
894878
auto File = Params.textDocument.uri.file();
895-
auto Code = DraftMgr.getDraft(File);
896-
if (!Code)
897-
return Reply(llvm::make_error<LSPError>(
898-
"onDocumentFormatting called for non-added file",
899-
ErrorCode::InvalidParams));
900-
901-
Server->formatFile(File, Code->Contents,
902-
[Code = Code->Contents, Reply = std::move(Reply)](
879+
auto Code = Server->getDraft(File);
880+
Server->formatFile(File,
881+
/*Rng=*/llvm::None,
882+
[Code = std::move(Code), Reply = std::move(Reply)](
903883
llvm::Expected<tooling::Replacements> Result) mutable {
904884
if (Result)
905-
Reply(replacementsToEdits(Code, Result.get()));
885+
Reply(replacementsToEdits(*Code, Result.get()));
906886
else
907887
Reply(Result.takeError());
908888
});
@@ -978,11 +958,6 @@ static llvm::Optional<Command> asCommand(const CodeAction &Action) {
978958
void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
979959
Callback<llvm::json::Value> Reply) {
980960
URIForFile File = Params.textDocument.uri;
981-
auto Code = DraftMgr.getDraft(File.file());
982-
if (!Code)
983-
return Reply(llvm::make_error<LSPError>(
984-
"onCodeAction called for non-added file", ErrorCode::InvalidParams));
985-
986961
// Checks whether a particular CodeActionKind is included in the response.
987962
auto KindAllowed = [Only(Params.context.only)](llvm::StringRef Kind) {
988963
if (Only.empty())
@@ -1005,8 +980,8 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
1005980

1006981
// Now enumerate the semantic code actions.
1007982
auto ConsumeActions =
1008-
[Reply = std::move(Reply), File, Code = std::move(*Code),
1009-
Selection = Params.range, FixIts = std::move(FixIts), this](
983+
[Reply = std::move(Reply), File, Selection = Params.range,
984+
FixIts = std::move(FixIts), this](
1010985
llvm::Expected<std::vector<ClangdServer::TweakRef>> Tweaks) mutable {
1011986
if (!Tweaks)
1012987
return Reply(Tweaks.takeError());
@@ -1246,7 +1221,7 @@ void ClangdLSPServer::applyConfiguration(
12461221
}
12471222
}
12481223

1249-
reparseOpenFilesIfNeeded(
1224+
Server->reparseOpenFilesIfNeeded(
12501225
[&](llvm::StringRef File) { return ModifiedFiles.count(File) != 0; });
12511226
}
12521227

@@ -1557,17 +1532,17 @@ bool ClangdLSPServer::shouldRunCompletion(
15571532
const CompletionParams &Params) const {
15581533
if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter)
15591534
return true;
1560-
auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
1535+
auto Code = Server->getDraft(Params.textDocument.uri.file());
15611536
if (!Code)
15621537
return true; // completion code will log the error for untracked doc.
1563-
auto Offset = positionToOffset(Code->Contents, Params.position,
1538+
auto Offset = positionToOffset(*Code, Params.position,
15641539
/*AllowColumnsBeyondLineLength=*/false);
15651540
if (!Offset) {
15661541
vlog("could not convert position '{0}' to offset for file '{1}'",
15671542
Params.position, Params.textDocument.uri.file());
15681543
return true;
15691544
}
1570-
return allowImplicitCompletion(Code->Contents, *Offset);
1545+
return allowImplicitCompletion(*Code, *Offset);
15711546
}
15721547

15731548
void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
@@ -1681,16 +1656,5 @@ void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
16811656
NotifyFileStatus(Status.render(File));
16821657
}
16831658

1684-
void ClangdLSPServer::reparseOpenFilesIfNeeded(
1685-
llvm::function_ref<bool(llvm::StringRef File)> Filter) {
1686-
// Reparse only opened files that were modified.
1687-
for (const Path &FilePath : DraftMgr.getActiveFiles())
1688-
if (Filter(FilePath))
1689-
if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
1690-
Server->addDocument(FilePath, std::move(Draft->Contents),
1691-
encodeVersion(Draft->Version),
1692-
WantDiagnostics::Auto);
1693-
}
1694-
16951659
} // namespace clangd
16961660
} // namespace clang

clang-tools-extra/clangd/ClangdLSPServer.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
194194
/// produce '->' and '::', respectively.
195195
bool shouldRunCompletion(const CompletionParams &Params) const;
196196

197-
/// Requests a reparse of currently opened files using their latest source.
198-
/// This will typically only rebuild if something other than the source has
199-
/// changed (e.g. the CDB yields different flags, or files included in the
200-
/// preamble have been modified).
201-
void reparseOpenFilesIfNeeded(
202-
llvm::function_ref<bool(llvm::StringRef File)> Filter);
203197
void applyConfiguration(const ConfigurationSettings &Settings);
204198

205199
/// Runs profiling and exports memory usage metrics if tracing is enabled and
@@ -282,8 +276,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
282276
BackgroundQueue::Stats PendingBackgroundIndexProgress;
283277
/// LSP extension: skip WorkDoneProgressCreate, just send progress streams.
284278
bool BackgroundIndexSkipCreate = false;
285-
// Store of the current versions of the open documents.
286-
DraftStore DraftMgr;
287279

288280
Options Opts;
289281
// The CDB is created by the "initialize" LSP method.

0 commit comments

Comments
 (0)