The relational operators for std::optional were using the wrong types in the declval expressions used to constrain them. Instead of using const lvalues they were using non-const rvalues, which meant that a type might satisfy the constraints but then give an error when the function body was instantiated.
libstdc++-v3/ChangeLog: PR libstdc++/96269 * include/std/optional (operator==, operator!=, operator<) (operator>, operator<=, operator>=): Fix types used in SFINAE constraints. * testsuite/20_util/optional/relops/96269.cc: New test. Tested powerpc64le-linux. Committed to trunk. I'll backport to gcc-10 too.
commit cdd2d448d8200ed5ebcb232163954367b553291e Author: Jonathan Wakely <jwak...@redhat.com> Date: Thu Nov 5 18:36:19 2020 libstdc++: Fix constraints on std::optional comparisons [PR 96269] The relational operators for std::optional were using the wrong types in the declval expressions used to constrain them. Instead of using const lvalues they were using non-const rvalues, which meant that a type might satisfy the constraints but then give an error when the function body was instantiated. libstdc++-v3/ChangeLog: PR libstdc++/96269 * include/std/optional (operator==, operator!=, operator<) (operator>, operator<=, operator>=): Fix types used in SFINAE constraints. * testsuite/20_util/optional/relops/96269.cc: New test. diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index f9f42efe09ce..5ea5b39d0e69 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -1002,11 +1002,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using __optional_relop_t = enable_if_t<is_convertible<_Tp, bool>::value, bool>; + template<typename _Tp, typename _Up> + using __optional_eq_t = __optional_relop_t< + decltype(std::declval<const _Tp&>() == std::declval<const _Up&>()) + >; + + template<typename _Tp, typename _Up> + using __optional_ne_t = __optional_relop_t< + decltype(std::declval<const _Tp&>() != std::declval<const _Up&>()) + >; + + template<typename _Tp, typename _Up> + using __optional_lt_t = __optional_relop_t< + decltype(std::declval<const _Tp&>() < std::declval<const _Up&>()) + >; + + template<typename _Tp, typename _Up> + using __optional_gt_t = __optional_relop_t< + decltype(std::declval<const _Tp&>() > std::declval<const _Up&>()) + >; + + template<typename _Tp, typename _Up> + using __optional_le_t = __optional_relop_t< + decltype(std::declval<const _Tp&>() <= std::declval<const _Up&>()) + >; + + template<typename _Tp, typename _Up> + using __optional_ge_t = __optional_relop_t< + decltype(std::declval<const _Tp&>() >= std::declval<const _Up&>()) + >; + // Comparisons between optional values. template<typename _Tp, typename _Up> constexpr auto operator==(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Up>())> + -> __optional_eq_t<_Tp, _Up> { return static_cast<bool>(__lhs) == static_cast<bool>(__rhs) && (!__lhs || *__lhs == *__rhs); @@ -1015,7 +1045,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, typename _Up> constexpr auto operator!=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Up>())> + -> __optional_ne_t<_Tp, _Up> { return static_cast<bool>(__lhs) != static_cast<bool>(__rhs) || (static_cast<bool>(__lhs) && *__lhs != *__rhs); @@ -1024,7 +1054,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, typename _Up> constexpr auto operator<(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Up>())> + -> __optional_lt_t<_Tp, _Up> { return static_cast<bool>(__rhs) && (!__lhs || *__lhs < *__rhs); } @@ -1032,7 +1062,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, typename _Up> constexpr auto operator>(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Up>())> + -> __optional_gt_t<_Tp, _Up> { return static_cast<bool>(__lhs) && (!__rhs || *__lhs > *__rhs); } @@ -1040,7 +1070,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, typename _Up> constexpr auto operator<=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Up>())> + -> __optional_le_t<_Tp, _Up> { return !__lhs || (static_cast<bool>(__rhs) && *__lhs <= *__rhs); } @@ -1048,7 +1078,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, typename _Up> constexpr auto operator>=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Up>())> + -> __optional_ge_t<_Tp, _Up> { return !__rhs || (static_cast<bool>(__lhs) && *__lhs >= *__rhs); } @@ -1134,73 +1164,73 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, typename _Up> constexpr auto operator==(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Up>())> + -> __optional_eq_t<_Tp, _Up> { return __lhs && *__lhs == __rhs; } template<typename _Tp, typename _Up> constexpr auto operator==(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t<decltype(declval<_Up>() == declval<_Tp>())> + -> __optional_eq_t<_Up, _Tp> { return __rhs && __lhs == *__rhs; } template<typename _Tp, typename _Up> constexpr auto operator!=(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Up>())> + -> __optional_ne_t<_Tp, _Up> { return !__lhs || *__lhs != __rhs; } template<typename _Tp, typename _Up> constexpr auto operator!=(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t<decltype(declval<_Up>() != declval<_Tp>())> + -> __optional_ne_t<_Up, _Tp> { return !__rhs || __lhs != *__rhs; } template<typename _Tp, typename _Up> constexpr auto operator<(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Up>())> + -> __optional_lt_t<_Tp, _Up> { return !__lhs || *__lhs < __rhs; } template<typename _Tp, typename _Up> constexpr auto operator<(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t<decltype(declval<_Up>() < declval<_Tp>())> + -> __optional_lt_t<_Up, _Tp> { return __rhs && __lhs < *__rhs; } template<typename _Tp, typename _Up> constexpr auto operator>(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Up>())> + -> __optional_gt_t<_Tp, _Up> { return __lhs && *__lhs > __rhs; } template<typename _Tp, typename _Up> constexpr auto operator>(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t<decltype(declval<_Up>() > declval<_Tp>())> + -> __optional_gt_t<_Up, _Tp> { return !__rhs || __lhs > *__rhs; } template<typename _Tp, typename _Up> constexpr auto operator<=(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Up>())> + -> __optional_le_t<_Tp, _Up> { return !__lhs || *__lhs <= __rhs; } template<typename _Tp, typename _Up> constexpr auto operator<=(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t<decltype(declval<_Up>() <= declval<_Tp>())> + -> __optional_le_t<_Up, _Tp> { return __rhs && __lhs <= *__rhs; } template<typename _Tp, typename _Up> constexpr auto operator>=(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Up>())> + -> __optional_ge_t<_Tp, _Up> { return __lhs && *__lhs >= __rhs; } template<typename _Tp, typename _Up> constexpr auto operator>=(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t<decltype(declval<_Up>() >= declval<_Tp>())> + -> __optional_ge_t<_Up, _Tp> { return !__rhs || __lhs >= *__rhs; } #ifdef __cpp_lib_three_way_comparison diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/96269.cc b/libstdc++-v3/testsuite/20_util/optional/relops/96269.cc new file mode 100644 index 000000000000..2054d3643c66 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/optional/relops/96269.cc @@ -0,0 +1,76 @@ +// Copyright (C) 2020 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include <optional> + +struct X +{ + template <typename T> + bool operator==(const T&) /* not const */ { return false; } + + template <typename T> + bool operator!=(const T&) /* not const */ { return false; } + + template <typename T> + bool operator<(const T&) /* not const */ { return false; } + + template <typename T> + bool operator>(const T&) /* not const */ { return false; } + + template <typename T> + bool operator<=(const T&) /* not const */ { return false; } + + template <typename T> + bool operator>=(const T&) /* not const */ { return false; } +}; + +void test01() +{ + // PR 96269 optional comparison with nullopt fails + std::optional<X> x; + bool eq [[maybe_unused]] = std::nullopt == x; + + bool ne [[maybe_unused]] = std::nullopt != x; + bool lt [[maybe_unused]] = std::nullopt < x; + bool gt [[maybe_unused]] = std::nullopt > x; + bool le [[maybe_unused]] = std::nullopt <= x; + bool ge [[maybe_unused]] = std::nullopt >= x; +} + +template<typename T> + concept optional_lt_cmp + = requires(std::optional<T> o, T t) { { o < t } -> std::same_as<bool>; }; + +template<typename T> + concept optional_gt_cmp + = requires(std::optional<T> o, T t) { { o > t } -> std::same_as<bool>; }; + +template<typename T> + concept optional_le_cmp + = requires(std::optional<T> o, T t) { { o <= t } -> std::same_as<bool>; }; + +template<typename T> + concept optional_ge_cmp + = requires(std::optional<T> o, T t) { { o >= t } -> std::same_as<bool>; }; + +static_assert( ! optional_lt_cmp<X> ); +static_assert( ! optional_gt_cmp<X> ); +static_assert( ! optional_le_cmp<X> ); +static_assert( ! optional_ge_cmp<X> );