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

[Zachary Turner via llvm-commits]

The more I think about it the more I feel like the explicit format adapter
is the way to go. To prevent it from crashing if someone passes by value,
can we just explicitly delete the instantiation of `formatProvider<Error>`?
On Tue, Jul 10, 2018 at 9:22 AM Sam McCall <sammccall at google.com> wrote:

> 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/09337b30/attachment.html>