PATCH: add a c'tor to ErrorOr that allows us to perform two user defined conversions

[David Blaikie via llvm-commits]

LGTM, commit when ready

(optional thoughts: ImplicitConversionCausesMove could perhaps use some
checks? (check that the resulting ErrorOr is in the "non-error" state (not
much else you can check, since the Destination type is stateless anyway) &
not sure if it's worth leaving in the ImplicitConversionWithoutMove test -
but yeah, looks pretty reasonable to me))

Thanks!

On Mon, Feb 8, 2016 at 8:10 PM, Nick Lewycky <nlewycky at google.com> wrote:

> On 8 February 2016 at 17:32, David Blaikie <dblaikie at gmail.com> wrote:
>
>> (sorry for the delay)
>>
>> Here's some code that demonstrates the move problem:
>>
>> TEST(ErrorOr, ImplicitConversionCausesMove) {
>>   struct Source {
>>   };
>>   struct Destination {
>>     Destination(const Source&){}
>>     Destination(Source&&) = delete;
>>   };
>>   Source s;
>>   ErrorOr<Destination> x = s;
>> }
>>
>> with the 'std::move' the code calls the deleted ctor taking a Source&&,
>> dropping the std::move gets it back on the right track.
>>
>> Presumably we can remove the ErrorOr(T) ctor in favor of this more
>> general perfectly forwarding one anyway, perhaps?
>>
>
> That went well. Updated patch attached!
>
> Nick
>
>
>>
>> On Thu, Feb 4, 2016 at 11:30 AM, Nick Lewycky <nlewycky at google.com>
>> wrote:
>>
>>> On 3 February 2016 at 21:41, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>> Drop the std::move (this would incorrectly move in this case, I think:
>>>>
>>>> std::string s;
>>>> ErrorOr<std::string> g = s;
>>>>
>>>> I think?
>>>>
>>>
>>> I tried this case out, but it it matches "ErrorOr(T Val)" and doesn't
>>> call my new c'tor overload at all. Do you have a similar thing for me to
>>> try?
>>>
>>>
>>>> Perhaps you could add a test case for a case like that to ensure the
>>>> move does not occur (you'd need a type that recorded its move operations,
>>>> etc)
>>>>
>>>
>>> Other than that, please commit
>>>>
>>>
>>> I got feedback from Richard Smith off-list that I should also check for
>>> types that convert to std::error_code. I haven't been able to get that case
>>> to do anything wrong either.
>>>
>>> More testcases added, but logic in ErrorOr not changed. Please review!
>>>
>>> On Wed, Feb 3, 2016 at 6:58 PM, Nick Lewycky via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> The attached patch changes ErrorOr to make this code:
>>>>>
>>>>>   ErrorOr<string> test() { return "literal"; }
>>>>>
>>>>> work. Without this patch it fails because it would require two
>>>>> user-defined conversions in a row. Please review!
>>>>>
>>>>> I'm not sure whether it's correct to describe this as "perfect
>>>>> forwarding"?
>>>>>
>>>>> Nick
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160208/d17e5767/attachment.html>