Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid constraint recursion for expected<any, ...> and expected<void, any> #4013

Merged
merged 5 commits into from
Sep 7, 2023
Merged

Avoid constraint recursion for expected<any, ...> and expected<void, any> #4013

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f90311d
fix `expected<any,...>` and `expected<void,any>`
achabense Sep 6, 2023
e156420
add test
achabense Sep 6, 2023
7c1ed3d
hide `_Not_self` and `_Allow_unwrapping`; simplify `friend swap(expec…
achabense Sep 6, 2023
39f87ed
Casey's review comments
CaseyCarter Sep 6, 2023
64b824e
Explicitly say `private:`.
StephanTLavavej Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 30 additions & 46 deletions stl/inc/expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,21 @@ class expected {
template <class _UTy, class _UErr>
friend class expected;

template <class _Uty, class _UErr>
static constexpr bool _Allow_unwrapping = disjunction_v<is_same<remove_cv_t<_Ty>, bool>,
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
negation<disjunction<is_constructible<_Ty, expected<_Uty, _UErr>&>, //
is_constructible<_Ty, expected<_Uty, _UErr>>, //
is_constructible<_Ty, const expected<_Uty, _UErr>&>, //
is_constructible<_Ty, const expected<_Uty, _UErr>>, //
is_convertible<expected<_Uty, _UErr>&, _Ty>, //
is_convertible<expected<_Uty, _UErr>&&, _Ty>, //
is_convertible<const expected<_Uty, _UErr>&, _Ty>, //
is_convertible<const expected<_Uty, _UErr>&&, _Ty>>>> //
&& !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>&> //
&& !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>> //
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>&> //
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>>;

public:
using value_type = _Ty;
using error_type = _Err;
Expand Down Expand Up @@ -238,23 +253,8 @@ public:
// clang-format on

template <class _Uty, class _UErr>
static constexpr bool _Allow_unwrapping = disjunction_v<is_same<remove_cv_t<_Ty>, bool>,
negation<disjunction<is_constructible<_Ty, expected<_Uty, _UErr>&>, //
is_constructible<_Ty, expected<_Uty, _UErr>>, //
is_constructible<_Ty, const expected<_Uty, _UErr>&>, //
is_constructible<_Ty, const expected<_Uty, _UErr>>, //
is_convertible<expected<_Uty, _UErr>&, _Ty>, //
is_convertible<expected<_Uty, _UErr>&&, _Ty>, //
is_convertible<const expected<_Uty, _UErr>&, _Ty>, //
is_convertible<const expected<_Uty, _UErr>&&, _Ty>>>> //
&& !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>&> //
&& !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>> //
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>&> //
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>>;
Copy link
Contributor Author

@achabense achabense Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my untimely reply; if it's late to make new pushes and these changes look ok, I will carry these changes to another pr.

  1. I think it's beneficial not to relocate _Allow_unwrapping here and in expected<void, ...>, as it is used closely after definition so its function can be more obvious.
  2. These can also be private:

    STL/stl/inc/expected

    Line 351 in 6c69a73

    struct _NODISCARD _GuardTy {

    STL/stl/inc/expected

    Line 363 in 6c69a73

    static constexpr void _Reinit_expected(_First& _New_val, _Second& _Old_val, _Args&&... _Vals) noexcept(


template <class _Uty, class _UErr>
requires is_constructible_v<_Ty, const _Uty&> && is_constructible_v<_Err, const _UErr&>
&& _Allow_unwrapping<_Uty, _UErr>
requires _Different_from<expected<_Uty, _UErr>, expected> && is_constructible_v<_Ty, const _Uty&>
&& is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr>
constexpr explicit(!is_convertible_v<const _Uty&, _Ty> || !is_convertible_v<const _UErr&, _Err>)
expected(const expected<_Uty, _UErr>& _Other) noexcept(is_nothrow_constructible_v<_Ty, const _Uty&> //
&& is_nothrow_constructible_v<_Err, const _UErr&>) // strengthened
Expand All @@ -267,7 +267,8 @@ public:
}

template <class _Uty, class _UErr>
requires is_constructible_v<_Ty, _Uty> && is_constructible_v<_Err, _UErr> && _Allow_unwrapping<_Uty, _UErr>
requires _Different_from<expected<_Uty, _UErr>, 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
Expand Down Expand Up @@ -1166,6 +1167,12 @@ class expected<_Ty, _Err> {
template <class _UTy, class _UErr>
friend class expected;

template <class _Uty, class _UErr>
static constexpr bool _Allow_unwrapping = !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>&>
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
&& !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>>
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>&>
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>>;

public:
using value_type = _Ty;
using error_type = _Err;
Expand Down Expand Up @@ -1202,13 +1209,8 @@ public:
// clang-format on

template <class _Uty, class _UErr>
static constexpr bool _Allow_unwrapping = !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>&>
&& !is_constructible_v<unexpected<_Err>, expected<_Uty, _UErr>>
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>&>
&& !is_constructible_v<unexpected<_Err>, const expected<_Uty, _UErr>>;

template <class _Uty, class _UErr>
requires is_void_v<_Uty> && is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr>
requires _Different_from<expected<_Uty, _UErr>, expected> && is_void_v<_Uty>
&& is_constructible_v<_Err, const _UErr&> && _Allow_unwrapping<_Uty, _UErr>
constexpr explicit(!is_convertible_v<const _UErr&, _Err>) expected(const expected<_Uty, _UErr>& _Other) noexcept(
is_nothrow_constructible_v<_Err, const _UErr&>) // strengthened
: _Has_value(_Other._Has_value) {
Expand All @@ -1218,7 +1220,8 @@ public:
}

template <class _Uty, class _UErr>
requires is_void_v<_Uty> && is_constructible_v<_Err, _UErr> && _Allow_unwrapping<_Uty, _UErr>
requires _Different_from<expected<_Uty, _UErr>, 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
: _Has_value(_Other._Has_value) {
Expand Down Expand Up @@ -1380,26 +1383,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]
Expand Down
5 changes: 5 additions & 0 deletions tests/std/tests/P0323R12_expected/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#define _CONTAINER_DEBUG_LEVEL 1

#include <any>
#include <cassert>
#include <concepts>
#include <exception>
Expand Down Expand Up @@ -2135,6 +2136,10 @@ void test_lwg_3843() {
}
}

// Test GH-4011: these predicates triggered constraint recursion.
static_assert(copyable<expected<any, int>>);
static_assert(copyable<expected<void, any>>);

int main() {
test_unexpected::test_all();
static_assert(test_unexpected::test_all());
Expand Down