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.

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<const rule*, match_result>
+  match_result_impl
   match_impl (action a, target& t, bool apply)
   {
-    pair<const rule*, match_result> 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 <utility> // pair
-
 #include <build/rule>
 #include <build/prerequisite>
 #include <build/context>
@@ -50,7 +48,15 @@ namespace build
     return static_cast<T&> (search (T::static_type, dir, name, ext, scope));
   }
 
-  std::pair<const rule*, match_result>
+
+  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};