================ @@ -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