From fbb1fc469da566dd21be47a7998dfad11d3085b7 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 23 Nov 2022 13:42:52 +0200 Subject: Take into account ad hoc recipes in rule::sub_match() (fixed GH issue #227) --- libbuild2/algorithm.cxx | 130 +++++++++++++++++++++++++----------------------- libbuild2/rule.cxx | 22 ++++++++ libbuild2/rule.hxx | 12 +++-- 3 files changed, 98 insertions(+), 66 deletions(-) diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 889b4e9..0326839 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -434,6 +434,73 @@ namespace build2 t[a].rule = rm; } + // Note: not static since also called by rule::sub_match(). + // + const rule_match* + match_adhoc_recipe (action a, target& t, match_extra& me) + { + auto df = make_diag_frame ( + [a, &t](const diag_record& dr) + { + if (verb != 0) + dr << info << "while matching ad hoc recipe to " << diag_do (a, t); + }); + + auto match = [a, &t, &me] (const adhoc_rule& r, bool fallback) -> bool + { + me.init (fallback); + + if (auto* f = (a.outer () + ? t.ctx.current_outer_oif + : t.ctx.current_inner_oif)->adhoc_match) + return f (r, a, t, string () /* hint */, me); + else + return r.match (a, t, string () /* hint */, me); + }; + + // The action could be Y-for-X while the ad hoc recipes are always for + // X. So strip the Y-for part for comparison (but not for the match() + // calls; see below for the hairy inner/outer semantics details). + // + action ca (a.inner () + ? a + : action (a.meta_operation (), a.outer_operation ())); + + auto b (t.adhoc_recipes.begin ()), e (t.adhoc_recipes.end ()); + auto i (find_if ( + b, e, + [&match, ca] (const shared_ptr& r) + { + auto& as (r->actions); + return (find (as.begin (), as.end (), ca) != as.end () && + match (*r, false)); + })); + + if (i == e) + { + // See if we have a fallback implementation. + // + // See the adhoc_rule::reverse_fallback() documentation for details on + // what's going on here. + // + i = find_if ( + b, e, + [&match, ca, &t] (const shared_ptr& r) + { + auto& as (r->actions); + + // Note that the rule could be there but not match (see above), + // thus this extra check. + // + return (find (as.begin (), as.end (), ca) == as.end () && + r->reverse_fallback (ca, t.type ()) && + match (*r, true)); + }); + } + + return i != e ? &(*i)->rule_match : nullptr; + } + // Return the matching rule or NULL if no match and try_match is true. // const rule_match* @@ -455,67 +522,8 @@ namespace build2 // if (!t.adhoc_recipes.empty ()) { - auto df = make_diag_frame ( - [a, &t](const diag_record& dr) - { - if (verb != 0) - dr << info << "while matching ad hoc recipe to " << diag_do (a, t); - }); - - auto match = [a, &t, &me] (const adhoc_rule& r, bool fallback) -> bool - { - me.init (fallback); - - if (auto* f = (a.outer () - ? t.ctx.current_outer_oif - : t.ctx.current_inner_oif)->adhoc_match) - return f (r, a, t, string () /* hint */, me); - else - return r.match (a, t, string () /* hint */, me); - }; - - // The action could be Y-for-X while the ad hoc recipes are always for - // X. So strip the Y-for part for comparison (but not for the match() - // calls; see below for the hairy inner/outer semantics details). - // - action ca (a.inner () - ? a - : action (a.meta_operation (), a.outer_operation ())); - - auto b (t.adhoc_recipes.begin ()), e (t.adhoc_recipes.end ()); - auto i (find_if ( - b, e, - [&match, ca] (const shared_ptr& r) - { - auto& as (r->actions); - return (find (as.begin (), as.end (), ca) != as.end () && - match (*r, false)); - })); - - if (i == e) - { - // See if we have a fallback implementation. - // - // See the adhoc_rule::reverse_fallback() documentation for details on - // what's going on here. - // - i = find_if ( - b, e, - [&match, ca, &t] (const shared_ptr& r) - { - auto& as (r->actions); - - // Note that the rule could be there but not match (see above), - // thus this extra check. - // - return (find (as.begin (), as.end (), ca) == as.end () && - r->reverse_fallback (ca, t.type ()) && - match (*r, true)); - }); - } - - if (i != e) - return &(*i)->rule_match; + if (const rule_match* r = match_adhoc_recipe (a, t, me)) + return r; } // If this is an outer operation (Y-for-X), then we look for rules diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx index e5d5248..c5366a6 100644 --- a/libbuild2/rule.cxx +++ b/libbuild2/rule.cxx @@ -30,10 +30,25 @@ namespace build2 return nullptr; } + const rule_match* + match_adhoc_recipe (action, target&, match_extra&); // algorithm.cxx + bool rule:: sub_match (const string& n, operation_id o, action a, target& t, match_extra& me) const { + // First check for an ad hoc recipe (see match_rule() for details). + // + if (!t.adhoc_recipes.empty ()) + { + // Use scratch match_extra since if there is no recipe, then we don't + // want to keep any changes and if there is, then we want it discarded. + // + match_extra s; + if (match_adhoc_recipe (action (a.meta_operation (), o), t, s) != nullptr) + return false; + } + const string& h (t.find_hint (o)); return name_rule_map::sub (h, n) && match (a, t, h, me); } @@ -56,6 +71,13 @@ namespace build2 sub_match (const string& n, operation_id o, action a, target& t) const { + if (!t.adhoc_recipes.empty ()) + { + match_extra s; + if (match_adhoc_recipe (action (a.meta_operation (), o), t, s) != nullptr) + return false; + } + return name_rule_map::sub (t.find_hint (o), n) && match (a, t); } diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx index da069ef..913c597 100644 --- a/libbuild2/rule.hxx +++ b/libbuild2/rule.hxx @@ -22,7 +22,8 @@ namespace build2 // you need to modify some state (e.g., counters or some such), then make // sure things are MT-safe. // - // Note: match() is only called once but may not be followed by apply(). + // Note: match() could be called multiple times (so should be idempotent) + // and it may not be followed by apply(). // // The hint argument is the rule hint, if any, that was used to select this // rule. While normally not factored into the match decision, a rule may @@ -72,10 +73,11 @@ namespace build2 // only if our update rule also matches. // // Arranging this, however, is not a simple matter of calling the other - // rule's match(): we also have to take into account the rule hints for - // that operation. This helper performs all the necessary steps. Note: - // should only be called from match() (see target::find_hint() for - // details). + // rule's match(): we also have to take into account ad hoc recipes and + // rule hints for that operation. This helper performs all the necessary + // checks. Note: should only be called from match() (see + // target::find_hint() for details). Note also that ad hoc recipes are + // checked for hint_op, not action's operation. // bool sub_match (const string& rule_name, operation_id hint_op, -- cgit v1.1