[PATCH] D94554: [clangd] Add a Filesystem that overlays Dirty files.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks, LG!



================
Comment at: clang-tools-extra/clangd/ClangdServer.h:389
+
+  class DirtyFS : public ThreadsafeFS {
+  public:
----------------
njames93 wrote:
> Probably needs moving to its own place, but wasn't sure where best to put it.
I think ClangdServer is fine, but this can be a unique_ptr<ThreadsafeFS> with the class moved to the implementation file


================
Comment at: clang-tools-extra/clangd/DraftStore.cpp:80
 
-  Draft &D = Drafts[File];
-  updateVersion(D, Version);
-  D.Contents = Contents.str();
-  return D.Version;
+  auto Res = Drafts.try_emplace(File);
+  auto &D = Res.first->getValue();
----------------
we don't need try_emplace here, we overwrite the whole value regardless.
Can we use `Draft &D = Drafts[File]` again?


================
Comment at: clang-tools-extra/clangd/DraftStore.cpp:83
+  updateVersion(D.Draft, Version);
+  time(&D.MTime);
+  D.Draft.Contents = std::make_shared<std::string>(Contents);
----------------
nit: std::time() (and std::time_t)


================
Comment at: clang-tools-extra/clangd/DraftStore.cpp:113
+    MemoryBuffer::init(BufferContents->c_str(),
+                       BufferContents->c_str() + BufferContents->size(), true);
+  }
----------------
nit: /*RequiresNullTerminator=*/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94554