boost-ext / sml

C++14 State Machine library
https://boost-ext.github.io/sml
Boost Software License 1.0
1.1k stars 173 forks source link

How to avoid delegating member functions #93

Open redboltz opened 7 years ago

redboltz commented 7 years ago

I'm using sml with property using dependency. I wrote the class property that have core methods. I also define the class table that has the property as the dependency. Finally, I define the class with_prop that combines property and sml::sm

. Thewith_propis a public interface object of other classes. It is requires to provideprocess_event()`.

However, I need to define many delegation member functions at with_prop. I want to avoid it. I tried to pass with_prop itself to sm, but I got compile errors.

Is there any good way to do that? It is a headache to migrate from msm to sml.

Here is the code:

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct property {
    // I want to implement many methods like this
    // in order to access from actions of state machine.
    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }
};

struct table {
    auto operator()() const noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [] (property& p) {
                std::cout << "on_entry" << std::endl;
                p.method1();
            }
        );
    }
};

struct with_prop {
    with_prop() : sm(p) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    // I want to implement many methods like this
    // in order to access from outer classes.
    // I need to write many delegating member functions
    // like this...
    void method1() {
        p.method1();
    }
    void method2() {
        p.method2();
    }

private:
    property p;
    sml::sm<table> sm;
};

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}
redboltz commented 7 years ago

I find the way.

My first try was:

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct with_prop;

struct table {
    auto operator()() const noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [] (with_prop& p) {
                std::cout << "on_entry" << std::endl;
                p.method1(); // compile error: p is incomplete
            }
        );
    }
};

struct with_prop {
    with_prop() : sm(*this) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }

private:
    sml::sm<table> sm;
};

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}

I got error. Because p is incomplete type. It's normal. Then I tried to separate declaration and definition of table::operator().

Here is the code:

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct table {
    auto operator()() const noexcept;
};

struct with_prop {
    with_prop() : sm(*this) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }

private:
    sml::sm<table> sm;
};

inline auto table::operator()() const noexcept
{
    using namespace sml;
    return make_transition_table(
        *"s1"_s  + sml::on_entry<_> /
        [] (with_prop& p) {
            std::cout << "on_entry" << std::endl;
            p.method1();
        }
    );
}

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}

I got the compile error:

sml/include/boost/sml.hpp: In substitution of ‘template<class T, typename boost::sml::v1_1_0::aux::enable_if<boost::sml::v1_1_0::concepts::composable<typename T::sm>::value, int>::type <anonymous> > using state_machine = boost::sml::v1_1_0::back::sm<TSrc> [with T = boost::sml::v1_1_0::back::sm_policy<table>; typename boost::sml::v1_1_0::aux::enable_if<boost::sml::v1_1_0::concepts::composable<typename T::sm>::value, int>::type <anonymous> = 0]’:
/home/kondo/work/sml/include/boost/sml.hpp:1545:67:   required by substitution of ‘template<class T, class ... TPolicies> using sm = boost::sml::v1_1_0::detail::state_machine<boost::sml::v1_1_0::back::sm_policy<T, TPolicies ...> > [with T = table; TPolicies = {}]’
prop3.cpp:32:18:   required from here
/home/kondo/work/sml/include/boost/sml.hpp:1545:67: error: no type named ‘type’ in ‘struct boost::sml::v1_1_0::aux::enable_if<false, int>’
 using sm = detail::state_machine<back::sm_policy<T, TPolicies...>>;

Finally, I got a solution. Instead of separating table::operator(), separating actions. It works fine!

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct with_prop;

struct fobj {
    void operator()(with_prop& p) const;
};

struct table {
    auto operator()() const noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [] (with_prop& p) {
                fobj()(p);
            }
        );
    }
};

struct with_prop {
    with_prop() : sm(*this) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    // I want to implement many methods like this
    // in order to access from outer classes.
    // I need to write many delegating member functions
    // like this...
    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }

private:
    sml::sm<table> sm;
};

inline void fobj::operator()(with_prop& p) const {
    std::cout << "on_entry" << std::endl;
    p.method1();
}

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}
redboltz commented 7 years ago

I found a better solution. Defining operator()() as a member function template. I don't need to separate declaration and definition:)

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct with_prop;

struct fobj {
    template <typename T>
    void operator()(T& p) const {
        std::cout << "on_entry" << std::endl;
        p.method1();
    }

};

struct table {
    auto operator()() const noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [](with_prop& p) { fobj()(p); }
        );
    }
};

struct with_prop {
    with_prop() : sm(*this) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }

private:
    sml::sm<table> sm;
};

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}
redboltz commented 7 years ago

Finally, I eliminate function object! The inner lambda expression with auto parameter and explicit return value type make the body of lambda expression instantiation lazily.

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct with_prop;

struct table {
    auto operator()() const noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [](with_prop& p) { 
                // -> void is mandatory to avoid function body instantiation
                // to deduce return value type.
                [](auto& p) -> void { 
                    std::cout << "on_entry" << std::endl;
                    p.method1();
                }(p);
            }
        );
    }
};

struct with_prop {
    with_prop() : sm(*this) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }

private:
    sml::sm<table> sm;
};

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}
redboltz commented 4 years ago

I noticed that some of my solutions depend on undefined behavior of the compiler. That means the code is ill-formed. See https://stackoverflow.com/questions/58246088/lazy-instantiation-for-lambda-expression

https://github.com/boost-experimental/sml/issues/93#issuecomment-283629397 and https://github.com/boost-experimental/sml/issues/93#issuecomment-283630876 are compiled and worked as I expected but it is by accident.

The last code of https://github.com/boost-experimental/sml/issues/93#issuecomment-283613193 is well-formed.

It is well-formed one (re-post with comments):

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct with_prop;

// Must locate before the definition of `table` due to *1
struct fobj {
    void operator()(with_prop& p) const;
};

struct table {
    // Cannot separate declaration and definition because the return type is deduced.
    auto operator()() const noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [] (with_prop& p) {
                fobj()(p); // create fobj *1
            }
        );
    }
};

// Class that has a state-machine
// Must locate after the definition of `table` due to *2
struct with_prop {
    with_prop() : sm(*this) {}

    template <typename Event>
    void process_event(Event&& ev) {
        sm.process_event(std::forward<Event>(ev));
    }

    // I want to implement many methods like this
    // in order to access from outer classes.
    // I need to write many delegating member functions
    // like this...
    void method1() {
        std::cout << "method1" << std::endl;
    }
    void method2() {
        std::cout << "method2" << std::endl;
    }

private:
    sml::sm<table> sm; // state-machine that refers to table *2 
};

// Must locate after the definition of `with_prop`
inline void fobj::operator()(with_prop& p) const {
    std::cout << "on_entry" << std::endl;
    p.method1();
}

int main() {
    with_prop wp;
    std::cout << "> Send Event1" << std::endl;
    wp.process_event(ev1());
    wp.method2();
}

We need to separate the declaration and definition of the function object (fobj) that calls property's (with_prop) member function (method1() and method2()).

I believe that this is very common scenario to use SML with an application class. However, this solution is very complicated. Is there any better solution? So I reopen the issue.

redboltz commented 4 years ago

Let me explain what the problem is.

You might think I can place member functions in table directly. I can do that. But the member function can be called only inside of the state machine as guard and/or action. I want to call member functions not only inside but also outside of the state machine.

Here is an example: https://wandbox.org/permlink/TkvN8PX69JbRGy0r

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct table {
    // Cannot separate declaration and definition because the return type is deduced.
    auto operator()() noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [this] {
                inc();
            },
            "s1"_s + sml::event<ev1> /
            [this] {
                print();
            } = "s1"_s
        );
    }

    void inc() {
        ++val;
    }
    void print() const {
        std::cout << val << std::endl;
    }
    int val = 0;
};

int main() {
    int i;
    sml::sm<table> sm(i); // state-machine that refers to table *2 
    std::cout << "> Send Event1" << std::endl;
    sm.process_event(ev1());
    std::cout << "> Send Event1" << std::endl;
    sm.process_event(ev1());
    // want to call the function like print() from outside of the state machine
    // sm.print();
}

See the last sm.print(). It makes compile error. So I inject my object to the state machine.

redboltz commented 4 years ago

I tried to find a different way. Instead of inject my data, perhaps there is a way that somehow extract my data from the state machine.

I found the following conversion operators:

https://github.com/boost-experimental/sml/blob/4525a24797bf0bb426b1d1c36a401f9e07bfbedd/include/boost/sml.hpp#L1694-L1701

It seems that the operator returns a (const) reference of some internal object.

https://wandbox.org/permlink/AnTOH8vYoB4ZK7it

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct table {
    // Cannot separate declaration and definition because the return type is deduced.
    auto operator()() noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [this] {
                inc();
            },
            "s1"_s + sml::event<ev1> /
            [this] {
                print();
            } = "s1"_s
        );
    }

    void inc() {
        ++val;
    }
    void print() const {
        std::cout << val << std::endl;
    }
    int val = 0;
};

int main() {
    sml::sm<table> sm; // state-machine that refers to table *2 
    std::cout << "> Send Event1" << std::endl;
    sm.process_event(ev1());
    std::cout << "> Send Event1" << std::endl;
    sm.process_event(ev1());

    std::cout << "From outside" << std::endl;
    table const& cr = sm; // get const reference using conversion operator
    table & r = sm; // get reference using conversion operator
    r.inc();
    cr.print();
}

I can call table's member functions from the outside of the state machine via the references of table object. It solves my problem! But I'm still not sure is this right way? I hope so.

updated Removed misleading parameter int i.

redboltz commented 4 years ago

I noticed that my solution doesn't work for the class that has a reference member. For example, if val is reference in above code as follows:

    int& val;

then there is no way to initialize it. Even if I add the constructor

struct table {
    table(int& val):val(val) {}

it isn't called.

So I give up using reference. Use pointer instead of the reference (BTW, I don't like this kind of pointer).

https://wandbox.org/permlink/f3xdqOEKS8ov6jiC

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;

struct ev1 {};

struct table {
    // Cannot separate declaration and definition because the return type is deduced.
    auto operator()() noexcept {
        using namespace sml;
        return make_transition_table(
            *"s1"_s  + sml::on_entry<_> /
            [this] {
                inc(); // access val before initialize
            },
            "s1"_s + sml::event<ev1> /
            [this] {
                print();
            } = "s1"_s
        );
    }

    void inc() {
        ++*val;
    }
    void print() const {
        std::cout << *val << std::endl;
    }
    int* val;
};

int main() {
    int i = 42;
    sml::sm<table> sm; // state machine run here, and val is not initialized yet
    table& r = sm;
    r.val = &i;
    std::cout << "> Send Event1" << std::endl;
    sm.process_event(ev1());
    std::cout << "> Send Event1" << std::endl;
    sm.process_event(ev1());
}

The code segfault. Because val is accessed before initialized. Is there any way to cold start the state machine? So far, I couldn't find it. The workaround is add dummy state and event to the state machine that doesn't access val. It is not elegant.

borreeero commented 2 years ago

You can give sml::sm<table> constructor a properly initialized table object like so:

int i = 42;
sml::sm<table> sm{table{&i}};

Would this work for you?