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

Reply via email to