On Sun, Jan 2, 2022 at 7:42 AM Desmond Gold Bongcawel via Std-Proposals <
std-proposals_at_[hidden]> wrote:

> While reading this paper https://wg21.link/P2505, I have found out some
> "ambiguity" specifically in wording "or_else" member function in both
> primary template and void partial specialization of std::expected.
>

Yep, good catch, that looks like a bug in P2505. I'm adding the paper's
author Jeff Garland to this thread. (You should always feel free to do that
too, btw! The "contact" line in WG21 papers is there for exactly this
reason!)

@Jeff:
The bugs seem to be mostly due to incompletely copy-pasted-and-modified
code from the "monadic optional" paper.

(1) Under template <class F> constexpr auto transform(F&& f) const&&;

return std::move(*this)

 is missing its semicolon.

(2) You're missing the `&` and `const&&` overloads of `or_else`. This
seems too blatant to be accidental, but it's also inconsistent and
unmotivated. Sy Brand's tl::expected provides all four overloads, as one
would naturally expect. So should you.

(3) Under template <class F> constexpr expected or_else(F&& f) const&;
`invocable<>` should read `invocable<decltype(error())>`
`is_same_v<remove_cvref_t<invoke_result_t<F>>, expected>` should read
`is_same_v<remove_cvref_t<invoke_result_t<F, decltype(error())>>, expected>
`

return invoke(std::forward<F>(f)(error()));

should read `return invoke(std::forward<F>(f), error());`

(4) Under template <class F> constexpr expected or_else(F&& f) &&;
`move_constructable` should read `move_constructible.` (spelling, and with
the end-of-sentence period).
Similar errors as in #3.

(5) There is a design issue with `expected::or_else` which does not come up
with `optional::or_else`. This code should be legal:
    std::expected<Connection, PortableError> convertError(SSLError e) {
return std::unexpected(PortableError(e)); }
    std::expected<Connection, SSLError> e1 = ~~~;
    std::expected<Connection, PortableError> e2 = e1.or_else(convertError);
Therefore, actually, the signature should read
    template<class F> constexpr *see below* or_else(F&& f) &;
    template<class F> constexpr *see below* or_else(F&& f) const&;
    Let U be remove_cvref_t<invoke_result_t<F, decltype(error())>>.
    Constraints: U is a specialization of `expected`, and
is_same_v<U::value_type, T> is `true`.
    Effects: Equivalent to:
        if (*this) {
            return U(value());
        } else {
            return invoke(std::forward<F>(), error());
        }
Likewise for the && and const&& overloads.
Btw, I will investigate whether Sy Brand's tl::expected supports this
use-case, and I'll submit a pull request if it doesn't.

(6) Under `expected::and_then`, you actually have the opposite bug: you
haven't constrained it enough!
You write
> Let U be invoke_result_t<F, decltype(value())>.
> Constraints: F models invocable<decltype(value())>. E and U model
copy_constructible.
> Mandates: remove_cvref_t<U> is a specialization of expected.
but what you should write is
> Let U be remove_cvref_t<invoke_result_t<F, decltype(value())>>.
> Constraints: U is a specialization of expected, and
is_same_v<U::error_type, E> is `true`. E and U model copy_constructible.

Otherwise, you could end up with something like
    std::expected<Address, DNSError> get_address(const Connection&);
    std::expected<Connection, SSLError> e3 = ~~~;
    auto e4 = e3.and_then(get_address); // Yikes!
where get_address(e3.value()) is well-formed of type `expected<Address,
DNSError>`, but unfortunately there's no way to combine that type with the
original `expected<~~~, SSLError>` to get a common result type. You need to
constrain the error_type of U to be the same as the error_type of `*this`.

[The rest of Desmond's original message follows below my signature]

–Arthur



> From:
>
> template <class F> constexpr expected or_else(F&& f) const&
>
> *Constraints:* F models invocable<> and E models copy_constructible.
>
> *Mandates:* is_same_v<remove_cvref_t<invoke_result_t<F>>, expected> is
> true.
>
> *Effects:* Equivalent to:
>
> if (*this) {
> return *this;
> } else {
> return invoke(std::forward<F>(f)(error()));
> }
>
> 1 template <class F> constexpr expected or_else(F&& f) &&;
>
> *Constraints:* F models invocable<> and E models move_constructable
>
> *Mandates:* is_same_v<remove_cvref_t<invoke_result_t<F>>, expected> is
> true.
>
> *Effects:* Equivalent to:
>
> if (*this) {
> return std::move(*this);
> }else {
> return invoke(std::forward<F>(f)(std::move(error())));
> }
>
> What does invoke(std::forward<F>(f)(error())) or
> invoke(std::forward<F>(f)(std::move(error()))) mean in this case? While
> from void partial specialization of std::expected:
>
> template <class F> constexpr expected or_else(F&& f) const&;
>
> *Constraints:* F models invocable<> and E models copy_constructible.
>
> *Effects:* Equivalent to:
>
> if (*this) {
> return *this;
> } else {
> return invoke(std::forward<F>(f)());
> }
>
> 1 template <class F> constexpr expected or_else(F&& f) &&;
>
> *Constraints:* F models invocable<> and E models move_constructible.
>
> *Effects:* Equivalent to:
>
> if (*this) {
> return std::move(*this);
> }else {
> return invoke(std::forward<F>(f)());
> }
>
> Does or_else member function need invocable that takes *nothing* (nullary)
> or takes from *current** object's error()* ?
> There are three possible solutions to solve:
> 1.) Constrain such that F models invocable<> and the expression
> invoke(std::forward<F>(f)) is used inside else-block (no parameter)
> 2.) Constrain such that F models invocable<*cv* error_type *ref*> and
> the expression invoke(std::forward<F>(f), error()) for l-value ref
> overlads or invoke(std::forward<F>(f), std::move(error())) for r-value
> ref overloads.
> 3.) Combine the first two solutions.
> --
> Std-Proposals mailing list
> Std-Proposals_at_[hidden]
> https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals
>