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.

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

expected (rev9): Omissions and mistakes #42

Open
ecorm opened this issue Oct 6, 2020 · 0 comments
Open

expected (rev9): Omissions and mistakes #42

ecorm opened this issue Oct 6, 2020 · 0 comments

Comments

@ecorm
Copy link

ecorm commented Oct 6, 2020

Under Section 1.6

  1. template<class U, class G> explicit(see below) constexpr expected(const expected<U, G>& rhs) : Are the constraints involving E and G still applicable when T and U are cv void?

  2. template<class U, class G> explicit(see below) constexpr expected(expected<U, G>&& rhs) : Are the constraints involving E and G still applicable when T and U are cv void?

  3. template<class... Args> constexpr explicit expected(unexpect_t, Args&&... args); : Should be "with the arguments in_place, std::forward<Args>(args)...."

  4. template<class U, class... Args> constexpr explicit expected(unexpect_t, initializer_list<U> il, Args&&... args); : Should be "with the arguments in_place, il, std::forward<Args>(args)...."

Under Section 1.8

  1. expected& operator=(const expected& rhs) noexcept(see below); : noexcept specification does not match the one in the class synopsis.

  2. expected& operator=(expected&& rhs) noexcept(see below);

    • Missing std::move, should be

    move constructs an unexpected_type<E> tmp from unexpected(std::move(this->error())) (which can’t throw as E is nothrow-move-constructible)

    • For !bool(*this) && bool(rhs), missing "and set has_val to true"

    • Constraint for when T is cv void should be is_move_constructible_v<E> is true and is_move_assignable_v<E> is true.

    • Constraint for when T is not cv void should be (determined by following the logic in the table):
      is_move_constructible_v<T> && is_move_assignable_v<T> && is_move_constructible_v<E> && is_move_assignable_v<E> && (is_nothrow_move_constructible_v<E> || is_nothrow_move_constructible_v<T>)

    • The expression inside noexcept should be equivalent to:
      is_nothrow_move_assignable_v<T> && is_nothrow_move_constructible_v<T> && is_nothrow_move_assignable_v<E> && is_nothrow_move_constructible_v<E>

  3. template<class G = E> expected<T, E>& operator=(const unexpected<G>& e); : Table is inverted

  4. expected<T, E>& operator=(const unexpected<G>& e); and expected<T, E>& operator=(unexpected<G>&& e);

    • Should be e.value() instead of e.error().

    • Temporary unexpected<E> should be created from unexpected<G> before destroying val to ensure strong exception guarantee.

  5. expected<T, E>& operator=(unexpected<G>&& e);, for !bool(*this) : Missing std::move, should be "move assign unexpected(std::move(e).value()) to unex".

  6. template<class U = T> expected<T, E>& operator=(U&& v); : Missing std::move, should be

move constructs an unexpected<E> tmp from unexpected(std::move(this->error())) (which can’t throw as E is nothrow-move-constructible)

  1. Why are there no assignment operator overloads taking expected<U, G>? There are after all assignment operator overloads for std::optional taking std::optional<U>.

  2. template<class... Args> T& emplace(Args&&... args); : By following the logic under Effects, the constraint should be is_constructible_v<T, Args...> && (is_nothrow_constructible_v<T, Args...> || is_nothrow_move_constructible_v<T> || is_nothrow_move_constructible_v<E>). Upon exception, one or more arguments may be left in a post-moved state, so the "nothing changes" remark is not completely accurate.

  3. template<class U, class... Args> T& emplace(initializer_list<U> il, Args&&... args); : By following the logic under Effects, the constraint should be is_constructible_v<T, L, Args...> && (is_nothrow_constructible_v<T, L, Args...> || is_nothrow_move_constructible_v<T> || is_nothrow_move_constructible_v<E>) where L is std::initializer_list<U>. Upon exception, one or more arguments may be left in a post-moved state, so the "nothing changes" remark is not completely accurate.

Under Section 1.9

  1. Constraints should be is_nothrow_move_constructible_v, not is_move_constructible_v.

  2. In table, for bool(*this) && !bool(rhs) and is cv void, should be unexpected(std::move(rhs.error())) (missing .error())

Under Section 1.10

  1. constexpr T&& expected::value() && and constexpr const T&& expected::value() const&& : Should they throw bad_expected_access(std::move(*this).error()) instead, in order to allow move-only error types?

  2. value_or : Behavior and/or constraints need to be clarified when T is void. Does it ignore the argument and do nothing, or is it disabled entirely?

Under Section 1.12

  1. operator!=: Should be "expression *x != v is well-formed.

  2. operator!=: Should be "Returns: bool(x) ? *x != v : true;. If x holds an error, than is it unequal to any value v. Otherwise, !( x != v) would not be the same as x == v.

Under Section 1.14

  1. Constraints don't match those for member swap.

Under Section 1.17.1

  1. Should be Err in constraints, instead of U.

  2. template<class Err> constexpr explicit unexpected(Err&& e) : Should be unexpect_t in constraints, not unexpected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant