ilya-biryukov marked an inline comment as done.
ilya-biryukov 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;
----------------
kadircet wrote:
> 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.
You are right, I'm sorry. I did not read into the code, just assumed it was
doing something it doesn't.
Again, very sorry for the disturbance.
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
