From f7e9830c0c413f05737002dcc8d06e73cb379980 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 1 Jul 2015 10:45:14 +0200 Subject: Group state support --- build/algorithm.cxx | 28 +++++++++++++++++++++++----- build/algorithm.ixx | 33 ++++++++++++++++++++++++++++----- build/cli/rule | 3 --- build/cli/rule.cxx | 29 +---------------------------- build/cxx/rule.cxx | 6 +++--- build/operation.cxx | 2 +- build/target | 45 +++++++++++++++++++++++++++++++++++---------- build/target.cxx | 3 ++- 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 (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 , - // for details. + // reverse_execute_prerequisites(); see , + // 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 + noop_action (action, target&); // Defined in . + + target_state + group_action (action, target&); // Defined in . // 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 ()); - state = (f == nullptr || *f != &noop_action) - ? target_state::unknown - : target_state::unchanged; + raw_state = target_state::unknown; + + if (recipe_function** f = recipe_.target ()) + { + 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 // -- cgit v1.1