[PATCH] D49013: [Support] Allow formatv() to consume llvm::Error by-value without crashing.

[Sam McCall via llvm-commits]

On Tue, Jul 10, 2018, 18:04 David Blaikie <dblaikie at gmail.com> wrote:

> That seems plausible (op<< for Error) - but does that handle the case
> mentioned above "we may not actually want to produce the string (eg: if the
> Logger implementation decides to filter out the formatv_object_base based
> on level)" - is op<< still called even with this filtering? (seems like if
> it was, then it wouldn't avoid the stringification costs? but if it doesn't
> call the op<< then it'll still fail the consume semantic requirement of
> llvm::Error)
>
Ack, you're right. If you don't print the formatv_object, the Error isn't
consumed. Worse, if you print it twice, the second call sees a moved-From
error. Fundamentally, a non-const <<Error can't be used in a const
<<formatv_object.

Thanks for pointing this out. I think D49129 is a dead end.
(Why can't Error just be a normal value type... so hard to reason about :-(
)


> On Tue, Jul 10, 2018 at 7:03 AM Sam McCall via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> sammccall added a comment.
>>
>> An alternate approach in https://reviews.llvm.org/D49129: give `Error`
>> an overloaded `operator<<` that has the same consume semantics at
>> `toString` when passed rvalues.
>>
>> This makes `formatv`, `to_string`, `toString` all do basically the same
>> thing (which is the only sensible thing to do when passed an rvalue).
>>
>> It does require minor changes to `formatv` and `to_string` to make them
>> propagate the right kind of reference, but these seem reasonable.
>>
>> Overall I think I like that approach better than this one (more
>> consistent, Error handling lives in Error.h). WDYT?
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D49013
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180710/0e44fe1c/attachment.html>