[lldb] [llvm] Modify the localCache API to require an explicit commit on CachedFile… (PR #115331)

[via llvm-commits]

================
@@ -88,9 +89,10 @@ Expected<FileCache> llvm::localCache(const Twine &CacheNameRef,
             AddBuffer(std::move(AddBuffer)), TempFile(std::move(TempFile)),
             ModuleName(ModuleName), Task(Task) {}
 
-      ~CacheStream() {
-        // TODO: Manually commit rather than using non-trivial destructor,
-        // allowing to replace report_fatal_errors with a return Error.
+      Error commit() override {
+        if (Committed)
+          return Error::success();
----------------
anjenner wrote:

I added a unit test and also tested what happens if the stream is written after a commit. It aborts the process with an assert failure:

> /usr/include/c++/11/bits/unique_ptr.h:407: typename std::add_lvalue_reference<_Tp>::type std::unique_ptr<_Tp, _Dp>::operator*() const [with _Tp = llvm::raw_pwrite_stream; _Dp = std::default_delete<llvm::raw_pwrite_stream>; typename std::add_lvalue_reference<_Tp>::type = llvm::raw_pwrite_stream&]: Assertion 'get() != pointer()' failed.

This is the same thing that will happen anywhere else in LLVM if an output stream is used after its reset() method has been called. This seems reasonable to me - it will be very obvious if a CacheStream is used after commit, so any such bugs will be found and fixed quickly. Providing a proper error message is of limited value since this should never happen unless there is a bug in the code that is calling the caching API - end users of the compiler won't see it. If such an error message is desirable then it should be implemented at the raw_ostream level so that so it appears for other "write after reset" bugs, so it is outside of the scope of this PR.

https://github.com/llvm/llvm-project/pull/115331