This is an archive of the discontinued LLVM Phabricator instance.

[mlir:LSP] Switch document sync mode to Incremental
ClosedPublic

Authored by rriddle on May 19 2022, 4:38 PM.

Details

Summary

This is much more efficient over the full mode, as it only requires sending
smalls chunks of files. It also works around a weird command ordering
issue (full document updates are being sent after other commands like
code completion) in newer versions of vscode.

Diff Detail

Event Timeline

rriddle created this revision.May 19 2022, 4:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2022, 4:38 PM
rriddle requested review of this revision.May 19 2022, 4:38 PM
rriddle updated this revision to Diff 431757.May 24 2022, 12:23 PM
rriddle edited the summary of this revision. (Show Details)
rriddle updated this revision to Diff 433928.Jun 2 2022, 5:41 PM
jpienaar accepted this revision.Jun 5 2022, 10:26 AM

Nice, injecting errors for testing :-)

mlir/lib/Tools/lsp-server-support/Protocol.cpp
476

And these can't get out of sync? E.g., would there be a failure rather than the state we believe internally here doesn't match what the user sees?

mlir/lib/Tools/lsp-server-support/Protocol.h
330

Why not getAsSMRange?

This revision is now accepted and ready to land.Jun 5 2022, 10:26 AM
This revision was automatically updated to reflect the committed changes.
rriddle marked 2 inline comments as done.
rriddle added inline comments.Jun 6 2022, 8:21 PM
mlir/lib/Tools/lsp-server-support/Protocol.cpp
476

Given that we communicate LSP commands via stdin/stdout it would only really get out-of-sync if the client is sending commands in an invalid order, but in that case given that we only get ranges and positions there isn't much we could do in terms of fully guaranteeing the text file is what it is supposed to be. If we do detect an invalid range though, we essentially forget the file (no longer serve e.g. diagnostics to the user). This is what clangd does in this situation as well.