ilya-biryukov added inline comments.

================
Comment at: clangd/DraftStore.h:36
   /// Replace contents of the draft for \p File with \p Contents.
-  void updateDraft(PathRef File, StringRef Contents);
+  void addDraft(PathRef File, StringRef Contents);
+
----------------
simark wrote:
> ilya-biryukov wrote:
> > Could we add versions from LSP's `VersionedTextDocumentIdentifier` here and 
> > make the relevant sanity checks?
> > Applying diffs to the wrong version will cause everything to fall apart, so 
> > we should detect this error and signal it to the client as soon as possible.
> I agree that applying diffs to the wrong version will break basically 
> everything, but even if we detect a version mismatch, I don't see what we 
> could do, since we don't have a mean to report the error to the client.  The 
> only thing we could do is log it (which we already do.
If we couldn't apply a diff, we should return errors from all future operations 
on this file until it gets closed and reopened. Otherwise clangd and the editor 
would see inconsistent contents for the file and results provided by clangd 
would be unreliable.
The errors from any actions on the invalid file would actually be visible to 
the users.

The simplest way to achieve that is to remove the file from `DraftStore` and 
`ClangdServer` on errors from `updateDraft`.
This will give "calling action on non-tracked file" errors for future 
operations and the actual root cause can be figured out from the logs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to