Skip to content

fix two-phase lookup and test assertion in pool_type_impl<T&> fix (#504 follow-up)#675

Merged
kris-jusiak merged 1 commit into
boost-ext:masterfrom
PavelGuzenfeld:fix/issue-504-test-assertion
May 23, 2026
Merged

fix two-phase lookup and test assertion in pool_type_impl<T&> fix (#504 follow-up)#675
kris-jusiak merged 1 commit into
boost-ext:masterfrom
PavelGuzenfeld:fix/issue-504-test-assertion

Conversation

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor

Follow-up to #674 (merged as e4fdeb2), which introduced two issues that break CI on all platforms.

Problem 1: two-phase name lookup

pool_type_impl<T&>(const init&, const TObject&) calls try_get<T> in a member-initialiser list. try_get is declared later in the same namespace; without a prior declaration the compiler cannot recognise try_get<T>(...) as a template call at definition time (C++ [temp.dep.res] §13.8.3). All four CI platforms now fail with:

include/boost/sml.hpp:658: error: 'try_get' was not declared in this scope

Fix: add forward declarations for pool_type, missing_ctor_parameter, and all try_get overloads immediately before the #if defined(BOOST_SML_CREATE_DEFAULT_CONSTRUCTIBLE_DEPS) block.

Problem 2: invalid is<>() assertion in the regression test

The test called:

expect(sm.is<sml::state<sub504>>(sml::X));

sml::state<sub504> is a value expression, not a type; sm::is<T> requires a raw class type (T::type must be valid). Clang, GCC, and MSVC all reject this with "invalid template argument".

Fix: remove the assertion. The action lambda [](dep504& d) { expect(99 == d.val); } is the actual liveness check — if the reference were dangling, reading d.val would be UB/crash, which is what the test is designed to catch.

Tests

make test CXX=g++ CXXSTD=c++17 and CMake Debug build both pass with zero errors.

…ost-ext#504 follow-up)

The boost-ext#504 commit (e4fdeb2) introduced two issues:

1. pool_type_impl<T&>(const init&, const TObject&) calls try_get<T>
   in a member-initialiser list.  try_get is declared later in the same
   namespace; without a prior declaration the compiler cannot recognise
   try_get<T>(...) as a template call at definition time (C++ two-phase
   name lookup, [temp.dep.res]).  Fix: add forward declarations for
   pool_type, missing_ctor_parameter, and all try_get overloads before
   the pool_type_impl<T&> specialisation.

2. The regression test ref_dep_copy_from_pool_not_dangling used
   sm.is<sml::state<sub504>>(sml::X) to verify the sub-state.
   sml::state<sub504> is a value expression, not a type; the 'is'
   template expects a raw class type (T::type must be valid).  Clang,
   GCC, and MSVC all rejected the call with 'invalid template argument'.
   Fix: remove the assertion; the action lambda already calls
   expect(99 == d.val), which is the correct liveness check for the fix.
@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

PavelGuzenfeld commented May 23, 2026

Please merge this one first and then I'll debase all the rest to fix the CI.

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

Note: the upstream master branch CI is also currently failing with the same dependencies.cpp:396 error (run 26344508978) — this PR fixes both the PR regression and the broken master.

PavelGuzenfeld added a commit to PavelGuzenfeld/sml that referenced this pull request May 23, 2026
…valid)

Upstream master commit e4fdeb2 added a regression test for issue boost-ext#504 that
used sm.is<sml::state<sub504>>(sml::X) — sml::state<sub504> is a value
expression, not a type, so 'is<T>' rejects it on all compilers.

The same bad assertion appears in this branch because dependencies.cpp is
inherited from the base commit.  Remove the assertion; the action lambda
already calls expect(99 == d.val) which is the correct liveness check.

(Fixes are tracked by PR boost-ext#675.)
PavelGuzenfeld added a commit to PavelGuzenfeld/sml that referenced this pull request May 23, 2026
…valid)

Upstream master commit e4fdeb2 added a regression test for issue boost-ext#504 that
used sm.is<sml::state<sub504>>(sml::X) — sml::state<sub504> is a value
expression, not a type, so 'is<T>' rejects it on all compilers.

The same bad assertion appears in this branch because dependencies.cpp is
inherited from the base commit.  Remove the assertion; the action lambda
already calls expect(99 == d.val) which is the correct liveness check.

(Fixes are tracked by PR boost-ext#675.)
@kris-jusiak kris-jusiak merged commit 8834f3a into boost-ext:master May 23, 2026
5 checks passed
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