From f90311d1741db93da591b6381747e7a70487c0cc Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Wed, 6 Sep 2023 19:00:20 +0800 Subject: [PATCH 1/5] fix `expected` and `expected` --- stl/inc/expected | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/stl/inc/expected b/stl/inc/expected index 465a1616e96..f7e4ba58f75 100644 --- a/stl/inc/expected +++ b/stl/inc/expected @@ -237,6 +237,9 @@ public: is_trivially_move_constructible_v<_Ty> && is_trivially_move_constructible_v<_Err> = default; // clang-format on + template + static constexpr bool _Not_self = !is_same_v<_Ty, _Uty> || !is_same_v<_Err, _UErr>; + template static constexpr bool _Allow_unwrapping = disjunction_v, bool>, negation&>, // @@ -253,8 +256,8 @@ public: && !is_constructible_v, const expected<_Uty, _UErr>>; template - requires is_constructible_v<_Ty, const _Uty&> && is_constructible_v<_Err, const _UErr&> - && _Allow_unwrapping<_Uty, _UErr> + requires _Not_self<_Uty, _UErr> && is_constructible_v<_Ty, const _Uty&> + && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v || !is_convertible_v) expected(const expected<_Uty, _UErr>& _Other) noexcept(is_nothrow_constructible_v<_Ty, const _Uty&> // && is_nothrow_constructible_v<_Err, const _UErr&>) // strengthened @@ -267,7 +270,8 @@ public: } template - requires is_constructible_v<_Ty, _Uty> && is_constructible_v<_Err, _UErr> && _Allow_unwrapping<_Uty, _UErr> + requires _Not_self<_Uty, _UErr> && is_constructible_v<_Ty, _Uty> && is_constructible_v<_Err, _UErr> + && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v<_Uty, _Ty> || !is_convertible_v<_UErr, _Err>) expected(expected<_Uty, _UErr>&& _Other) noexcept( is_nothrow_constructible_v<_Ty, _Uty>&& is_nothrow_constructible_v<_Err, _UErr>) // strengthened @@ -342,7 +346,7 @@ public: } // clang-format off - ~expected() requires is_trivially_destructible_v<_Ty> && is_trivially_destructible_v<_Err> = default; + ~expected() requires is_trivially_destructible_v<_Ty>&& is_trivially_destructible_v<_Err> = default; // clang-format on // [expected.object.assign] @@ -1201,6 +1205,9 @@ public: expected(expected&&) requires is_trivially_move_constructible_v<_Err> = default; // clang-format on + template + static constexpr bool _Not_self = !is_same_v<_Ty, _Uty> || !is_same_v<_Err, _UErr>; + template static constexpr bool _Allow_unwrapping = !is_constructible_v, expected<_Uty, _UErr>&> && !is_constructible_v, expected<_Uty, _UErr>> @@ -1208,7 +1215,8 @@ public: && !is_constructible_v, const expected<_Uty, _UErr>>; template - requires is_void_v<_Uty> && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr> + requires _Not_self<_Uty, _UErr> && is_void_v<_Uty> && is_constructible_v<_Err, const _UErr&> + && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v) expected(const expected<_Uty, _UErr>& _Other) noexcept( is_nothrow_constructible_v<_Err, const _UErr&>) // strengthened : _Has_value(_Other._Has_value) { @@ -1218,7 +1226,8 @@ public: } template - requires is_void_v<_Uty> && is_constructible_v<_Err, _UErr> && _Allow_unwrapping<_Uty, _UErr> + requires _Not_self<_Uty, _UErr> && is_void_v<_Uty> && is_constructible_v<_Err, _UErr> + && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v<_UErr, _Err>) expected(expected<_Uty, _UErr>&& _Other) noexcept(is_nothrow_constructible_v<_Err, _UErr>) // strengthened : _Has_value(_Other._Has_value) { From e156420e0e3a2cd6a3a99886c07a21fa00a88337 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Wed, 6 Sep 2023 20:07:01 +0800 Subject: [PATCH 2/5] add test --- stl/inc/expected | 2 +- tests/std/tests/P0323R12_expected/test.cpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/stl/inc/expected b/stl/inc/expected index f7e4ba58f75..f5e347c767f 100644 --- a/stl/inc/expected +++ b/stl/inc/expected @@ -346,7 +346,7 @@ public: } // clang-format off - ~expected() requires is_trivially_destructible_v<_Ty>&& is_trivially_destructible_v<_Err> = default; + ~expected() requires is_trivially_destructible_v<_Ty> && is_trivially_destructible_v<_Err> = default; // clang-format on // [expected.object.assign] diff --git a/tests/std/tests/P0323R12_expected/test.cpp b/tests/std/tests/P0323R12_expected/test.cpp index 91214c634fd..c4ddf55dc17 100644 --- a/tests/std/tests/P0323R12_expected/test.cpp +++ b/tests/std/tests/P0323R12_expected/test.cpp @@ -3,6 +3,7 @@ #define _CONTAINER_DEBUG_LEVEL 1 +#include #include #include #include @@ -2135,6 +2136,11 @@ void test_lwg_3843() { } } +void test_gh_4011() { + static_assert(copyable>); + static_assert(copyable>); +} + int main() { test_unexpected::test_all(); static_assert(test_unexpected::test_all()); @@ -2151,4 +2157,5 @@ int main() { test_reinit_regression(); test_lwg_3843(); + test_gh_4011(); } From 7c1ed3d3f058f8c8552bffc0a047c57333971d16 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Wed, 6 Sep 2023 22:59:20 +0800 Subject: [PATCH 3/5] hide `_Not_self` and `_Allow_unwrapping`; simplify `friend swap(expected)` --- stl/inc/expected | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/stl/inc/expected b/stl/inc/expected index f5e347c767f..7936d00519f 100644 --- a/stl/inc/expected +++ b/stl/inc/expected @@ -237,6 +237,7 @@ public: is_trivially_move_constructible_v<_Ty> && is_trivially_move_constructible_v<_Err> = default; // clang-format on +private: template static constexpr bool _Not_self = !is_same_v<_Ty, _Uty> || !is_same_v<_Err, _UErr>; @@ -255,6 +256,7 @@ public: && !is_constructible_v, const expected<_Uty, _UErr>&> // && !is_constructible_v, const expected<_Uty, _UErr>>; +public: template requires _Not_self<_Uty, _UErr> && is_constructible_v<_Ty, const _Uty&> && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr> @@ -1205,6 +1207,7 @@ public: expected(expected&&) requires is_trivially_move_constructible_v<_Err> = default; // clang-format on +private: template static constexpr bool _Not_self = !is_same_v<_Ty, _Uty> || !is_same_v<_Err, _UErr>; @@ -1214,6 +1217,7 @@ public: && !is_constructible_v, const expected<_Uty, _UErr>&> && !is_constructible_v, const expected<_Uty, _UErr>>; +public: template requires _Not_self<_Uty, _UErr> && is_void_v<_Uty> && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr> @@ -1389,26 +1393,7 @@ public: is_nothrow_move_constructible_v<_Err>&& is_nothrow_swappable_v<_Err>) requires is_swappable_v<_Err> && is_move_constructible_v<_Err> { - using _STD swap; - if (_Left._Has_value && _Right._Has_value) { - // nothing - } else if (_Left._Has_value) { - _STD construct_at(_STD addressof(_Left._Unexpected), _STD move(_Right._Unexpected)); - if constexpr (!is_trivially_destructible_v<_Err>) { - _Right._Unexpected.~_Err(); - } - _Left._Has_value = false; - _Right._Has_value = true; - } else if (_Right._Has_value) { - _STD construct_at(_STD addressof(_Right._Unexpected), _STD move(_Left._Unexpected)); - if constexpr (!is_trivially_destructible_v<_Err>) { - _Left._Unexpected.~_Err(); - } - _Left._Has_value = true; - _Right._Has_value = false; - } else { - swap(_Left._Unexpected, _Right._Unexpected); // intentional ADL - } + _Left.swap(_Right); } // [expected.void.obs] From 39f87ed319bcfb6eaf62550afbffd6b5aed8391c Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 6 Sep 2023 11:17:37 -0700 Subject: [PATCH 4/5] Casey's review comments --- stl/inc/expected | 64 +++++++++------------- tests/std/tests/P0323R12_expected/test.cpp | 8 +-- 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/stl/inc/expected b/stl/inc/expected index 7936d00519f..727601d326c 100644 --- a/stl/inc/expected +++ b/stl/inc/expected @@ -190,6 +190,21 @@ class expected { template friend class expected; + template + static constexpr bool _Allow_unwrapping = disjunction_v, bool>, + negation&>, // + is_constructible<_Ty, expected<_Uty, _UErr>>, // + is_constructible<_Ty, const expected<_Uty, _UErr>&>, // + is_constructible<_Ty, const expected<_Uty, _UErr>>, // + is_convertible&, _Ty>, // + is_convertible&&, _Ty>, // + is_convertible&, _Ty>, // + is_convertible&&, _Ty>>>> // + && !is_constructible_v, expected<_Uty, _UErr>&> // + && !is_constructible_v, expected<_Uty, _UErr>> // + && !is_constructible_v, const expected<_Uty, _UErr>&> // + && !is_constructible_v, const expected<_Uty, _UErr>>; + public: using value_type = _Ty; using error_type = _Err; @@ -237,28 +252,8 @@ public: is_trivially_move_constructible_v<_Ty> && is_trivially_move_constructible_v<_Err> = default; // clang-format on -private: - template - static constexpr bool _Not_self = !is_same_v<_Ty, _Uty> || !is_same_v<_Err, _UErr>; - template - static constexpr bool _Allow_unwrapping = disjunction_v, bool>, - negation&>, // - is_constructible<_Ty, expected<_Uty, _UErr>>, // - is_constructible<_Ty, const expected<_Uty, _UErr>&>, // - is_constructible<_Ty, const expected<_Uty, _UErr>>, // - is_convertible&, _Ty>, // - is_convertible&&, _Ty>, // - is_convertible&, _Ty>, // - is_convertible&&, _Ty>>>> // - && !is_constructible_v, expected<_Uty, _UErr>&> // - && !is_constructible_v, expected<_Uty, _UErr>> // - && !is_constructible_v, const expected<_Uty, _UErr>&> // - && !is_constructible_v, const expected<_Uty, _UErr>>; - -public: - template - requires _Not_self<_Uty, _UErr> && is_constructible_v<_Ty, const _Uty&> + requires _Different_from, expected> && is_constructible_v<_Ty, const _Uty&> && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v || !is_convertible_v) expected(const expected<_Uty, _UErr>& _Other) noexcept(is_nothrow_constructible_v<_Ty, const _Uty&> // @@ -272,8 +267,8 @@ public: } template - requires _Not_self<_Uty, _UErr> && is_constructible_v<_Ty, _Uty> && is_constructible_v<_Err, _UErr> - && _Allow_unwrapping<_Uty, _UErr> + requires _Different_from, expected> && is_constructible_v<_Ty, _Uty> + && is_constructible_v<_Err, _UErr> && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v<_Uty, _Ty> || !is_convertible_v<_UErr, _Err>) expected(expected<_Uty, _UErr>&& _Other) noexcept( is_nothrow_constructible_v<_Ty, _Uty>&& is_nothrow_constructible_v<_Err, _UErr>) // strengthened @@ -1172,6 +1167,12 @@ class expected<_Ty, _Err> { template friend class expected; + template + static constexpr bool _Allow_unwrapping = !is_constructible_v, expected<_Uty, _UErr>&> + && !is_constructible_v, expected<_Uty, _UErr>> + && !is_constructible_v, const expected<_Uty, _UErr>&> + && !is_constructible_v, const expected<_Uty, _UErr>>; + public: using value_type = _Ty; using error_type = _Err; @@ -1207,20 +1208,9 @@ public: expected(expected&&) requires is_trivially_move_constructible_v<_Err> = default; // clang-format on -private: - template - static constexpr bool _Not_self = !is_same_v<_Ty, _Uty> || !is_same_v<_Err, _UErr>; - template - static constexpr bool _Allow_unwrapping = !is_constructible_v, expected<_Uty, _UErr>&> - && !is_constructible_v, expected<_Uty, _UErr>> - && !is_constructible_v, const expected<_Uty, _UErr>&> - && !is_constructible_v, const expected<_Uty, _UErr>>; - -public: - template - requires _Not_self<_Uty, _UErr> && is_void_v<_Uty> && is_constructible_v<_Err, const _UErr&> - && _Allow_unwrapping<_Uty, _UErr> + requires _Different_from, expected> && is_void_v<_Uty> + && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v) expected(const expected<_Uty, _UErr>& _Other) noexcept( is_nothrow_constructible_v<_Err, const _UErr&>) // strengthened : _Has_value(_Other._Has_value) { @@ -1230,7 +1220,7 @@ public: } template - requires _Not_self<_Uty, _UErr> && is_void_v<_Uty> && is_constructible_v<_Err, _UErr> + requires _Different_from, expected> && is_void_v<_Uty> && is_constructible_v<_Err, _UErr> && _Allow_unwrapping<_Uty, _UErr> constexpr explicit(!is_convertible_v<_UErr, _Err>) expected(expected<_Uty, _UErr>&& _Other) noexcept(is_nothrow_constructible_v<_Err, _UErr>) // strengthened diff --git a/tests/std/tests/P0323R12_expected/test.cpp b/tests/std/tests/P0323R12_expected/test.cpp index c4ddf55dc17..5b822c1c57c 100644 --- a/tests/std/tests/P0323R12_expected/test.cpp +++ b/tests/std/tests/P0323R12_expected/test.cpp @@ -2136,10 +2136,9 @@ void test_lwg_3843() { } } -void test_gh_4011() { - static_assert(copyable>); - static_assert(copyable>); -} +// Test GH-4011: these predicates triggered constraint recursion. +static_assert(copyable>); +static_assert(copyable>); int main() { test_unexpected::test_all(); @@ -2157,5 +2156,4 @@ int main() { test_reinit_regression(); test_lwg_3843(); - test_gh_4011(); } From 64b824e63f12a0ade5a78355fce0a40dba62403f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 6 Sep 2023 13:17:22 -0700 Subject: [PATCH 5/5] Explicitly say `private:`. --- stl/inc/expected | 2 ++ 1 file changed, 2 insertions(+) diff --git a/stl/inc/expected b/stl/inc/expected index 727601d326c..12b06882c2f 100644 --- a/stl/inc/expected +++ b/stl/inc/expected @@ -184,6 +184,7 @@ struct _Check_expected_argument : true_type { _EXPORT_STD template class expected { +private: static_assert(_Check_expected_argument<_Ty>::value); static_assert(_Check_unexpected_argument<_Err>::value); @@ -1162,6 +1163,7 @@ private: template requires is_void_v<_Ty> class expected<_Ty, _Err> { +private: static_assert(_Check_unexpected_argument<_Err>::value); template