[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits



================
Comment at: clangd/index/BackgroundIndexStorage.cpp:39
+template <typename WriterFunction>
+llvm::Error writeAtomically(llvm::StringRef OutPath, WriterFunction &&Writer) {
+  // Write to a temporary file first.
----------------
NIT: use `llvm::function_ref` here. It forces to clearly state the signature of the function and allows to avoid templates.


================
Comment at: clangd/index/BackgroundIndexStorage.cpp:44
+  auto EC = llvm::sys::fs::createUniqueFile(
+      llvm::Twine(OutPath, ".tmp.%%%%%%%%"), FD, TempPath);
+  if (EC)
----------------
NIT: I believe there's an overloaded `operator +(StringRef, const char*)` that would produce a Twine, so maybe simplify to `OutPath + ".tmp.%%%%%"`?
Just a suggestion, up to you.


================
Comment at: clangd/index/BackgroundIndexStorage.cpp:60
+  // If everything went well, we already moved the file to another name. So
+  // don't delete it, as it might be taken by another file.
+  RemoveOnFail.release();
----------------
NIT: `s/it/the name/`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55417