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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
