Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.

[Ville Voutilainen <ville dot voutilainen at gmail dot com>]

On 13 July 2016 at 01:31, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 11/07/16 23:41 +0300, Ville Voutilainen wrote:
>>
>> @@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         }
>>     };
>>
>> +  template<typename _Tp>
>> +    using __optional_relop_t =
>> +    enable_if_t<is_constructible<bool, _Tp>::value, bool>;
>
>
> Should this be is_convertible<_Tp, bool> instead?

Yeah.. it would be more reasonable to return a type explicitly
convertible to bool from a relop
if a non-bool is returned, but since built-in operators are not
contextually-convertible-to-bool,
that "protection" wouldn't buy much. And since the implementation
doesn't really do bool-wrappings
everywhere, let's go with is_convertible and change that if someone complains.

>
>>   template<typename _Tp>
>> -    constexpr bool
>> +    constexpr auto
>>     operator!=(const optional<_Tp>& __lhs, _Tp const& __rhs)
>
>
> Dunno why this has _Tp const& rather than const _Tp&, could you fix it
> while you're in the file anyway? It's a bit confusing to have one
> place using a different style.

Ha, that was indeed in just one place.

I made the above changes and also made converting assignment operators
SFINAE. That SFINAE
seems consistent with how constructors and relops work. And yes, there
are still some members like
emplace that static_assert rather than SFINAE, but I think that's ok for now.

2016-07-13  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Implement P0307R2, Making Optional Greater Equal Again.
    * include/std/optional (operator=(_Up&&)): Constrain.
    (operator=(const optional<_Up>&)): Likewise.
    (operator=(optional<_Up>&&)): Likewise.
    (__optional_relop_t): New.
    (operator==(const optional<_Tp>&, const optional<_Tp>&)): Constrain.
    (operator!=(const optional<_Tp>&, const optional<_Tp>&)):
    Constrain and make transparent.
    (operator<(const optional<_Tp>&, const optional<_Tp>&)): Constrain.
    (operator>(const optional<_Tp>&, const optional<_Tp>&)):
    Constrain and make transparent.
    (operator<=(const optional<_Tp>&, const optional<_Tp>&)): Likewise.
    (operator>=(const optional<_Tp>&, const optional<_Tp>&)): Likewise.
    (operator==(const optional<_Tp>&, const _Tp&): Constrain.
    (operator==(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator!=(const optional<_Tp>&, _Tp const&)):
    Constrain and make transparent.
    (operator!=(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator<(const optional<_Tp>&, const _Tp&)): Constrain.
    (operator<(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator>(const optional<_Tp>&, const _Tp&)):
    Constrain and make transparent.
    (operator>(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator<=(const optional<_Tp>&, const _Tp&)): Likewise.
    (operator<=(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator>=(const optional<_Tp>&, const _Tp&)): Likewise.
    (operator>=(const _Tp&, const optional<_Tp>&)): Likewise.
    * testsuite/20_util/optional/constexpr/relops/2.cc: Adjust.
    * testsuite/20_util/optional/constexpr/relops/4.cc: Likewise.
    * testsuite/20_util/optional/relops/1.cc: Likewise.
    * testsuite/20_util/optional/relops/2.cc: Likewise.
    * testsuite/20_util/optional/relops/3.cc: Likewise.
    * testsuite/20_util/optional/relops/4.cc: Likewise.
    * testsuite/20_util/optional/requirements.cc: Add tests to verify
    that optional's relops are transparent and don't synthesize
    operators. Also test that assignment sfinaes.

Attachment: P0307R2_2.diff
Description: Text document