initialize: save the state of the policies it initializes, and only those

include/boost/openmethod/initialize.hpp

@@ -124,27 +124,83 @@ struct initialize_policies<Registry, mp11::mp_list<Policies...>> {
     }
 };
 
+// Selects the policies whose state the transaction has to save: those that
+// have a `state`, *and* whose `initialize` will actually be called for this
+// Context and Options - the same test initialize_policy makes, so the two
+// cannot disagree about which policies run.
+template<class Context, class Options>
+struct policy_state_is_volatile_q {
+    template<class PolicyFn>
+    using fn = mp11::mp_bool<
+        has_policy_state<PolicyFn>::value &&
+        has_initialize<PolicyFn, const Context&, const Options&>>;
+};
+
 // Saves the policies' states on construction, and puts them back on
 // destruction unless commit() was called - so a policy's `initialize` that
 // throws, after itself or another policy has already written to shared
 // state, leaves the registry as it was. The registry's mutable state is one
-// variable, registry_state<Registry>::st; its `policies` tuple is copied
-// whole, states of policies without an `initialize` included, since
-// restoring an untouched state is harmless, and simpler than picking. The
-// other members need no saving: initialize() only reads the class and
-// method lists, and write_global_data() replaces dispatch_data at commit
-// time only, after which nothing can throw - on rollback it still holds the
-// previous tables, which the classes' static_vptrs point into. (That is
-// also why a copy could not stand in for it: it would be another buffer.)
-template<class Registry>
+// variable, registry_state<Registry>::st. Its other members need no saving:
+// initialize() only reads the class and method lists, and write_global_data()
+// replaces dispatch_data at commit time only, after which nothing can throw -
+// on rollback it still holds the previous tables, which the classes'
+// static_vptrs point into. (That is also why a copy could not stand in for
+// it: it would be another buffer.)
+//
+// Only the states of the policies that are about to be initialized are saved.
+// Copying the `policies` tuple whole is simpler, but it is not harmless: a
+// state that no `initialize` touches is not derived data this call is about to
+// replace, it is configuration the caller owns. The error handler is the case
+// that bites - it is *called* from inside the window, by design
+// (fast_perfect_hash reports a search failure through it), so a handler that
+// disarms itself with set() before throwing would have that undone on the way
+// out. Saving wide also forces every policy state in the registry to be
+// copyable, including those of policies that have no `initialize` at all -
+// which rules out the std::ostringstream an `output` policy written to the
+// documented state pattern naturally holds - and copies each of them, vectors
+// and all, on every successful initialize().
+template<class Registry, class Context, class Options>
 class registry_state_transaction {
+    using policy_fns = mp11::mp_transform_q<
+        policy_fn_q<Registry>, typename Registry::policy_list>;
+    using saved_states = mp11::mp_transform<
+        policy_state_t,
+        mp11::mp_filter_q<
+            policy_state_is_volatile_q<Context, Options>, policy_fns>>;
+    using saved_type = mp11::mp_apply<detail::tuple, saved_states>;
+
+    static_assert(
+        mp11::mp_all_of<saved_states, std::is_copy_assignable>::value,
+        "the `state` of a policy that defines `initialize` must be copyable: "
+        "initialize() saves it, and puts it back if a policy throws");
+
+    // Element-wise: `saved` holds a subset of the registry's tuple.
+    template<class Tuple>
+    struct each;
+
+    template<class... States>
+    struct each<detail::tuple<States...>> {
+        static void save(detail::tuple<States...>& to) {
+            (...,
+             (detail::get<States>(to) =
+                  detail::get<States>(Registry::state().policies)));
+        }
+
+        static void restore(detail::tuple<States...>& from) {
+            (...,
+             (detail::get<States>(Registry::state().policies) =
+                  std::move(detail::get<States>(from))));
+        }
+    };
+
   public:
-    registry_state_transaction() : saved(Registry::state().policies) {
+    registry_state_transaction() {
+        each<saved_type>::save(saved);
     }
 
     ~registry_state_transaction() {
         if (!committed) {
-            Registry::state().policies = std::move(saved);
+            each<saved_type>::restore(saved);
         }
     }
 
@@ -157,7 +213,7 @@ class registry_state_transaction {
     }
 
   private:
-    typename registry_state_type<Registry>::policies_type saved;
+    saved_type saved;
     bool committed = false;
 };
 
@@ -1878,7 +1934,9 @@ void registry<Policies...>::compiler<Options...>::write_global_data() {
 
     ++tr << rflush(4, dispatch_data_size) << " " << gv_iter << " end\n";
 
-    detail::registry_state_transaction<registry> transaction;
+    detail::registry_state_transaction<
+        registry, compiler, std::tuple<Options...>>
+        transaction;
     detail::initialize_policies<registry>::fn(*this, options);
     transaction.commit();
 
@@ -2222,8 +2280,14 @@ void registry<Policies...>::compiler<Options...>::print_slots() {
 //! call: no static v-table pointer, `next` pointer, dispatch table or policy
 //! state is modified. The registry is nonetheless marked as not initialized,
 //! since that state does not reflect the current registrations; `initialize`
-//! must be called again, successfully, before any method is called. Policy
-//! states are restored from a copy, so a policy's `state` must be copyable.
+//! must be called again, successfully, before any method is called.
+//!
+//! Only the policies that define `initialize` have their `state` saved and
+//! restored, so only those states have to be copyable. A state that no
+//! `initialize` writes to is configuration, not derived data, and is left
+//! alone: an @ref error_handler policy is *called* during `initialize`, and a
+//! handler that changes the configuration - installing a different handler
+//! with `set`, say - keeps that change whether the call succeeds or throws.
 //!
 //! @par Example
 //!

test/test_initialize_policy_state_scope.cpp

@@ -0,0 +1,132 @@
+// Copyright (c) 2017-2026 Jean-Louis Leroy
+// 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)
+
+// initialize() saves and restores the state of the policies it initializes,
+// and only those. A state that no `initialize` writes to is configuration the
+// caller owns, not derived data the call is about to replace, so a failed
+// initialize() must leave it alone - the error handler is the case that
+// matters, since it is called from inside the transaction window by design.
+//
+// Two consequences, both checked here: such a state survives a rollback, and
+// it does not have to be copyable - which it would if the transaction copied
+// the whole policy tuple, ruling out the std::ostringstream an `output` policy
+// written to the documented state pattern naturally holds.
+
+#include <boost/openmethod.hpp>
+#include <boost/openmethod/initialize.hpp>
+
+#define BOOST_TEST_MODULE initialize_policy_state_scope
+#include <boost/test/unit_test.hpp>
+
+#include "test_util.hpp"
+
+#include <sstream>
+#include <stdexcept>
+#include <string>
+#include <tuple>
+#include <type_traits>
+
+using namespace boost::openmethod;
+
+// Configuration: it has a `state`, and deliberately no `initialize`. The state
+// is move-only, like the std::ostringstream it carries - the transaction must
+// not require it to be copyable.
+struct config_policy {
+    using category = config_policy;
+
+    template<class Registry>
+    struct fn {
+        struct state {
+            state() = default;
+            state(const state&) = delete;
+            auto operator=(const state&) -> state& = delete;
+
+            int generation = 0;
+            std::ostringstream os;
+        };
+    };
+};
+
+// Writes to its own state and to the configuration policy's - the way an error
+// handler called from inside the window changes the handler it installs - then
+// throws on demand.
+struct explosive_policy {
+    using category = explosive_policy;
+
+    template<class Registry>
+    struct fn {
+        struct state {
+            int generation = 0;
+        };
+
+        inline static bool armed = false;
+
+        template<class Context, class... Options>
+        static void initialize(const Context&, const std::tuple<Options...>&) {
+            ++Registry::template state<explosive_policy>().generation;
+            ++Registry::template state<config_policy>().generation;
+
+            if (armed) {
+                throw std::runtime_error("boom");
+            }
+        }
+    };
+};
+
+template<int N>
+struct test_registry :
+    test_registry_<N>::template with<config_policy, explosive_policy> {};
+
+using test_reg = test_registry<__COUNTER__>;
+using config_state = config_policy::fn<test_reg::registry_type>::state;
+using explosive = explosive_policy::fn<test_reg::registry_type>;
+
+// The point of the exercise: a policy that has no `initialize` may hold a
+// state the transaction could not copy even if it wanted to. If this ever
+// becomes copyable the test below stops proving anything.
+static_assert(!std::is_copy_constructible_v<config_state>);
+static_assert(!std::is_copy_assignable_v<config_state>);
+
+struct Animal {
+    virtual ~Animal() = default;
+};
+
+struct Dog : Animal {};
+
+struct BOOST_OPENMETHOD_ID(poke);
+
+using poke = method<
+    BOOST_OPENMETHOD_ID(poke), auto(virtual_<Animal&>)->std::string, test_reg>;
+
+auto poke_animal(Animal&) -> std::string {
+    return "silence";
+}
+
+BOOST_AUTO_TEST_CASE(config_state_survives_a_failed_initialize) {
+    BOOST_OPENMETHOD_REGISTER(use_classes<Animal, Dog, test_reg>);
+    BOOST_OPENMETHOD_REGISTER(poke::override<poke_animal>);
+
+    initialize<test_reg>();
+    BOOST_TEST(test_reg::state<explosive_policy>().generation == 1);
+    BOOST_TEST(test_reg::state<config_policy>().generation == 1);
+
+    explosive::armed = true;
+    BOOST_CHECK_THROW(initialize<test_reg>(), std::runtime_error);
+    explosive::armed = false;
+
+    // The initializing policy's own state is derived data: rolled back, so the
+    // second run's increment is undone.
+    BOOST_TEST(test_reg::state<explosive_policy>().generation == 1);
+
+    // The configuration policy's is not: the write made inside the window
+    // stands, exactly as an error handler that disarms itself before throwing
+    // would expect.
+    BOOST_TEST(test_reg::state<config_policy>().generation == 2);
+
+    // And the registry still works.
+    initialize<test_reg>();
+    Dog dog;
+    BOOST_TEST(poke::fn(dog) == "silence");
+}