From cdd2d448d8200ed5ebcb232163954367b553291e Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Thu, 5 Nov 2020 18:36:19 +0000 Subject: [PATCH] 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. --- libstdc++-v3/include/std/optional | 66 +++++++++++----- .../20_util/optional/relops/96269.cc | 76 +++++++++++++++++++ 2 files changed, 124 insertions(+), 18 deletions(-) create mode 100644 libstdc++-v3/testsuite/20_util/optional/relops/96269.cc diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index f9f42efe09c..5ea5b39d0e6 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::value, bool>; + template + using __optional_eq_t = __optional_relop_t< + decltype(std::declval() == std::declval()) + >; + + template + using __optional_ne_t = __optional_relop_t< + decltype(std::declval() != std::declval()) + >; + + template + using __optional_lt_t = __optional_relop_t< + decltype(std::declval() < std::declval()) + >; + + template + using __optional_gt_t = __optional_relop_t< + decltype(std::declval() > std::declval()) + >; + + template + using __optional_le_t = __optional_relop_t< + decltype(std::declval() <= std::declval()) + >; + + template + using __optional_ge_t = __optional_relop_t< + decltype(std::declval() >= std::declval()) + >; + // Comparisons between optional values. template constexpr auto operator==(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t() == declval<_Up>())> + -> __optional_eq_t<_Tp, _Up> { return static_cast(__lhs) == static_cast(__rhs) && (!__lhs || *__lhs == *__rhs); @@ -1015,7 +1045,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template constexpr auto operator!=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t() != declval<_Up>())> + -> __optional_ne_t<_Tp, _Up> { return static_cast(__lhs) != static_cast(__rhs) || (static_cast(__lhs) && *__lhs != *__rhs); @@ -1024,7 +1054,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template constexpr auto operator<(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t() < declval<_Up>())> + -> __optional_lt_t<_Tp, _Up> { return static_cast(__rhs) && (!__lhs || *__lhs < *__rhs); } @@ -1032,7 +1062,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template constexpr auto operator>(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t() > declval<_Up>())> + -> __optional_gt_t<_Tp, _Up> { return static_cast(__lhs) && (!__rhs || *__lhs > *__rhs); } @@ -1040,7 +1070,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template constexpr auto operator<=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t() <= declval<_Up>())> + -> __optional_le_t<_Tp, _Up> { return !__lhs || (static_cast(__rhs) && *__lhs <= *__rhs); } @@ -1048,7 +1078,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template constexpr auto operator>=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs) - -> __optional_relop_t() >= declval<_Up>())> + -> __optional_ge_t<_Tp, _Up> { return !__rhs || (static_cast(__lhs) && *__lhs >= *__rhs); } @@ -1134,73 +1164,73 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template constexpr auto operator==(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t() == declval<_Up>())> + -> __optional_eq_t<_Tp, _Up> { return __lhs && *__lhs == __rhs; } template constexpr auto operator==(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t() == declval<_Tp>())> + -> __optional_eq_t<_Up, _Tp> { return __rhs && __lhs == *__rhs; } template constexpr auto operator!=(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t() != declval<_Up>())> + -> __optional_ne_t<_Tp, _Up> { return !__lhs || *__lhs != __rhs; } template constexpr auto operator!=(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t() != declval<_Tp>())> + -> __optional_ne_t<_Up, _Tp> { return !__rhs || __lhs != *__rhs; } template constexpr auto operator<(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t() < declval<_Up>())> + -> __optional_lt_t<_Tp, _Up> { return !__lhs || *__lhs < __rhs; } template constexpr auto operator<(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t() < declval<_Tp>())> + -> __optional_lt_t<_Up, _Tp> { return __rhs && __lhs < *__rhs; } template constexpr auto operator>(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t() > declval<_Up>())> + -> __optional_gt_t<_Tp, _Up> { return __lhs && *__lhs > __rhs; } template constexpr auto operator>(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t() > declval<_Tp>())> + -> __optional_gt_t<_Up, _Tp> { return !__rhs || __lhs > *__rhs; } template constexpr auto operator<=(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t() <= declval<_Up>())> + -> __optional_le_t<_Tp, _Up> { return !__lhs || *__lhs <= __rhs; } template constexpr auto operator<=(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t() <= declval<_Tp>())> + -> __optional_le_t<_Up, _Tp> { return __rhs && __lhs <= *__rhs; } template constexpr auto operator>=(const optional<_Tp>& __lhs, const _Up& __rhs) - -> __optional_relop_t() >= declval<_Up>())> + -> __optional_ge_t<_Tp, _Up> { return __lhs && *__lhs >= __rhs; } template constexpr auto operator>=(const _Up& __lhs, const optional<_Tp>& __rhs) - -> __optional_relop_t() >= 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 00000000000..2054d3643c6 --- /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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include + +struct X +{ + template + bool operator==(const T&) /* not const */ { return false; } + + template + bool operator!=(const T&) /* not const */ { return false; } + + template + bool operator<(const T&) /* not const */ { return false; } + + template + bool operator>(const T&) /* not const */ { return false; } + + template + bool operator<=(const T&) /* not const */ { return false; } + + template + bool operator>=(const T&) /* not const */ { return false; } +}; + +void test01() +{ + // PR 96269 optional comparison with nullopt fails + std::optional 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 + concept optional_lt_cmp + = requires(std::optional o, T t) { { o < t } -> std::same_as; }; + +template + concept optional_gt_cmp + = requires(std::optional o, T t) { { o > t } -> std::same_as; }; + +template + concept optional_le_cmp + = requires(std::optional o, T t) { { o <= t } -> std::same_as; }; + +template + concept optional_ge_cmp + = requires(std::optional o, T t) { { o >= t } -> std::same_as; }; + +static_assert( ! optional_lt_cmp ); +static_assert( ! optional_gt_cmp ); +static_assert( ! optional_le_cmp ); +static_assert( ! optional_ge_cmp ); -- 2.30.2