[v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
Tested on Linux-X64.
The test adjustments are so that the tests are kept valid, which
required adding a bunch of now-required relops to the test types. The new
transparent-but-non-synthesizing aspects of the relops are tested
separately. The constraints are a valid implementation of the
current Requires-clauses on these operators; I will file an LWG
issue suggesting that such Requires-clauses are instead made
SFINAE-constraints like in this implementation. The rationale
for that being that if such "wrapper types" are supposed to be
"transparent", it would be rather good if they are transparent
for SFINAE-querying for the existence of such relops as well, rather
than always report true and then fail to instantiate.
2016-07-11 Ville Voutilainen
Implement P0307R2, Making Optional Greater Equal Again.
* include/std/optional (__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.
diff --git a/libstdc++-v3/include/std/optional
b/libstdc++-v3/include/std/optional
index e9a86a4..1786c2c 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
};
+ template
+using __optional_relop_t =
+enable_if_t::value, bool>;
+
// [X.Y.8] Comparisons between optional values.
template
-constexpr bool
+constexpr auto
operator==(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
+-> __optional_relop_t() == declval<_Tp>())>
{
return static_cast(__lhs) == static_cast(__rhs)
&& (!__lhs || *__lhs == *__rhs);
}
template
-constexpr bool
+constexpr auto
operator!=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-{ return !(__lhs == __rhs); }
+-> __optional_relop_t() != declval<_Tp>())>
+{
+ return static_cast(__lhs) != static_cast(__rhs)
+ || (static_cast(__lhs) && *__lhs != *__rhs);
+}
template
-constexpr bool
+constexpr auto
operator<(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
+-> __optional_relop_t() < declval<_Tp>())>
{
return static_cast(__rhs) && (!__lhs || *__lhs < *__rhs);
}
template
-constexpr bool
+constexpr auto
operator>(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-{ return __rhs < __lhs; }
+-> __optional_relop_t() > declval<_Tp>())>
+{
+ return static_cast(__lhs) && (!__rhs || *__lhs > *__rhs);
+}
template
-constexpr bool
+constexpr auto
operator<=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-{ return !(__rhs < __lhs); }
+-> __optional_relop_t() <= declval<_Tp>())>
+{
+ return !__lhs || (static_cast(__rhs) && *__lhs <= *__rhs);
+}
template
-constexpr bool
+constexpr auto
operator>=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-{ return !(__lhs < __rhs); }
+-> __optional_relop_t() >= declval<_Tp>())>
+{
+ return !__rhs || (static_cast(__lhs) && *__lhs >= *__rhs);
+}
// [X.Y.9] Comparisons with nullopt.
template
@@ -884,64 +903,76 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// [X.Y.10] Comparisons with value type.
template
-constexpr bool
+constexpr auto
ope
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
On 11/07/16 23:41 +0300, Ville Voutilainen wrote: @@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } }; + template +using __optional_relop_t = +enable_if_t::value, bool>; Should this be is_convertible<_Tp, bool> instead? template -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.
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
On 13 July 2016 at 01:31, Jonathan Wakely wrote:
> On 11/07/16 23:41 +0300, Ville Voutilainen wrote:
>>
>> @@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> }
>> };
>>
>> + template
>> +using __optional_relop_t =
>> +enable_if_t::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
>> -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
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.
diff --git a/libstdc++-v3/include/std/optional
b/libstdc++-v3/include/std/optional
index e9a86a4..45929c7 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -132,7 +132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*
* Practically speaking this detects the presence of such an operator when
* called on a const-qualified lvalue (i.e.
-* declval<_Tp * const&>().operator&()).
+* declval().operator&()).
*/
template
struct _Has_addressof
@@ -577,16 +577,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template,
+ is_assignable<_Tp&, _Up>,
__not_>,
__not_<__is_optional<_Up>>>::value,
bool> = true>
optional&
operator=(_Up&& __u)
{
- static_assert(__and_,
- is_assignable<_Tp&, _Up>>(),
-"Cannot assign to value type from argument");
-
if (this->_M_is_engaged())
this->_M_get() = std::forward<_Up>(__u);
else
@@ -597,15 +595,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template>>::value,
- bool> = true>
+ is_constructible<_Tp, _Up>,
+ is_assignable<_Tp&, _Up>,
+ __not_>>::value,
+bool> = true>
optional&
operator=(const optional<_Up>& __u)
{
- static_assert(__and_,
- is_assignable<_Tp&, _Up>>(),
-
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
On 13 July 2016 at 13:05, Ville Voutilainen wrote: >> 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. Well, technically two, the other was in a comment. The patch changes that, too.
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
On 13/07/16 13:05 +0300, Ville Voutilainen wrote: Ha, that was indeed in just one place. See below. 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. operators. Also test that assignment sfinaes. OK. diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index e9a86a4..45929c7 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -132,7 +132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * * Practically speaking this detects the presence of such an operator when * called on a const-qualified lvalue (i.e. -* declval<_Tp * const&>().operator&()). +* declval().operator&()). */ template struct _Has_addressof That comment was wrong anyway, it says _Tp* const& when it should have been _Tp const&. declval<_Tp * const&>().operator&() doesn't make any sense. Not sure why I've never spotted that until now. Please change it to const _Tp& and change "i.e." to "e.g." (because since my change last year it detects both members and non-members). OK for trunk with that tweak, thanks. I'll make the same change to the comment in .
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
2016-07-13 12:05 GMT+02:00 Ville Voutilainen : > On 13 July 2016 at 01:31, Jonathan Wakely wrote: >> On 11/07/16 23:41 +0300, Ville Voutilainen wrote: >>> >>> @@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>> } >>> }; >>> >>> + template >>> +using __optional_relop_t = >>> +enable_if_t::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. How would you feel about the introduction of an internal trait __is_boolean_testable, that would test both is_convertible and is_constructible for now, so that we could reuse that at places like these and others pointed out by LWG 2114? If you like that idea, I would work on a contribution. Thanks, - Daniel
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
On 13 July 2016 at 21:25, Daniel Krügler wrote: > How would you feel about the introduction of an internal trait > __is_boolean_testable, that would test both is_convertible bool> and is_constructible for now, so that we could > reuse that at places like these and others pointed out by LWG 2114? > > If you like that idea, I would work on a contribution. Sounds like an excellent idea to me.
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
2016-07-13 20:32 GMT+02:00 Ville Voutilainen : > On 13 July 2016 at 21:25, Daniel Krügler wrote: >> How would you feel about the introduction of an internal trait >> __is_boolean_testable, that would test both is_convertible> bool> and is_constructible for now, so that we could >> reuse that at places like these and others pointed out by LWG 2114? >> >> If you like that idea, I would work on a contribution. > > Sounds like an excellent idea to me. I have opened an enhancement request and have now a (actually two) proposal(s) for this. I would appreciate if you could express your opinion here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71899#c1 Thanks, - Daniel
