From 4be404cd8b7f4c7b450364defea92cd02e9b7a62 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 21 Jul 2015 15:51:54 +0200 Subject: Improve nested operations support The idea is this: we need to be able to override "conditional, inner for outer" recipes with the "unconditional inner" ones. Here is the concrete scenario: we have {update, test} action and the lib{} target that is both mentioned as a prerequisite of ./ and exe{}, which is a test. At first, we want to ignore lib{} when reached as a prerequisite of ./. But then we get to it via exe{} (which is a test and thus should be updated). At this point we should override the recipe for lib{} with the one that would update it rather than ignore. --- build/algorithm.cxx | 165 +++++++++++++++++++++++++++------------------------ build/algorithm.ixx | 12 +++- build/cli/target | 2 +- build/cli/target.cxx | 2 +- build/operation | 14 +++++ build/target | 14 +++-- build/target.cxx | 2 +- 7 files changed, 124 insertions(+), 87 deletions(-) diff --git a/build/algorithm.cxx b/build/algorithm.cxx index 290bfa5..c9dcbfe 100644 --- a/build/algorithm.cxx +++ b/build/algorithm.cxx @@ -41,10 +41,10 @@ namespace build return create_new_target (pk); } - pair + match_result_impl match_impl (action a, target& t, bool apply) { - pair r (nullptr, nullptr); + match_result_impl r; // Clear the resolved targets list before calling match(). The rule // is free to, say, resize() this list in match() (provided that it @@ -52,75 +52,90 @@ namespace build // t.prerequisite_targets.clear (); - size_t oi (a.operation () - 1); // Operation index in rule_map. - scope& bs (t.base_scope ()); - - for (auto tt (&t.type ()); - tt != nullptr && !t.recipe (a); - tt = tt->base) + // If this is a nested operation, first try the outer operation. + // This allows a rule to implement a "precise match", that is, + // both inner and outer operations match. + // + for (operation_id oo (a.outer_operation ()), io (a.operation ()), + o (oo != 0 ? oo : io); o != 0; o = (oo != 0 ? io : 0)) { - // Search scopes outwards, stopping at the project root. + // Adjust action for recipe: on the first iteration we want it + // {inner, outer} (which is the same as 'a') while on the second + // -- {inner, 0}. Note that {inner, 0} is the same or "stronger" + // (i.e., overrides; see action::operator<()) than 'a'. This + // allows "unconditional inner" to override "inner for outer" + // recipes. // - for (const scope* s (&bs); - s != nullptr; - s = s->root () ? global_scope : s->parent_scope ()) - { - const rule_map& om (s->rules); + action ra (a.meta_operation (), io, o != oo ? 0 : oo); - if (om.size () <= oi) - continue; // No entry for this operation id. + size_t oi (o - 1); // Operation index in rule_map. + scope& bs (t.base_scope ()); - const target_type_rule_map& ttm (om[oi]); + for (auto tt (&t.type ()); + tt != nullptr && !t.recipe (ra); + tt = tt->base) + { + // Search scopes outwards, stopping at the project root. + // + for (const scope* s (&bs); + s != nullptr; + s = s->root () ? global_scope : s->parent_scope ()) + { + const rule_map& om (s->rules); - if (ttm.empty ()) - continue; // Empty map for this operation id. + if (om.size () <= oi) + continue; // No entry for this operation id. - auto i (ttm.find (tt->id)); + const target_type_rule_map& ttm (om[oi]); - if (i == ttm.end () || i->second.empty ()) - continue; // No rules registered for this target type. + if (ttm.empty ()) + continue; // Empty map for this operation id. - const auto& rules (i->second); // Hint map. + auto i (ttm.find (tt->id)); - // @@ TODO - // - // Different rules can be used for different operations (update - // vs test is a good example). So, at some point, we will probably - // have to support a list of hints or even an operation-hint map - // (e.g., 'hint=cxx test=foo' if cxx supports the test operation - // but we want the foo rule instead). This is also the place where - // the '{build clean}=cxx' construct (which we currently do not - // support) can come handy. - // - // Also, ignore the hint (that is most likely ment for a different - // operation) if this is a unique match. - // - string hint; - auto rs (rules.size () == 1 - ? make_pair (rules.begin (), rules.end ()) - : rules.find_prefix (hint)); + if (i == ttm.end () || i->second.empty ()) + continue; // No rules registered for this target type. - for (auto i (rs.first); i != rs.second; ++i) - { - const string& n (i->first); - const rule& ru (i->second); + const auto& rules (i->second); // Hint map. - match_result m; - { - auto g ( - make_exception_guard ( - [](action a, target& t, const string& n) - { - info << "while matching rule " << n << " to " - << diag_do (a, t); - }, - a, t, n)); - - m = ru.match (a, t, hint); - } + // @@ TODO + // + // Different rules can be used for different operations (update + // vs test is a good example). So, at some point, we will probably + // have to support a list of hints or even an operation-hint map + // (e.g., 'hint=cxx test=foo' if cxx supports the test operation + // but we want the foo rule instead). This is also the place where + // the '{build clean}=cxx' construct (which we currently do not + // support) can come handy. + // + // Also, ignore the hint (that is most likely ment for a different + // operation) if this is a unique match. + // + string hint; + auto rs (rules.size () == 1 + ? make_pair (rules.begin (), rules.end ()) + : rules.find_prefix (hint)); - if (m) + for (auto i (rs.first); i != rs.second; ++i) { + const string& n (i->first); + const rule& ru (i->second); + + match_result m; + { + auto g ( + make_exception_guard ( + [](action a, target& t, const string& n) + { + info << "while matching rule " << n << " to " + << diag_do (a, t); + }, + ra, t, n)); + + if (!(m = ru.match (ra, t, hint))) + continue; + } + // Do the ambiguity test. // bool ambig (false); @@ -132,7 +147,6 @@ namespace build const string& n1 (i->first); const rule& ru1 (i->second); - match_result m1; { auto g ( make_exception_guard ( @@ -141,22 +155,20 @@ namespace build info << "while matching rule " << n1 << " to " << diag_do (a, t); }, - a, t, n1)); + ra, t, n1)); - m1 = ru1.match (a, t, hint); + if (!ru1.match (ra, t, hint)) + continue; } - if (m1) + if (!ambig) { - if (!ambig) - { - dr << fail << "multiple rules matching " << diag_doing (a, t) - << info << "rule " << n << " matches"; - ambig = true; - } - - dr << info << "rule " << n1 << " also matches"; + dr << fail << "multiple rules matching " << diag_doing (ra, t) + << info << "rule " << n << " matches"; + ambig = true; } + + dr << info << "rule " << n1 << " also matches"; } if (!ambig) @@ -170,14 +182,15 @@ namespace build info << "while applying rule " << n << " to " << diag_do (a, t); }, - a, t, n)); + ra, t, n)); - t.recipe (a, ru.apply (a, t, m)); + t.recipe (ra, ru.apply (ra, t, m)); } else { - r.first = &ru; - r.second = m; + r.ra = ra; + r.ru = &ru; + r.mr = m; } return r; @@ -208,7 +221,7 @@ namespace build // if (!g.recipe (a)) { - auto p (match_impl (a, g, false)); + auto mir (match_impl (a, g, false)); r = g.group_members (a); if (r.members != nullptr) @@ -217,7 +230,7 @@ namespace build // That didn't help, so apply the rule and go to the building // phase. // - g.recipe (a, p.first->apply (a, g, p.second)); + g.recipe (mir.ra, mir.ru->apply (mir.ra, g, mir.mr)); } // Note that we use execute_direct() rather than execute() here to diff --git a/build/algorithm.ixx b/build/algorithm.ixx index a1e2129..4581d5b 100644 --- a/build/algorithm.ixx +++ b/build/algorithm.ixx @@ -2,8 +2,6 @@ // copyright : Copyright (c) 2014-2015 Code Synthesis Ltd // license : MIT; see accompanying LICENSE file -#include // pair - #include #include #include @@ -50,7 +48,15 @@ namespace build return static_cast (search (T::static_type, dir, name, ext, scope)); } - std::pair + + struct match_result_impl + { + action ra; // Action to set recipe, not to pass to apply(). + const rule* ru; + match_result mr; + }; + + match_result_impl match_impl (action, target&, bool apply); inline void diff --git a/build/cli/target b/build/cli/target index 06bc311..7ee9a98 100644 --- a/build/cli/target +++ b/build/cli/target @@ -47,7 +47,7 @@ namespace build }; virtual group_view - group_members (action) const; + group_members (action_type) const; virtual timestamp load_mtime () const; diff --git a/build/cli/target.cxx b/build/cli/target.cxx index d8280f2..e8ca1cf 100644 --- a/build/cli/target.cxx +++ b/build/cli/target.cxx @@ -30,7 +30,7 @@ namespace build // cli.cxx // group_view cli_cxx:: - group_members (action) const + group_members (action_type) const { return h != nullptr ? group_view {m, (i != nullptr ? 3U : 2U)} diff --git a/build/operation b/build/operation index 75d6526..6fde9fd 100644 --- a/build/operation +++ b/build/operation @@ -79,6 +79,19 @@ namespace build action_id outer_id; }; + // This is an "overrides" comparison, i.e., it returns true + // if the recipe for x overrides recipe for y. The idea is + // that for the same inner operation, action with an outer + // operation is "weaker" than the one without. + // + inline bool + operator> (action x, action y) + { + return x.inner_id != y.inner_id || + (x.outer_id != y.outer_id && y.outer_id != 0); + } + + /* inline bool operator== (action x, action y) { @@ -87,6 +100,7 @@ namespace build inline bool operator!= (action x, action y) {return !(x == y);} + */ std::ostream& operator<< (std::ostream&, action); diff --git a/build/target b/build/target index e07f6e0..f5a26b4 100644 --- a/build/target +++ b/build/target @@ -303,16 +303,21 @@ namespace build std::size_t dependents; public: + typedef build::action action_type; + + action_type action; // Action this recipe is for. + + public: typedef build::recipe recipe_type; const recipe_type& - recipe (action a) const {return action_ == a ? recipe_ : empty_recipe;} + recipe (action_type a) const {return a > action ? empty_recipe : recipe_;} void - recipe (action a, recipe_type r) + recipe (action_type a, recipe_type r) { - assert (action_ != a || !recipe_); - action_ = a; + assert (a > action || !recipe_); + action = a; recipe_ = std::move (r); // Also reset the target state. If this is a noop recipe, then @@ -348,7 +353,6 @@ namespace build static const target_type static_type; private: - action action_; // Action this recipe is for. recipe_type recipe_; }; diff --git a/build/target.cxx b/build/target.cxx index f9662fa..5aa1214 100644 --- a/build/target.cxx +++ b/build/target.cxx @@ -51,7 +51,7 @@ namespace build // group_view target:: - group_members (action) const + group_members (action_type) const { assert (false); // Not a group or doesn't expose its members. return group_view {nullptr, 0}; -- cgit v1.1