First action not executed when using multiple member function actions

[WouterLok]

SML 1.1.3 introduced the option to specify member function pointer instead of using lambda. The normal case as shown in the example works, but when specifying multiple actions for a single event, I get a compilation warning:

[build] /opt/build/debug/src/test.cxx: In member function ‘auto {anonymous}::actions_guards::operator()()’: [build] /opt/build/debug/src/test.cxx:57:72: error: left operand of comma operator has no effect [-Werror=unused-value] [build] , "s5"_s + event [ &self::guard3 ] / (&self::action4, &self::action5) = X

When combining a callable struct or a lambda with a member function, all is well. I suppressed the warning, compiled again and executed the test. As expected the action is NOT executed.

Below is a modified version of the example sml/example/actions_guards.cpp. I modified actions for event5 to execute 2 actions both using a member function ptr, action4 is not executed, where action5 is.

//
// Copyright (c) 2016-2020 Kris Jusiak (kris at jusiak dot net)
//
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// http://www.boost.org/LICENSE_1_0.txt)
//

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

namespace sml = boost::sml;

namespace {

struct e1 {};
struct e2 {};
struct e3 {};
struct e4 {};
struct e5 {};

struct actions_guards {
  using self = actions_guards;

  auto operator()() {
    using namespace sml;

    auto guard1 = [] {
      std::cout << "guard1" << std::endl;
      return true;
    };

    auto guard2 = [](int i) {
      assert(42 == i);
      std::cout << "guard2" << std::endl;
      return false;
    };

    auto action1 = [](auto e) { std::cout << "action1: " << typeid(e).name() << std::endl; };

    struct action2 {
      void operator()(int i) {
        assert(42 == i);
        std::cout << "action2" << std::endl;
      }
    };

    // clang-format off
    return make_transition_table(
       *"idle"_s + event<e1> = "s1"_s
      , "s1"_s + event<e2> [ guard1 ] / action1 = "s2"_s
      , "s2"_s + event<e3> [ guard1 && ![] { return false;} ] / (action1, action2{}) = "s3"_s
      , "s3"_s + event<e4> [ !guard1 || guard2 ] / (action1, [] { std::cout << "action3" << std::endl; }) = "s4"_s
      , "s3"_s + event<e4> [ guard1 ] / ([] { std::cout << "action4" << std::endl; }, [this] { action4(); } ) = "s5"_s

     //  2 member function actions
      , "s5"_s + event<e5> [ &self::guard3 ] / (&self::action4, &self::action5)  = X 
    );
    // clang-format on
  }

  bool guard3(int i) const {
    assert(42 == i);
    std::cout << "guard3" << std::endl;
    return true;
  }

  void action4() const { std::cout << "action4 member" << std::endl; }

  void action5(int i, const e5&) {
    assert(42 == i);
    std::cout << "action5" << std::endl;
  }
};
}  // namespace

int main() {
  actions_guards ag{};
  sml::sm<actions_guards> sm{ag, 42};
  sm.process_event(e1{});
  sm.process_event(e2{});
  sm.process_event(e3{});
  sm.process_event(e4{});
  sm.process_event(e5{});
  assert(sm.is(sml::X));
}

// OUTPUT
guard1
action1: N12_GLOBAL__N_12e2E
guard1
action1: N12_GLOBAL__N_12e3E
action2
guard1
guard2
guard1
action4
action4 member
guard3
action5 << Should have action4 member first

Specifications

• Version: 1.1.3
• Platform: Linux
• Subsystem: RHEL 7.5
• Compiler: GCC 8.3.1 20190311

[uyha]

This happens because the member function pointers do not get converted to zero_wrapper by ADL, but instead, they are understood by the compiler as parameters to the operator, and hence only the last member function pointer is returned and then gets converted to zero_wrapper. I suggest wrapping the member function pointers as a zero_wrapper first and then pass them as actions.

  auto operator()() {
    using namespace sml;
    ...

    auto const action4 = aux::zero_wrapper{&self::action4_impl};
    auto const action5 = aux::zero_wrapper{&self::action5_impl};
    // clang-format off
    return make_transition_table(
       *"idle"_s + event<e1> = "s1"_s
      , "s1"_s + event<e2> [ guard1 ] / action1 = "s2"_s
      , "s2"_s + event<e3> [ guard1 && ![] { return false;} ] / (action1, action2{}) = "s3"_s
      , "s3"_s + event<e4> [ !guard1 || guard2 ] / (action1, [] { std::cout << "action3" << std::endl; }) = "s4"_s
      , "s3"_s + event<e4> [ guard1 ] / ([] { std::cout << "action4" << std::endl; }, [this] { action4(); } ) = "s5"_s

     //  2 member function actions
      , "s5"_s + event<e5> [ &self::guard3 ] / (action4, action5)  = X 
    );
    // clang-format on
   ...
  }

  void action4_impl() const { std::cout << "action4 member" << std::endl; }

  void action5_impl(int i, const e5&) {
    assert(42 == i);
    std::cout << "action5" << std::endl;
  }

[uyha]

Since the PR #390, aux::zero_wrapper should be replaced by wrap so the code doesn't touch the internal details of the library. So action4 and action5 should be defined as follow:

auto const action4 = wrap(&self::action4_impl);
auto const action5 = wrap(&self::action5_impl);