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

Add monadic bind operators, >= and > #87

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ezzieyguywuf
Copy link

These are equivalent to the haskell >>= and >> operators respectively. They
can be used as follows:

#include "tl/expected.h"

#include <string>
#include <iostream>

struct A
{
    int x;
};

struct B
{
    std::string s;
};

using EitherAB = tl::expected<A, B>;

auto good() -> EitherAB
{
    std::cout << "good" << '\n';
    return EitherAB(A{10});
}

auto goodUsesA(const A&) -> EitherAB
{
    std::cout << "goodUsesA" << '\n';
    return EitherAB(A{20});
}

auto badUsesA(const A&) -> EitherAB
{
    std::cout << "badUsesA" << '\n';
    return tl::make_unexpected(B{"Dang"});
}

auto bad() -> EitherAB
{
    std::cout << "bad" << '\n';
    return tl::make_unexpected(B{"Whoops"});
}

int main()
{
    // some function that returns a good value
    auto ret = good();

    // This is existing behaviour
    ret = ret.and_then(goodUsesA);

    // This is equivalent to and_then
    ret = good() >= goodUsesA >= goodUsesA >= badUsesA;

    // Sometimes, though, you want to call a function and don't care about its
    // return value. That's where > comes in.
    ret = good() >= goodUsesA > good > good;
}

Signed-off-by: Wolfgang E. Sanyer [email protected]

@ezzieyguywuf
Copy link
Author

I could use some help with this CI build failure. I've been able to duplicate it locally on gcc-10.3.0 by using -std=c++14. If I bump up to -std=c++17 then the error goes away.

I imagine this behaviour has something to do with some of the deprecation surrounding result_of associated with c++17. I've tried using std::result_of_t with -std=c++17, but for some reason I keep getting an error that the symbol is not found.

I've also tried using invoke_result_t (with the std:: namespace qualifier) since I noticed in the code that you have some #if macros that are conditionally defining some local (?) versions of the invoke* stuff. I also get symbol-not-found errors with this. I checked the preprocessed output and these definitions are still there, so I'm really at a loss as to what I'm doing wrong.

Any help or tips would be greatly appreciated. I'm happy to provide any other information you might need.

@julienlopez
Copy link

julienlopez commented Jul 11, 2021

Hello,

In the more general term, you operators probably don't need you to specify the return type at all, auto alone would do just fine.

The issue here is that your code is not correct and is only valid when your functors don't change the nested type.

So to fix both at once, you should probably just use the and_then member function and everything will work.

That being said, two issues I see coming:

  1. That won't fix your > operator, if you want to fix this one, you should probably check how the and_then member function does it, using decltype instead of invoke_result_t
  2. It would be nice to support both map and and_then functionalities. On possibility would be to use two operators, for example >= and >>=).

In any case, you should write some unit tests to check that everything works in all cases.

@ezzieyguywuf
Copy link
Author

@julienlopez thanks for your thoughts and your help.

I'll take a look at the fixes as you've suggested.

As you pointed out, the >= operator is intended to be the same as the and_then function.

As far as map is concerned, my understanding is that this is distinct from the > operator that i propose.

Whereas map allows you to apply some function to the stored value (or fail of the error value is being stored), > is intended to operate like and_then , except rather than forwarding the stored value as a function parameter, it simply invokes the function as-is.

I thought maybe naming it and_also might make sense.

@ezzieyguywuf ezzieyguywuf marked this pull request as draft July 11, 2021 16:26
@julienlopez
Copy link

Yes, map is different from your > operator.

There is nothing like >> in this lib, and it could be a nice addition, even if I rarely ended up in this case so far.

To my (very limited) understanding of haskell >>= can bind both functions in the form (A -> B) and (A -> m B), right?

In this lib, there is nothing to do both, so there are two function, map (taking a functor like A -> B) and and_then (taking a functor like A -> m B).

As it is, using map on a functor like A -> m B would result in an m m B in expected (ex<ex<B, E>, E> in a more c++ / template form).

Since I don't really see the point of having such a construct, having an operator handling both and calling the right function between map or and_then would be a nice addition to this lib imo, but it's gonna be trickier to implement, espcially if c++11 compatibility is to be kept.

Is this what you intend to do?

Signed-off-by: Wolfgang E. Sanyer <[email protected]>
@ezzieyguywuf ezzieyguywuf marked this pull request as ready for review July 11, 2021 18:21
@ezzieyguywuf
Copy link
Author

ezzieyguywuf commented Jul 11, 2021

Yes, map is different from your > operator.

There is nothing like >> in this lib, and it could be a nice addition, even if I rarely ended up in this case so far.

Yes I agree this would be nice: that is why I am trying to add it 😀

To my (very limited) understanding of haskell >>= can bind both functions in the form (A -> B) and (A -> m B), right?

No this is wrong, in haskell there are very distinct >>= operators and map/fmap functions.

Briefly, here are the different function signatures:

Control.Monad (>>=) :: forall a b . Monad m => m a -> (a -> m b) -> m b
Control.Monad fmap :: Functor f => (a -> b) -> f a -> f b

So, whereas >>= takes as operands a m a (a monad holding an a) and a function that accepts an a and returns a monad containing a b m b, the fmap function accepts a function taking a a and producing a b and a functor holding an a f a, and returns a functor containing a b f b.

Dang I guess that wasn't very brief or illuminating :-P

The difference between >>= and fmap are the types of functions they accept. >>= returns a monad whereas fmap returns a regular value.

There!

In this lib, there is nothing to do both, so there are two function, map (taking a functor like A -> B) and and_then (taking a functor like A -> m B).

As it is, using map on a functor like A -> m B would result in an m m B in expected (ex<ex<B, E>, E> in a more c++ / template form).

Since I don't really see the point of having such a construct, having an operator handling both and calling the right function between map or and_then would be a nice addition to this lib imo, but it's gonna be trickier to implement, espcially if c++11 compatibility is to be kept.

Is this what you intend to do?

So perhaps I'll start with some haskell code:

do
    x <- someMonadicOp
    return someOpThatNeedsX x

The do-notation is nice, but is simply syntactic sugar for

someMonadicOp >>= someOpThatNeedsX

Sometimes, you need something like this though:

do
    someMonadicOpThatChecksSomething
    someOtherMonadicOp
    x <- someMonadicOp
    return someOpThatNeedsX x

Again, if we unwrap the nice do-notation:

someMonadicOpThatChecksSomething >> someOtherMonadicOp >> someMonadicOp >>= someOpThatNeedsX

Just to be clear, this could also be written as:

someMonadicOpThatChecksSomething >>
     \_ -> someOtherMonadicOp >> 
        \_ -> someMonadicOp >>= 
            \x -> someOpThatNeedsX x

Where \x -> something is the haskell lambda function syntax.

I find this sort of programming style extremely helpful: notice here for example. I want to check that both the vertex and the edge are valid before proceeding to the actual logic, and if either are not valid then I want to provide a useful error message.

Anyway, I think my comment has drawled on long enough...please take a look at my latest commit and let me know your thoughts, I think I might be getting close.

This is opt-in, using the EXPECTED_BIND_OPERATORS cmake definition

Signed-off-by: Wolfgang E. Sanyer <[email protected]>
@ezzieyguywuf
Copy link
Author

Switching this back to draft - it turns out that my and_also function is not needed, as this functionality is [already built into and_then][https://github.com/TartanLlama/expected/blob/master/include/tl/expected.hpp#L1960]

In other words: and_then will either take a function that accepts a single tl::expected (and forward the tl::expected appropriately) or accept a function that takes no arguments.

I might update my PR to improve the documentation to make this clear.

I still think the bind operators could be useful.

Finally, I think it could be helpful to update and_then to accept arguemnts to the passed function. Then this:

// current implementation of and_then
auto ret =
    someFunc
    .and_then(std::bind(f, x, y, z)) // let's say: tl::expected<...> f(int, int, int, tl::expected<...>
    .and_then(std::bind(g, x, y, z)) // in this case tl::expecte<...> g(int, int, int)

Could be simplified to:

auto ret =
    someFunc
    .and_then(f, x, y, z)
    .and_then(g, x, y, z)

I'm toying around with it now do see if I can add this functionality

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

Successfully merging this pull request may close these issues.

2 participants