Implement ad hoc recipe sharing and locking

11 files changed, 270 insertions, 150 deletions

diff --git a/libbuild2/action.hxx b/libbuild2/action.hxx
index 5898ba8..906d7eb 100644
--- a/libbuild2/action.hxx
+++ b/libbuild2/action.hxx
@@ -61,6 +61,8 @@ namespace build2
   {
     action (): inner_id (0), outer_id (0) {} // Invalid action.
 
+    action (action_id a): action (a >> 4, a & 0xF) {}
+
     // If this is not a nested operation, then outer should be 0.
     //
     action (meta_operation_id m, operation_id inner, operation_id outer = 0)
@@ -103,6 +105,11 @@ namespace build2
   inline bool
   operator!= (action x, action y) {return !(x == y);}
 
+  inline bool operator== (action x, action_id y) {return x == action (y);}
+  inline bool operator!= (action x, action_id y) {return x != action (y);}
+  inline bool operator== (action_id x, action y) {return action (x) == y;}
+  inline bool operator!= (action_id x, action y) {return action (x) == y;}
+
   bool operator>  (action, action) = delete;
   bool operator<  (action, action) = delete;
   bool operator>= (action, action) = delete;
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index fe046ae..a3455ea 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -894,6 +894,14 @@ namespace build2
   }
 
   phase_switch::
+  phase_switch (phase_unlock&& u, phase_lock&& l)
+      : old_phase (u.l->phase), new_phase (l.phase)
+  {
+    phase_lock_instance = u.l; // Disarms phase_lock
+    u.l = nullptr;             // Disarms phase_unlock
+  }
+
+  phase_switch::
   ~phase_switch () noexcept (false)
   {
     phase_lock* pl (phase_lock_instance);
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 573b8d1..8e68784 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -553,9 +553,17 @@ namespace build2
   // Assuming we have a lock on the current phase, temporarily switch to a
   // new phase and switch back on destruction.
   //
+  // The second constructor can be used for a switch with an intermittent
+  // unlock:
+  //
+  // phase_unlock pu;
+  // phase_lock pl;
+  // phase_switch ps (move (pu), move (pl));
+  //
   struct LIBBUILD2_SYMEXPORT phase_switch
   {
-    explicit phase_switch (context&, run_phase);
+    phase_switch (context&, run_phase);
+    phase_switch (phase_unlock&&, phase_lock&&);
     ~phase_switch () noexcept (false);
 
     run_phase old_phase, new_phase;
diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx
index 34dc747..8dd8a6e 100644
--- a/libbuild2/dist/operation.cxx
+++ b/libbuild2/dist/operation.cxx
@@ -319,7 +319,7 @@ namespace build2
 
         ctx.current_operation (op_update, nullptr, false /* diag_noise */);
 
-        action a (perform_id, update_id);
+        action a (perform_update_id);
 
         mo_perform.match   (params, a, files,
                             1    /* diag (failures only) */,
diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx
index 4972d7b..ce6c9d6 100644
--- a/libbuild2/module.cxx
+++ b/libbuild2/module.cxx
@@ -66,14 +66,9 @@ namespace build2
   // Note: also used by ad hoc recipes thus not static.
   //
   void
-  create_module_context (context& ctx, const location& loc, const char* what)
+  create_module_context (context& ctx, const location& loc)
   {
     assert (ctx.module_context == nullptr);
-
-    if (!ctx.module_context_storage)
-      fail (loc) << "unable to update " << what <<
-        info << "updating of " << what << "s is disabled";
-
     assert (*ctx.module_context_storage == nullptr);
 
     // Since we are using the same scheduler, it makes sense to reuse the
@@ -98,7 +93,11 @@ namespace build2
     // Setup the context to perform update. In a sense we have a long-running
     // perform meta-operation batch (indefinite, in fact, since we never call
     // the meta-operation's *_post() callbacks) in which we periodically
-    // execute the update operation.
+    // execute update operations.
+    //
+    // Note that we perform each build in a separate update operation. Failed
+    // that, if the same target is update twice (which may happen with ad hoc
+    // recipes) we will see the old state.
     //
     if (mo_perform.meta_operation_pre != nullptr)
       mo_perform.meta_operation_pre ({} /* parameters */, loc);
@@ -107,88 +106,74 @@ namespace build2
 
     if (mo_perform.operation_pre != nullptr)
       mo_perform.operation_pre ({} /* parameters */, update_id);
-
-    ctx.module_context->current_operation (op_update);
   }
 
   // Note: also used by ad hoc recipes thus not static.
   //
   const target&
   update_in_module_context (context& ctx, const scope& rs, names tgt,
-                            const location& loc, const path& bf,
-                            const char* what, const char* name)
+                            const location& loc, const path& bf)
   {
-    action_targets tgs;
-    {
-      action a (perform_id, update_id);
-
-      // Cutoff the existing diagnostics stack and push our own entry.
-      //
-      diag_frame::stack_guard diag_cutoff (nullptr);
-
-      auto df = make_diag_frame (
-        [&loc, what, name] (const diag_record& dr)
-        {
-          dr << info (loc) << "while " << what;
+    // New update operation.
+    //
+    ctx.module_context->current_operation (op_update);
 
-          if (name != nullptr)
-            dr << ' ' << name;
-        });
+    // Un-tune the scheduler.
+    //
+    // Note that we can only do this if we are running serially because
+    // otherwise we cannot guarantee the scheduler is idle (we could have
+    // waiting threads from the outer context). This is fine for now since the
+    // only two tuning level we use are serial and full concurrency (turns out
+    // currently we don't really need this: we will always be called during
+    // load or match phases and we always do parallel match; but let's keep it
+    // in case things change).
+    //
+    auto sched_tune (ctx.sched.serial ()
+                     ? scheduler::tune_guard (ctx.sched, 0)
+                     : scheduler::tune_guard ());
+
+    // Remap verbosity level 0 to 1 unless we were requested to be silent.
+    // Failed that, we may have long periods of seemingly nothing happening
+    // while we quietly update the module, which may look like things have
+    // hung up.
+    //
+    // @@ CTX: modifying global verbosity level won't work if we have multiple
+    //         top-level contexts running in parallel.
+    //
+    auto verbg = make_guard (
+      [z = !silent && verb == 0 ? (verb = 1, true) : false] ()
+      {
+        if (z)
+          verb = 0;
+      });
+
+    // Note that for now we suppress progress since it would clash with the
+    // progress of what we are already doing (maybe in the future we can do
+    // save/restore but then we would need some sort of diagnostics that we
+    // have switched to another task).
+    //
+    action a (perform_update_id);
+    action_targets tgs;
 
-      // Un-tune the scheduler.
-      //
-      // Note that we can only do this if we are running serially because
-      // otherwise we cannot guarantee the scheduler is idle (we could have
-      // waiting threads from the outer context). This is fine for now since
-      // the only two tuning level we use are serial and full concurrency
-      // (turns out currently we don't really need this: we will always be
-      // called during load or match phases and we always do parallel match;
-      // but let's keep it in case things change).
-      //
-      auto sched_tune (ctx.sched.serial ()
-                       ? scheduler::tune_guard (ctx.sched, 0)
-                       : scheduler::tune_guard ());
-
-      // Remap verbosity level 0 to 1 unless we were requested to be silent.
-      // Failed that, we may have long periods of seemingly nothing happening
-      // while we quietly update the module, which may look like things have
-      // hung up.
-      //
-      // @@ CTX: modifying global verbosity level won't work if we have
-      //         multiple top-level contexts running in parallel.
-      //
-      auto verbg = make_guard (
-        [z = !silent && verb == 0 ? (verb = 1, true) : false] ()
-        {
-          if (z)
-            verb = 0;
-        });
-
-      // Note that for now we suppress progress since it would clash with
-      // the progress of what we are already doing (maybe in the future we
-      // can do save/restore but then we would need some sort of
-      // diagnostics that we have switched to another task).
-      //
-      mo_perform.search  ({},      /* parameters */
-                          rs,      /* root scope */
-                          rs,      /* base scope */
-                          bf,      /* buildfile */
-                          rs.find_target_key (tgt, loc),
-                          loc,
-                          tgs);
-
-      mo_perform.match   ({},      /* parameters */
-                          a,
-                          tgs,
-                          1,       /* diag (failures only) */
-                          false    /* progress */);
-
-      mo_perform.execute ({},      /* parameters */
-                          a,
-                          tgs,
-                          1,       /* diag (failures only) */
-                          false    /* progress */);
-    }
+    mo_perform.search  ({},      /* parameters */
+                        rs,      /* root scope */
+                        rs,      /* base scope */
+                        bf,      /* buildfile */
+                        rs.find_target_key (tgt, loc),
+                        loc,
+                        tgs);
+
+    mo_perform.match   ({},      /* parameters */
+                        a,
+                        tgs,
+                        1,       /* diag (failures only) */
+                        false    /* progress */);
+
+    mo_perform.execute ({},      /* parameters */
+                        a,
+                        tgs,
+                        1,       /* diag (failures only) */
+                        false    /* progress */);
 
     assert (tgs.size () == 1);
     return tgs[0].as<target> ();
@@ -348,7 +333,13 @@ namespace build2
       // Create the build context if necessary.
       //
       if (ctx.module_context == nullptr)
-        create_module_context (ctx, loc, "build system module");
+      {
+        if (!ctx.module_context_storage)
+          fail (loc) << "unable to update build system module " << mod <<
+            info << "building of build system modules is disabled";
+
+        create_module_context (ctx, loc);
+      }
 
       // Inherit loaded_modules lock from the outer context.
       //
@@ -378,17 +369,27 @@ namespace build2
       }
       else
       {
-        const scope& rs (lr.second);
+        const target* l;
+        {
+          // Cutoff the existing diagnostics stack and push our own entry.
+          //
+          diag_frame::stack_guard diag_cutoff (nullptr);
 
-        const target& l (
-          update_in_module_context (
-            ctx, rs, move (lr.first),
-            loc, path (), "loading build system module", mod.c_str ()));
+          auto df = make_diag_frame (
+            [&loc, &mod] (const diag_record& dr)
+            {
+              dr << info (loc) << "while loading build system module " << mod;
+            });
+
+          l = &update_in_module_context (
+            ctx, lr.second, move (lr.first),
+            loc, path ());
+        }
 
-        if (!l.is_a ("libs"))
+        if (!l->is_a ("libs"))
           fail (loc) << "wrong export from build system module " << mod;
 
-        lib = l.as<file> ().path ();
+        lib = l->as<file> ().path ();
 
         l5 ([&]{trace << "updated " << lib;});
       }
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 8d6beea..588968e 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -722,8 +722,12 @@ namespace build2
             // that token part of the replay (we cannot peek past the replay
             // sequence).
             //
-            auto parse = [this, &st] (token& t, type& tt,
-                                      const target_type* type, string pat)
+            auto parse = [
+              this,
+              &st,
+              recipes = small_vector<shared_ptr<adhoc_rule>, 1> ()]
+              (token& t, type& tt,
+               const target_type* type, string pat) mutable
             {
               token rt; // Recipe start token.
 
@@ -752,7 +756,7 @@ namespace build2
               if (type != nullptr)
                 fail (rt) << "recipe in target type/pattern";
 
-              parse_recipe (t, tt, rt);
+              parse_recipe (t, tt, rt, recipes);
             };
 
             for_each (parse);
@@ -1012,7 +1016,9 @@ namespace build2
   }
 
   void parser::
-  parse_recipe (token& t, type& tt, const token& start)
+  parse_recipe (token& t, type& tt,
+                const token& start,
+                small_vector<shared_ptr<adhoc_rule>, 1>& recipes)
   {
     // Parse a recipe chain.
     //
@@ -1038,11 +1044,11 @@ namespace build2
         default_target_ = target_;
     }
 
-    // True if seen a recipe that requires cleanup.
-    //
-    bool clean (false);
+    bool first (recipes.empty ()); // First target.
+    bool clean (false);            // Seen a recipe that requires cleanup.
 
-    for (token st (start);; st = t)
+    token st (start);
+    for (size_t i (0);; st = t, ++i)
     {
       optional<string> diag;
 
@@ -1111,31 +1117,37 @@ namespace build2
       //    me now.
       //
       shared_ptr<adhoc_rule> ar;
+      if (first)
+      {
+        // Note that this is always the location of the opening multi-curly-
+        // brace, whether we have the header or not. This is relied upon by
+        // the rule implementations (e.g., to calculate the first line of the
+        // recipe code).
+        //
+        location loc (get_location (st));
 
-      // Note that this is always the location of the opening multi-curly-
-      // brace, whether we have the header or not. This is relied upon by the
-      // rule implementations (e.g., to calculate the first line of the recipe
-      // code).
-      //
-      location loc (get_location (st));
+        if (!lang)
+        {
+          ar.reset (new adhoc_script_rule (move (t.value),
+                                           move (diag),
+                                           loc,
+                                           st.value.size ()));
+        }
+        else if (*lang == "c++")
+        {
+          ar.reset (new adhoc_cxx_rule (move (t.value), loc, st.value.size ()));
+          clean = true;
+        }
+        else
+          fail (lloc) << "unknown recipe language '" << *lang << "'";
 
-      if (!lang)
-      {
-        ar.reset (new adhoc_script_rule (move (t.value),
-                                         move (diag),
-                                         loc,
-                                         st.value.size ()));
-      }
-      else if (*lang == "c++")
-      {
-        ar.reset (new adhoc_cxx_rule (move (t.value), loc, st.value.size ()));
-        clean = true;
+        recipes.push_back (ar);
       }
       else
-        fail (lloc) << "unknown recipe language '" << *lang << "'";
+        ar = recipes[i];
 
-      action a (perform_id, update_id);
-      target_->adhoc_recipes.push_back (adhoc_recipe {a, move (ar)});
+      target_->adhoc_recipes.push_back (
+        adhoc_recipe {perform_update_id, move (ar)});
 
       next (t, tt);
       assert (tt == type::multi_rcbrace);
@@ -1152,7 +1164,7 @@ namespace build2
     //
     if (clean)
     {
-      action a (perform_id, clean_id);
+      action a (perform_clean_id);
       auto f (&adhoc_rule::clean_recipes_build);
 
       // First check if we have already done this.
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index 5b930c5..7a44e65 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -112,7 +112,9 @@ namespace build2
                           string = string ());
 
     void
-    parse_recipe (token&, token_type&, const token&);
+    parse_recipe (token&, token_type&,
+                  const token&,
+                  small_vector<shared_ptr<adhoc_rule>, 1>&);
 
     // Ad hoc target names inside < ... >.
     //
diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx
index ac45305..a4ae830 100644
--- a/libbuild2/rule.cxx
+++ b/libbuild2/rule.cxx
@@ -17,6 +17,13 @@ using namespace butl;
 
 namespace build2
 {
+  // rule (vtable)
+  //
+  rule::
+  ~rule ()
+  {
+  }
+
   // file_rule
   //
   // Note that this rule is special. It is the last, fallback rule. If
@@ -592,6 +599,12 @@ namespace build2
 
   // adhoc_cxx_rule
   //
+  adhoc_cxx_rule::
+  ~adhoc_cxx_rule ()
+  {
+    delete impl.load (memory_order_relaxed); // Serial execution.
+  }
+
   void adhoc_cxx_rule::
   dump (ostream& os, const string& ind) const
   {
@@ -606,12 +619,11 @@ namespace build2
   // From module.cxx.
   //
   void
-  create_module_context (context&, const location&, const char* what);
+  create_module_context (context&, const location&);
 
   const target&
   update_in_module_context (context&, const scope&, names tgt,
-                            const location&, const path& bf,
-                            const char* what, const char* name);
+                            const location&, const path& bf);
 
   pair<void*, void*>
   load_module_library (const path& lib, const string& sym, string& err);
@@ -650,10 +662,21 @@ namespace build2
     //
     string id (sha256 (code).abbreviated_string (12));
 
-    // @@ TODO: locking.
-    // @@ Need to unlock phase while waiting.
-    if (impl == nullptr)
+    // We use the relaxed memory order because any change must go through the
+    // serial load phase. In other words, all we need here is atomicity with
+    // ordering/visibility provided by the phase mutex.
+    //
+    cxx_rule* impl (this->impl.load (memory_order_relaxed));
+
+    while (impl == nullptr) // Breakout loop.
     {
+      // Switch the phase to (serial) load and re-check.
+      //
+      phase_switch ps (ctx, run_phase::load);
+
+      if ((impl = this->impl.load (memory_order_relaxed)) != nullptr)
+        break;
+
       using create_function = cxx_rule* (const location&);
       using load_function = create_function* ();
 
@@ -663,10 +686,6 @@ namespace build2
 
       string sym ("load_" + id);
 
-      // Switch the phase to load.
-      //
-      phase_switch ps (ctx, run_phase::load);
-
       optional<bool> altn (false); // Standard naming scheme.
       if (!is_src_root (pd, altn))
       {
@@ -734,7 +753,7 @@ namespace build2
 
           // If we want the user to be able to supply a custom constuctor,
           // then we have to give the class a predictable name (i.e., we
-          // cannot use id as part of its name) and put it into an anonymous
+          // cannot use id as part of its name) and put it into an unnamed
           // namespace. One clever idea is to call the class `constructor` but
           // the name could also be used for a custom destructor (still could
           // work) or for name qualification (would definitely look bizarre).
@@ -833,14 +852,22 @@ namespace build2
         }
       }
 
-      const target* l;
+      // Update the library target in the module context.
+      //
+      const target* l (nullptr);
       {
         bool nested (ctx.module_context == &ctx);
 
         // Create the build context if necessary.
         //
         if (ctx.module_context == nullptr)
-          create_module_context (ctx, loc, "ad hoc recipe");
+        {
+          if (!ctx.module_context_storage)
+            fail (loc) << "unable to update ad hoc recipe for target " << t <<
+              info << "building of ad hoc recipes is disabled";
+
+          create_module_context (ctx, loc);
+        }
 
         // "Switch" to the module context.
         //
@@ -848,26 +875,63 @@ namespace build2
 
         // Load the project in the module context.
         //
+        // Note that it's possible it has already been loaded (see above about
+        // the id calculation).
+        //
         path bf (pd / std_buildfile_file);
         scope& rs (load_project (ctx, pd, pd, false /* forwarded */));
-        source (rs, rs, bf);
 
-        if (nested)
+        // If the project has already been loaded then, as an optimization,
+        // check if the target has already been updated (this will make a
+        // difference we if we have identical recipes in several buildfiles).
+        //
+        if (!source_once (rs, rs, bf))
         {
-          // @@ TODO: we probably want to make this work.
+          const target_type* tt (rs.find_target_type ("libs"));
+          assert (tt != nullptr);
 
-          fail (loc) << "nested ad hoc recipe updates not yet supported" << endf;
+          l = ctx.targets.find (*tt, pd, dir_path () /* out */, id);
+          assert (l != nullptr);
+
+          if (l->executed_state (perform_update_id) == target_state::unknown)
+            l = nullptr;
         }
-        else
+
+        if (l == nullptr)
         {
-          l = &update_in_module_context (
-            ctx, rs, names {name (pd, "libs", id)},
-            loc, bf, "updating ad hoc recipe", nullptr);
+          if (nested)
+          {
+            // @@ TODO: we probably want to make this work.
+
+            fail (loc) << "nested ad hoc recipe updates not yet supported";
+          }
+          else
+          {
+            // Cutoff the existing diagnostics stack and push our own entry.
+            //
+            diag_frame::stack_guard diag_cutoff (nullptr);
+
+            auto df = make_diag_frame (
+              [this, &t] (const diag_record& dr)
+              {
+                dr << info (loc) << "while updating ad hoc recipe for target "
+                   << t;
+              });
+
+            l = &update_in_module_context (
+              ctx, rs, names {name (pd, "libs", id)},
+              loc, bf);
+          }
         }
       }
 
+      // Load the library.
+      //
       const path& lib (l->as<file> ().path ());
 
+      // Note again that it's possible the library has already been loaded
+      // (see above about the id calculation).
+      //
       string err;
       pair<void*, void*> hs (load_module_library (lib, sym, err));
 
@@ -891,7 +955,8 @@ namespace build2
         load_function* lf (function_cast<load_function*> (hs.second));
         create_function* cf (lf ());
 
-        impl.reset (cf (loc));
+        impl = cf (loc);
+        this->impl.store (impl, memory_order_relaxed); // Still in load phase.
       }
     }
 
@@ -901,6 +966,6 @@ namespace build2
   recipe adhoc_cxx_rule::
   apply (action a, target& t) const
   {
-    return impl->apply (a, t);
+    return impl.load (memory_order_relaxed)->apply (a, t);
   }
 }
diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx
index ca17f2e..79747d5 100644
--- a/libbuild2/rule.hxx
+++ b/libbuild2/rule.hxx
@@ -22,7 +22,7 @@ namespace build2
   //
   // Note: match() is only called once but may not be followed by apply().
   //
-  class rule
+  class LIBBUILD2_SYMEXPORT rule
   {
   public:
     virtual bool
@@ -33,6 +33,9 @@ namespace build2
 
     rule () = default;
 
+    virtual
+    ~rule ();
+
     rule (const rule&) = delete;
     rule& operator= (const rule&) = delete;
   };
@@ -216,11 +219,17 @@ namespace build2
     dump (ostream&, const string&) const override;
 
     adhoc_cxx_rule (string c, const location& l, size_t b)
-      : adhoc_rule (l, b), code (move (c)) {}
+      : adhoc_rule (l, b), code (move (c)), impl (nullptr) {}
+
+    virtual
+    ~adhoc_cxx_rule () override;
 
   public:
-    string                       code;
-    mutable unique_ptr<cxx_rule> impl;
+    // Note that this recipe (rule instance) can be shared between multiple
+    // targets which could all be matched in parallel.
+    //
+    const string              code;
+    mutable atomic<cxx_rule*> impl;
   };
 }
 
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index b237f1f..932aa22 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -583,6 +583,9 @@ namespace build2
     // This function can only be called during execution if we have observed
     // (synchronization-wise) that this target has been executed.
     //
+    // It can also be called during the serial load phase (but make sure you
+    // understand what you are doing).
+    //
     target_state
     executed_state (action, bool fail = true) const;
 
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index bb30c9c..36ca3c8 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -114,8 +114,6 @@ namespace build2
   inline pair<bool, target_state> target::
   matched_state_impl (action a) const
   {
-    assert (ctx.phase == run_phase::match);
-
     // Note that the "tried" state is "final".
     //
     const opstate& s (state[a]);
@@ -138,13 +136,14 @@ namespace build2
   inline target_state target::
   executed_state_impl (action a) const
   {
-    assert (ctx.phase == run_phase::execute);
     return (group_state (a) ? group->state : state)[a].state;
   }
 
   inline target_state target::
   matched_state (action a, bool fail) const
   {
+    assert (ctx.phase == run_phase::match);
+
     // Note that the target could be being asynchronously re-matched.
     //
     pair<bool, target_state> r (matched_state_impl (a));
@@ -158,6 +157,8 @@ namespace build2
   inline pair<bool, target_state> target::
   try_matched_state (action a, bool fail) const
   {
+    assert (ctx.phase == run_phase::match);
+
     pair<bool, target_state> r (matched_state_impl (a));
 
     if (fail && r.first && r.second == target_state::failed)
@@ -169,6 +170,8 @@ namespace build2
   inline target_state target::
   executed_state (action a, bool fail) const
   {
+    assert (ctx.phase == run_phase::execute || ctx.phase == run_phase::load);
+
     target_state r (executed_state_impl (a));
 
     if (fail && r == target_state::failed)
@@ -193,6 +196,8 @@ namespace build2
   inline bool target::
   unchanged (action a) const
   {
+    assert (ctx.phase == run_phase::match);
+
     return matched_state_impl (a).second == target_state::unchanged;
   }