kadircet marked an inline comment as done.
kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644
+          assert(It.second.Edits && "TextEdits hasn't been generated?");
+          if (auto Draft = DraftMgr.getDraft(It.first())) {
+            llvm::StringRef Contents = *Draft;
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > This callback is called asynchronously and the version inside `DraftMgr` 
> > may be newer than the one we used to calculate the original offsets inside 
> > replacements at this point.
> > 
> > We should not rely on `DraftMgr`, doing conversions on the source buffers 
> > inside `SourceManager` is probably the most reliable option we have.
> I think you've misunderstood what this code is doing (it needs comments!)
> 
> The SourceManager contains the code that we've calculated the edits against, 
> by definition. So checking against it does nothing.
> 
> When we send the edits to the client, it's going to apply them to the file if 
> it's not open, but to the dirty buffer if it is open.
> If the dirty buffer has different contents than what we saw (and calculated 
> edits against) then the edits will be wrong - that's what this code guards 
> against.
> 
> So looking at the latest available content in the draftmgr (at the end of the 
> request, not the start!) is the right thing here, I think.
> This callback is called asynchronously and the version inside DraftMgr may be 
> newer than the one we used to calculate the original offsets inside 
> replacements at this point.

That's exactly what we are checking though?

We want to bail out if user has a "different" version of the source code in 
their editor by the time we generated edits. Since editors will most likely try 
to apply the edits onto the version in editor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66637/new/

https://reviews.llvm.org/D66637



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

Reply via email to