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