diff --git a/doc/modules/ROOT/pages/version-history.adoc b/doc/modules/ROOT/pages/version-history.adoc index e32339ef..77451176 100644 --- a/doc/modules/ROOT/pages/version-history.adoc +++ b/doc/modules/ROOT/pages/version-history.adoc @@ -15,6 +15,7 @@ * Bug fixes (`backmp11`): ** Incorrect `FSM` type in calls to `on_entry(...)` and `on_exit(...)` (https://github.com/boostorg/msm/issues/167[#167]) ** Completion events fire too often (https://github.com/boostorg/msm/issues/166[#166]) +** Transitions in hierarchical state machines are not selected correctly (favor_compile_time) (https://github.com/boostorg/msm/issues/200[#200]) * **Breaking change (`backmp11`)**: Direct access to the event pool is changed from `public` to `protected`, because manipulating it outside of the library code can lead to undefined behavior. diff --git a/include/boost/msm/backmp11/detail/basic_polymorphic.hpp b/include/boost/msm/backmp11/detail/basic_polymorphic.hpp index 7bf156d1..ceee66f6 100644 --- a/include/boost/msm/backmp11/detail/basic_polymorphic.hpp +++ b/include/boost/msm/backmp11/detail/basic_polymorphic.hpp @@ -277,13 +277,13 @@ class basic_polymorphic { return basic_polymorphic{std::in_place_type, std::forward(args)...}; - }; + } template static basic_polymorphic make(const U& obj) { return basic_polymorphic{obj}; - }; + } T* get() const noexcept { diff --git a/include/boost/msm/backmp11/favor_compile_time.hpp b/include/boost/msm/backmp11/favor_compile_time.hpp index 82b6d6ef..9d9d8035 100644 --- a/include/boost/msm/backmp11/favor_compile_time.hpp +++ b/include/boost/msm/backmp11/favor_compile_time.hpp @@ -207,10 +207,9 @@ struct compile_policy_impl { public: template - process_result execute(StateMachine& sm, int region_id, any_event const& event) const + process_result execute(StateMachine& sm, int region_id, any_event const& event, process_result result) const { using cell_t = process_result (*)(StateMachine&, int, any_event const&); - process_result result = process_result::HANDLED_FALSE; for (const generic_cell cell : m_transition_cells) { result |= reinterpret_cast(cell)(sm, region_id, event); @@ -370,7 +369,7 @@ struct compile_policy_impl if (m_call_process_event) { result = m_call_process_event(sm, event); - if (result) + if (result & handled_true_or_deferred) { return result; } @@ -378,7 +377,7 @@ struct compile_policy_impl auto it = m_transition_chains.find(event.type()); if (it != m_transition_chains.end()) { - result = (it->second.execute)(sm, region_id, event); + result = (it->second.execute)(sm, region_id, event, result); } return result; } diff --git a/test/Backmp11Context.cpp b/test/Backmp11Context.cpp index 374feca0..2cc1373b 100644 --- a/test/Backmp11Context.cpp +++ b/test/Backmp11Context.cpp @@ -59,13 +59,13 @@ struct MachineBase_ : public state_machine_def> { Context& context = get_context(fsm); context.machine_entries++; - }; + } template void on_exit(const Event& /*event*/, Fsm& fsm) { get_context(fsm).machine_exits++; - }; + } using initial_state = Default; }; diff --git a/test/Backmp11Deferred.cpp b/test/Backmp11Deferred.cpp index a47b9f6a..00bf38d9 100644 --- a/test/Backmp11Deferred.cpp +++ b/test/Backmp11Deferred.cpp @@ -26,6 +26,9 @@ using namespace boost::msm::backmp11; namespace mp11 = boost::mp11; +namespace +{ + // Events struct Event1 { @@ -438,3 +441,5 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(action_deferred, Fsm, Fsms) } } // namespace action_deferred + +} // namespace diff --git a/test/Backmp11ManyDeferTransitions.cpp b/test/Backmp11ManyDeferTransitions.cpp index b86cc5cc..b000e04b 100644 --- a/test/Backmp11ManyDeferTransitions.cpp +++ b/test/Backmp11ManyDeferTransitions.cpp @@ -22,6 +22,9 @@ using namespace boost::msm::front; using namespace boost::msm::backmp11; namespace mp11 = boost::mp11; +namespace +{ + // Events struct Event1 {}; struct Event2 {}; @@ -235,3 +238,5 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(action_defer_transitions, TestMachine, TestMachine } } // namespace action_defer_transitions + +} // namespace diff --git a/test/Backmp11RootSm.cpp b/test/Backmp11RootSm.cpp index 33cfd93d..3fb19527 100644 --- a/test/Backmp11RootSm.cpp +++ b/test/Backmp11RootSm.cpp @@ -96,7 +96,7 @@ struct hierarchical_machine static_assert(std::is_same_v); } fsm.get_root_sm().machine_entries++; - }; + } template void on_exit(const Event& /*event*/, Fsm& fsm) @@ -106,7 +106,7 @@ struct hierarchical_machine static_assert(std::is_same_v); } fsm.get_root_sm().machine_exits++; - }; + } using initial_state = Default; }; @@ -128,7 +128,7 @@ struct hierarchical_machine // static_assert(std::is_same_v); } fsm.get_root_sm().machine_entries++; - }; + } template void on_exit(const Event& /*event*/, Fsm& fsm) @@ -142,7 +142,7 @@ struct hierarchical_machine // static_assert(std::is_same_v); } fsm.get_root_sm().machine_exits++; - }; + } }; using LowerMachine = state_machine; diff --git a/test/Backmp11Transitions.cpp b/test/Backmp11Transitions.cpp index bae1a416..021c8902 100644 --- a/test/Backmp11Transitions.cpp +++ b/test/Backmp11Transitions.cpp @@ -9,20 +9,19 @@ // file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) +#ifndef BOOST_MSM_NONSTANDALONE_TEST +#define BOOST_TEST_MODULE backmp11_transitions_test +#endif +#include + // back-end #include #include //front-end -#include -#include +#include "FrontCommon.hpp" #include "Utils.hpp" -#ifndef BOOST_MSM_NONSTANDALONE_TEST -#define BOOST_TEST_MODULE backmp11_members_test -#endif -#include - namespace msm = boost::msm; namespace mp11 = boost::mp11; @@ -32,15 +31,14 @@ using namespace msm::backmp11; namespace { -template -struct Counter -{ - size_t count{}; -}; - // Events. +struct EnterSubmachine{}; struct TriggerTransition{}; struct TriggerInternalTransition{}; +struct TriggerInternalTransitionWithGuard +{ + bool guard_value{}; +}; struct TriggerSmInternalTransition{}; struct TriggerAnyTransition{}; @@ -48,109 +46,161 @@ struct TriggerAnyTransition{}; struct MyAction { template - void operator()(const Event&, Fsm& fsm, Source&, Target&) + void operator()(const Event&, Fsm&, Source& source, Target&) + { + source.action_counter++; + } +}; + +// Guards +struct MyGuard +{ + template + bool operator()(const TriggerInternalTransitionWithGuard& event, Fsm&) const + { + return event.guard_value; + } +}; + +struct NotMyGuard +{ + template + bool operator()(const TriggerInternalTransitionWithGuard& event, Fsm&) const { - std::get>(fsm.action_calls).count++; + return !event.guard_value; } }; // States. -struct MyState : public state<>{}; -struct MyOtherState : public state<>{}; +struct MyState : public test::StateBase{}; +struct MyOtherState : public test::StateBase{}; class StateMachine; struct StateMachine_; -using Counters = std::tuple< - Counter, - Counter, - Counter, - Counter>; - -struct SmConfig : state_machine_config +struct default_config : state_machine_config { - using root_sm = StateMachine; - // using compile_policy = favor_compile_time; + // using root_sm = StateMachine; template using event_container = no_event_container; }; - -struct StateMachine_ : public state_machine_def +struct favor_compile_time_config : default_config { - using activate_deferred_events = int; + using compile_policy = favor_compile_time; +}; - template - void on_entry(const Event& /*event*/, Fsm& fsm) - { - fsm.entry_calls++; - }; +template +struct hierarchical_state_machine +{ - template - void on_exit(const Event& /*event*/, Fsm& fsm) - { - fsm.exit_calls++; - }; +struct Submachine_ : public test::StateMachineBase_ +{ + using initial_state = MyState; + using transition_table = mp11::mp_list< + Row + >; +}; +using Submachine = state_machine; +struct StateMachine_ : public test::StateMachineBase_ +{ using initial_state = MyState; using transition_table = mp11::mp_list< - Row, - Row, - Row + Row, + Row, + Row, + Row, + Row, + Row >; using internal_transition_table = mp11::mp_list< Internal, Internal >; }; + // Leave this class without inheriting constructors to check // that this only needs to be done in case we use a context // (for convenience). -class StateMachine : public state_machine +class StateMachine : public state_machine { - public: - size_t entry_calls{}; - size_t exit_calls{}; - Counters action_calls{}; }; -BOOST_AUTO_TEST_CASE( backmp11_members_test ) +}; + +using TestMachines = boost::mpl::vector< + hierarchical_state_machine<>, + hierarchical_state_machine + >; + + +BOOST_AUTO_TEST_CASE_TEMPLATE(start_stop, StateMachine, TestMachines) { - StateMachine test_machine; + using TestMachine = typename StateMachine::StateMachine; + TestMachine test_machine; test_machine.start(); - BOOST_REQUIRE(test_machine.entry_calls == 1); + BOOST_REQUIRE(test_machine.entry_counter == 1); - if constexpr (std::is_same_v< - typename StateMachine::config_t::compile_policy, - favor_runtime_speed>) + test_machine.stop(); + BOOST_REQUIRE(test_machine.exit_counter == 1); +} + +BOOST_AUTO_TEST_CASE_TEMPLATE(transitions, StateMachine, TestMachines) +{ + using TestMachine = typename StateMachine::StateMachine; + using compile_policy = typename TestMachine::config_t::compile_policy; + TestMachine test_machine; + + test_machine.start(); + + if constexpr (std::is_same_v) { test_machine.process_event(TriggerAnyTransition{}); - ASSERT_ONE_AND_RESET(std::get>(test_machine.action_calls).count); + ASSERT_ONE_AND_RESET(test_machine.action_counter); } test_machine.process_event(TriggerInternalTransition{}); - ASSERT_ONE_AND_RESET(std::get>(test_machine.action_calls).count); + ASSERT_ONE_AND_RESET(test_machine.template get_state().action_counter); // Ensure MyState consumes the event instead of StateMachine. test_machine.process_event(TriggerSmInternalTransition{}); - ASSERT_ONE_AND_RESET(std::get>(test_machine.action_calls).count); + ASSERT_ONE_AND_RESET(test_machine.template get_state().action_counter); + test_machine.process_event(TriggerTransition{}); - ASSERT_ONE_AND_RESET(std::get>(test_machine.action_calls).count); - BOOST_REQUIRE(test_machine.is_state_active()); + ASSERT_ONE_AND_RESET(test_machine.template get_state().action_counter); + BOOST_REQUIRE(test_machine.template is_state_active()); - if constexpr (std::is_same_v< - typename StateMachine::config_t::compile_policy, - favor_runtime_speed>) + if constexpr (std::is_same_v) { test_machine.process_event(TriggerAnyTransition{}); - ASSERT_ONE_AND_RESET(std::get>(test_machine.action_calls).count); + ASSERT_ONE_AND_RESET(test_machine.action_counter); } test_machine.process_event(TriggerSmInternalTransition{}); - ASSERT_ONE_AND_RESET(std::get>(test_machine.action_calls).count); + ASSERT_ONE_AND_RESET(test_machine.action_counter); + + test_machine.stop(); +} + +BOOST_AUTO_TEST_CASE_TEMPLATE(submachine_transitions, StateMachine, TestMachines) +{ + using TestMachine = typename StateMachine::StateMachine; + TestMachine test_machine; + auto& submachine = test_machine.template get_state(); + + test_machine.start(); + + test_machine.process_event(EnterSubmachine{}); + ASSERT_ONE_AND_RESET(submachine.entry_counter); + + test_machine.process_event(TriggerInternalTransitionWithGuard{false}); + ASSERT_ONE_AND_RESET(submachine.action_counter); + + test_machine.process_event(TriggerInternalTransitionWithGuard{true}); + ASSERT_ONE_AND_RESET(submachine.template get_state().action_counter); test_machine.stop(); - BOOST_REQUIRE(test_machine.exit_calls == 1); } } // namespace diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2f385d60..426664a7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -12,7 +12,7 @@ if(BOOST_MSM_TEST_ONLY_BACKMP11) add_compile_definitions("BOOST_MSM_TEST_ONLY_BACKMP11") endif() if(BOOST_MSM_TEST_STRICT) - add_compile_options(-Wall -Wextra -Werror -Wno-language-extension-token) + add_compile_options(-Wall -Wpedantic -Wextra -Werror -Wno-language-extension-token) endif() if(BOOST_MSM_TEST_ENABLE_SANITIZERS) add_compile_options(-fsanitize=address -fno-omit-frame-pointer) diff --git a/test/FrontCommon.hpp b/test/FrontCommon.hpp index 87d25a1f..82d44fba 100644 --- a/test/FrontCommon.hpp +++ b/test/FrontCommon.hpp @@ -36,6 +36,7 @@ struct StateBase : state<> size_t entry_counter{}; size_t exit_counter{}; + size_t action_counter{}; }; // Base for a state machine used in the tests. @@ -62,6 +63,7 @@ struct StateMachineBase_ : state_machine_def size_t entry_counter{}; size_t exit_counter{}; + size_t action_counter{}; };