================
@@ -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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits