Group state support

8 files changed, 93 insertions, 56 deletions

diff --git a/build/algorithm.cxx b/build/algorithm.cxx
index 40d4b0b..bec51aa 100644
--- a/build/algorithm.cxx
+++ b/build/algorithm.cxx
@@ -278,25 +278,26 @@ namespace build
   {
     // Implementation with some multi-threading ideas in mind.
     //
-    switch (target_state ts = t.state)
+    switch (t.raw_state)
     {
+    case target_state::group:
     case target_state::unknown:
     case target_state::postponed:
       {
-        t.state = target_state::failed; // So the rule can just throw.
+        t.raw_state = target_state::failed; // So the rule can just throw.
 
         auto g (
           make_exception_guard (
             [](action a, target& t){info << "while " << diag_doing (a, t);},
             a, t));
 
-        ts = t.recipe (a) (a, t);
+        target_state ts (t.recipe (a) (a, t));
         assert (ts != target_state::unknown && ts != target_state::failed);
 
         // The recipe may have set the target's state manually.
         //
-        if (t.state == target_state::failed)
-          t.state = ts;
+        if (t.raw_state == target_state::failed)
+          t.raw_state = ts;
 
         return ts;
       }
@@ -395,6 +396,23 @@ namespace build
   }
 
   target_state
+  group_action (action a, target& t)
+  {
+    target_state r (execute (a, *t.group));
+
+    // The standard execute() logic sets the state to failed just
+    // before calling the recipe (so that the recipe can just throw
+    // to indicate a failure). After the recipe is successfully
+    // executed and unless the recipe has updated the state manually,
+    // the recipe's return value is set as the new state. But we
+    // don't want that. So we are going to set it manually.
+    //
+    t.raw_state = target_state::group;
+
+    return r;
+  }
+
+  target_state
   default_action (action a, target& t)
   {
     return current_mode == execution_mode::first
diff --git a/build/algorithm.ixx b/build/algorithm.ixx
index ddd63f6..d4d0ec8 100644
--- a/build/algorithm.ixx
+++ b/build/algorithm.ixx
@@ -95,16 +95,39 @@ namespace build
   {
     t.dependents--;
 
-    switch (t.state)
+    switch (target_state ts = t.state ())
     {
     case target_state::unchanged:
-    case target_state::changed: return t.state;
+    case target_state::changed: return ts;
     default:
       {
         // Handle the "last" execution mode.
         //
+        // This gets interesting when we consider interaction with
+        // groups. It seem to make sense to treat group members as
+        // dependents of the group, so, for example, if we try to
+        // clean the group via three of its members, only the last
+        // attempt will actually execute the clean. This means that
+        // when we match a group member, inside we should also match
+        // the group in order to increment the dependents count.
+        // Though this seems to be a natural requirement (if we
+        // are delegating to the group, we need to find a recipe
+        // for it, just like we would for a prerequisite).
+        //
+        // Note that below we are going to change the group state
+        // to postponed. This is not a mistake: until we execute
+        // the recipe, we want to keep returning postponed. And
+        // once the recipe is executed, it will reset the state
+        // to group (see group_action()). To put it another way,
+        // the execution of this member is postponed, not of the
+        // group.
+        //
+        // One important invariant to keep in mind: the return
+        // value from execute() should always be the same as what
+        // would get returned by a subsequent call to state().
+        //
         if (current_mode == execution_mode::last && t.dependents != 0)
-          return (t.state = target_state::postponed);
+          return (t.raw_state = target_state::postponed);
 
         return execute_impl (a, t);
       }
@@ -114,10 +137,10 @@ namespace build
   inline target_state
   execute_direct (action a, target& t)
   {
-    switch (t.state)
+    switch (target_state ts = t.state ())
     {
     case target_state::unchanged:
-    case target_state::changed: return t.state;
+    case target_state::changed: return ts;
     default: return execute_impl (a, t);
     }
   }
diff --git a/build/cli/rule b/build/cli/rule
index d52d0e0..21fcf8a 100644
--- a/build/cli/rule
+++ b/build/cli/rule
@@ -25,9 +25,6 @@ namespace build
 
       static target_state
       perform_clean (action, target&);
-
-      static target_state
-      delegate (action, target&);
     };
   }
 }
diff --git a/build/cli/rule.cxx b/build/cli/rule.cxx
index 2d001c4..6377b29 100644
--- a/build/cli/rule.cxx
+++ b/build/cli/rule.cxx
@@ -178,7 +178,7 @@ namespace build
       {
         cli_cxx& g (*static_cast<cli_cxx*> (mr.target));
         build::match (a, g);
-        return &delegate;
+        return group_recipe;
       }
     }
 
@@ -281,25 +281,6 @@ namespace build
         }
       }
 
-      // Update member recipes. Without that the state update below
-      // won't stick.
-      //
-      if (!t.h ()->recipe (a))
-        t.h ()->recipe (a, &delegate);
-
-      if (!t.c ()->recipe (a))
-        t.c ()->recipe (a, &delegate);
-
-      if (t.i () != nullptr && !t.i ()->recipe (a))
-        t.i ()->recipe (a, &delegate);
-
-      // Update member states.
-      //
-      t.h ()->state = ts;
-      t.c ()->state = ts;
-      if (t.i () != nullptr)
-        t.i ()->state = ts;
-
       return ts;
     }
 
@@ -333,13 +314,5 @@ namespace build
 
       return r ? target_state::changed : ts;
     }
-
-    target_state compile::
-    delegate (action a, target& t)
-    {
-      // Delegate to our group.
-      //
-      return execute (a, *t.group);
-    }
   }
 }
diff --git a/build/cxx/rule.cxx b/build/cxx/rule.cxx
index 0812922..08a9202 100644
--- a/build/cxx/rule.cxx
+++ b/build/cxx/rule.cxx
@@ -663,10 +663,10 @@ namespace build
                 // have been in target_state::changed because of a dependency
                 // extraction run for some other source file.
                 //
-                target_state os (pt.state);
-                execute_direct (a, pt);
+                target_state os (pt.state ());
+                target_state ns (execute_direct (a, pt));
 
-                if (pt.state != os && pt.state != target_state::unchanged)
+                if (ns != os && ns != target_state::unchanged)
                 {
                   level5 ([&]{trace << "updated " << pt << ", restarting";});
                   restart = true;
diff --git a/build/operation.cxx b/build/operation.cxx
index f0594ad..4c32d42 100644
--- a/build/operation.cxx
+++ b/build/operation.cxx
@@ -126,7 +126,7 @@ namespace build
     //
     for (target& t: psp)
     {
-      switch (t.state)
+      switch (t.state ())
       {
       case target_state::postponed:
         {
diff --git a/build/target b/build/target
index 9587c6a..000496c 100644
--- a/build/target
+++ b/build/target
@@ -36,7 +36,15 @@ namespace build
 
   // Target state.
   //
-  enum class target_state {unknown, postponed, unchanged, changed, failed};
+  enum class target_state
+  {
+    group,           // Target's state is the group's state.
+    unknown,
+    postponed,
+    unchanged,
+    changed,
+    failed
+  };
 
   std::ostream&
   operator<< (std::ostream&, target_state);
@@ -68,15 +76,20 @@ namespace build
   // on all the prerequisites in a loop, skipping ignored. Specially,
   // for actions with the "first" execution mode, it calls
   // execute_prerequisites() while for those with the "last" mode --
-  // reverse_execute_prerequisites(); see <operation>, <algorithm>
-  // for details.
+  // reverse_execute_prerequisites(); see <build/operation>,
+  // <build/algorithm> for details. The group recipe calls the group's
+  // recipe.
   //
   extern const recipe empty_recipe;
   extern const recipe noop_recipe;
   extern const recipe default_recipe;
+  extern const recipe group_recipe;
 
   target_state
-  noop_action (action, target&); // Defined in <algorithm>
+  noop_action (action, target&); // Defined in <build/algorithm>.
+
+  target_state
+  group_action (action, target&); // Defined in <build/algorithm>.
 
   // Prerequisite references as used in the target::prerequisites list
   // below.
@@ -264,7 +277,13 @@ namespace build
     }
 
   public:
-    target_state state;
+    target_state raw_state;
+
+    target_state
+    state () const
+    {
+      return raw_state != target_state::group ? raw_state : group->raw_state;
+    }
 
     // Number of direct targets that depend on this target in the current
     // action. It is incremented during the match phase and then decremented
@@ -296,12 +315,18 @@ namespace build
 
       // Also reset the target state. If this is a noop recipe, then
       // mark the target unchanged so that we don't waste time executing
-      // the recipe.
+      // the recipe. If this is a group recipe, then mark the state as
+      // coming from the group.
       //
-      recipe_function** f (recipe_.target<recipe_function*> ());
-      state = (f == nullptr || *f != &noop_action)
-        ? target_state::unknown
-        : target_state::unchanged;
+      raw_state = target_state::unknown;
+
+      if (recipe_function** f = recipe_.target<recipe_function*> ())
+      {
+        if (*f == &noop_action)
+          raw_state = target_state::unchanged;
+        else if (*f == &group_action)
+          raw_state = target_state::group;
+      }
 
       dependents = 0;
     }
diff --git a/build/target.cxx b/build/target.cxx
index 7958a85..f692a5e 100644
--- a/build/target.cxx
+++ b/build/target.cxx
@@ -32,7 +32,7 @@ namespace build
   // target_state
   //
   static const char* target_state_[] = {
-    "unknown", "postponed", "unchanged", "changed", "failed"};
+    "group", "unknown", "postponed", "unchanged", "changed", "failed"};
 
   ostream&
   operator<< (ostream& os, target_state ts)
@@ -45,6 +45,7 @@ namespace build
   const recipe empty_recipe;
   const recipe noop_recipe (&noop_action);
   const recipe default_recipe (&default_action);
+  const recipe group_recipe (&group_action);
 
   // target
   //