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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
