fix: re-enable const lvalue reference dependency usage

[dornbirndevelops]

Problem:

• the following minimal reproducible example fails to compile with the recent changes:

#include <boost/sml.hpp>

struct Dependency { bool ok{}; };
struct Event {};
struct State {};
struct Guard { bool operator()(const Event&, const Dependency& d) const { return d.ok; } };

struct SM {
    auto operator()() const noexcept
    {
        using namespace boost::sml;

        constexpr auto guard = Guard{};

        return make_transition_table(
          * state<State> + event<Event> [guard] = X
        );
    }
};

void lValueScenario()
{
    auto d = Dependency{};
    auto sm = boost::sml::sm<SM>{d};
    sm.process_event(Event{});
}

void constLValueScenario()
{
    // expectation:
    // - const lvalue references are allowed in case all guards/actions are accessing the dependency via const lvalue reference
    // actual:
    // - does not compile anymore, user is forced to use non-const lvalue references
    const auto d = Dependency{};
    auto sm = boost::sml::sm<SM>{d};
    sm.process_event(Event{});
}

Solution:

• introduce a meta programming function collapse_const_refs_t that performs normalization only if at least one non-const reference usage is present

Issue: #711

Reviewers:
@kris-jusiak
@PavelGuzenfeld

[PavelGuzenfeld]

Thanks for the PR @dornbirndevelops ,
I'm double-checking.

[PavelGuzenfeld]

Built this branch and it checks out — fix is correct. #711 repro compiles on GCC c++14/17/20 (macro on+off) and MSVC c++20, #530 still passes (const_ref_state_dep_sees_live_state + const guard seeing live mutations), full ft suite 113/113. The collapse_const_refs_t + SFINAE-split get_arg is the right call.

Only thing missing is a functional regression test — right now it's just the two header static_asserts, which never actually build an sm<> with a const-injected dep.

One gotcha on where to put it: #711 only reproduces with BOOST_SML_CREATE_DEFAULT_CONSTRUCTIBLE_DEPS OFF. With it on, the ref-slot specialization copies into a backing store and the const injection compiles even on the broken header. dependencies.cpp defines that macro for the whole TU, so a test added there would pass against the pre-fix header too — i.e. wouldn't guard anything. It needs its own macro-off file.

Something like test/ft/dependencies_const_ref.cpp:

#include <boost/sml.hpp>
namespace sml = boost::sml;

struct dep711 { int n{}; };
struct ev711 {};

// #711: dep used only via `const T&` must accept a const lvalue.
test const_lvalue_dependency_injection = [] {
  struct c {
    auto operator()() const noexcept {
      using namespace sml;
      auto guard = [](const dep711& d) { return d.n == 7; };
      return make_transition_table(*"idle"_s + event<ev711>[guard] = X);
    }
  };
  const dep711 d{7};
  sml::sm<c> sm{d};
  sm.process_event(ev711{});
  expect(sm.is(sml::X));
};

test nonconst_lvalue_dependency_injection = [] {
  struct c {
    auto operator()() const noexcept {
      using namespace sml;
      auto guard = [](const dep711& d) { return d.n == 7; };
      return make_transition_table(*"idle"_s + event<ev711>[guard] = X);
    }
  };
  dep711 d{7};
  sml::sm<c> sm{d};
  sm.process_event(ev711{});
  expect(sm.is(sml::X));
};

// #530 under normal injection: const guard sees the mutable action's write.
test const_guard_sees_live_mutation_of_injected_dep = [] {
  struct c {
    auto operator()() const noexcept {
      using namespace sml;
      auto inc   = [](dep711& d) { ++d.n; };
      auto check = [](const dep711& d) { return d.n == 8; };
      return make_transition_table(
        *"s1"_s + event<ev711> / inc   = "s2"_s,
         "s2"_s + event<ev711> [check] = X);
    }
  };
  dep711 d{7};
  sml::sm<c> sm{d};
  sm.process_event(ev711{});
  sm.process_event(ev711{});
  expect(sm.is(sml::X));
  expect(d.n == 8);
};

plus in test/ft/CMakeLists.txt:

add_executable(test_dependencies_const_ref dependencies_const_ref.cpp)
add_test(test_dependencies_const_ref test_dependencies_const_ref)

I checked this file passes on the branch and fails to compile on the pre-fix header (binding reference of type 'dep711&' to 'const dep711' discards qualifiers), so it actually locks the regression. Only const_lvalue_dependency_injection is the strict #711 catcher; the other two are bonus coverage.

[PavelGuzenfeld]

Fix verified correct, the gap is a functional regression test, and it should go in a new macro-OFF file (dependencies_const_ref.cpp) since #711 doesn't reproduce with BOOST_SML_CREATE_DEFAULT_CONSTRUCTIBLE_DEPS on — with the test body and the two CMake lines, noted as verified to fail on the pre-fix header.

[dornbirndevelops]

I don't understand the Codacy Static Code Analysis issue.

Include file: <boost/sml.hpp> not found.

I tried to mimic the existing dependencies.cpp translation unit.

sml/test/ft/dependencies.cpp

Line 9 in de8d45d

#include <boost/sml.hpp>

The include path gets added due to

sml/CMakeLists.txt

Line 141 in de8d45d

include_directories($<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>)

It works fine locally.

What did I miss?

[PavelGuzenfeld]

I don't understand the Codacy Static Code Analysis issue.

Include file: <boost/sml.hpp> not found.

I tried to mimic the existing dependencies.cpp translation unit.

sml/test/ft/dependencies.cpp

Line 9 in de8d45d

#include <boost/sml.hpp>

The include path gets added due to

sml/CMakeLists.txt

Line 141 in de8d45d

include_directories($<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>)

It works fine locally.

What did I miss?

To suppress one may add :

 // cppcheck-suppress missingIncludeSystem
  #include <boost/sml.hpp>

Thanks for the extra test coverage @dornbirndevelops :)

[PavelGuzenfeld]

LGTM :)

[kris-jusiak]

Thank you