Yes, thanks for the feedback — I will be working on updates over the next few days due to similar feedback from others. And yeah, the paper started with a copy of optional — but the more you get into expected the more differences there are. Another item not in the paper currently is transform<void, E> — Sy’s implementation doesn’t support it, but there are use cases for it and it seems completely reasonable in retrospect.

Jeff

> On Jan 2, 2022, at 11:29 AM, Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]> wrote:
>
> On Sun, Jan 2, 2022 at 7:42 AM Desmond Gold Bongcawel via Std-Proposals <std-proposals_at_[hidden] <mailto:std-proposals_at_[hidden]>> wrote:
> While reading this paper https://wg21.link/P2505 <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] <mailto:Std-Proposals_at_[hidden]>
> https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals <https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals>