Take into account ad hoc recipes in rule::sub_match() (fixed GH issue #227)

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<adhoc_rule>& 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<adhoc_rule>& 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<adhoc_rule>& 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<adhoc_rule>& 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,