Redo fallback reverse operation machinery in ad hoc recipes

7 files changed, 78 insertions, 64 deletions

diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index fe74fc6..c94b50f 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -104,43 +104,33 @@ namespace build2
     os << ind << string (braces, '}');
   }
 
-  bool adhoc_buildscript_rule::
-  match (action a, target& t, const string&, match_extra&,
-         optional<action> fb) const
+  optional<action> adhoc_buildscript_rule::
+  reverse_fallback (action a, const target_type& tt) const
   {
-    if (!fb)
-      ;
-    // If this is clean for a file target and we are supplying the update,
-    // then we will also supply the standard clean.
-    //
-    else if (a   == perform_clean_id  &&
-             *fb == perform_update_id &&
-             t.is_a<file> ())
-      ;
-    else
-      return false;
-
-    // It's unfortunate we have to resort to this but we need to remember this
-    // in apply().
+    // We can provide clean for a file target if we are providing update.
     //
-    t.data (fb.has_value ());
+    if (a == perform_update_id && tt.is_a<file> ())
+      return perform_clean_id;
 
-    return true;
+    return nullopt;
   }
 
   recipe adhoc_buildscript_rule::
-  apply (action a, target& t, match_extra& e) const
+  apply (action a, target& t, match_extra& me) const
   {
-    return apply (a, t, e, nullopt);
+    return apply (a, t, me, nullopt);
   }
 
   recipe adhoc_buildscript_rule::
-  apply (action a, target& t, match_extra&, const optional<timestamp>& d) const
+  apply (action a,
+         target& t,
+         match_extra& me,
+         const optional<timestamp>& d) const
   {
-    // We don't support deadlines of any of these case (see below).
+    // We don't support deadlines for any of these cases (see below).
     //
-    if (d && (a.outer ()      ||
-              t.data<bool> () ||
+    if (d && (a.outer ()  ||
+              me.fallback ||
               (a == perform_update_id && t.is_a<file> ())))
       return empty_recipe;
 
@@ -177,7 +167,7 @@ namespace build2
 
     // See if we are providing the standard clean as a fallback.
     //
-    if (t.data<bool> ())
+    if (me.fallback)
       return &perform_clean_depdb;
 
     if (a == perform_update_id && t.is_a<file> ())
diff --git a/libbuild2/adhoc-rule-buildscript.hxx b/libbuild2/adhoc-rule-buildscript.hxx
index 38cce1e..bf14472 100644
--- a/libbuild2/adhoc-rule-buildscript.hxx
+++ b/libbuild2/adhoc-rule-buildscript.hxx
@@ -22,9 +22,8 @@ namespace build2
                                 public adhoc_rule_with_deadline
   {
   public:
-    virtual bool
-    match (action, target&, const string&, match_extra&,
-           optional<action>) const override;
+    virtual optional<action>
+    reverse_fallback (action, const target_type&) const override;
 
     virtual recipe
     apply (action, target&, match_extra&) const override;
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index a19a6a0..acff325 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -345,16 +345,22 @@ namespace build2
             dr << info << "while matching ad hoc recipe to " << diag_do (a, t);
         });
 
-      auto match = [a, &t] (const adhoc_rule& r, optional<action> ra)
+      auto match = [a, &t] (const adhoc_rule& r, bool fallback) -> bool
       {
-        match_extra me;
+        match_extra me {fallback};
 
+        bool m;
         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,  ra);
+          m = f (r, a, t, string () /* hint */, me);
         else
-          return r.match (a, t, string () /* hint */, me, ra);
+          m = r.match (a, t, string () /* hint */, me);
+
+        if (m)
+          t[a].match_extra = move (me);
+
+        return m;
       };
 
       // The action could be Y-for-X while the ad hoc recipes are always for
@@ -372,13 +378,16 @@ namespace build2
                 {
                   auto& as (r.actions);
                   return (find (as.begin (), as.end (), ca) != as.end () &&
-                          match (*r.rule, nullopt));
+                          match (*r.rule, false));
                 }));
 
       if (i == e)
+      {
+        // See if we have a fallback implementation.
+        //
         i = find_if (
           b, e,
-          [&match, ca] (const adhoc_recipe& r)
+          [&match, ca, &t] (const adhoc_recipe& r)
           {
             // See the adhoc_rule::match() documentation for details on what's
             // going on here.
@@ -386,12 +395,17 @@ namespace build2
             auto& as (r.actions);
             if (find (as.begin (), as.end (), ca) == as.end ())
             {
-              for (auto ra: as)
-                if (match (*r.rule, ra))
+              for (auto sa: as)
+              {
+                optional<action> ra (r.rule->reverse_fallback (sa, t.type ()));
+
+                if (ra && *ra == ca && match (*r.rule, true))
                   return true;
+              }
             }
             return false;
           });
+      }
 
       if (i != e)
         return &i->rule->rule_match;
@@ -470,6 +484,7 @@ namespace build2
             if (&ru == skip)
               continue;
 
+            match_extra me {false};
             {
               auto df = make_diag_frame (
                 [a, &t, &n](const diag_record& dr)
@@ -479,7 +494,6 @@ namespace build2
                        << diag_do (a, t);
                 });
 
-              match_extra me;
               if (!ru.match (a, t, hint, me))
                 continue;
             }
@@ -509,7 +523,7 @@ namespace build2
                 //
                 // @@ Can't we temporarily swap things out in target?
                 //
-                match_extra me1;
+                match_extra me1 {false};
                 if (!ru1.match (a, t, hint, me1))
                   continue;
               }
@@ -525,7 +539,10 @@ namespace build2
             }
 
             if (!ambig)
+            {
+              t[a].match_extra = move (me);
               return &r;
+            }
             else
               dr << info << "use rule hint to disambiguate this match";
           }
@@ -607,6 +624,9 @@ namespace build2
     if (const scope* rs = bs.root_scope ())
       penv = auto_project_env (*rs);
 
+    const rule& r (m.second);
+    match_extra& me (t[a].match_extra);
+
     auto df = make_diag_frame (
       [a, &t, &m](const diag_record& dr)
       {
@@ -615,9 +635,6 @@ namespace build2
              << diag_do (a, t);
       });
 
-    const rule& r (m.second);
-    match_extra me;
-
     if (auto* f = (a.outer ()
                    ? t.ctx.current_outer_oif
                    : t.ctx.current_inner_oif)->adhoc_apply)
diff --git a/libbuild2/operation.hxx b/libbuild2/operation.hxx
index cfd6a95..de3ae7c 100644
--- a/libbuild2/operation.hxx
+++ b/libbuild2/operation.hxx
@@ -233,8 +233,7 @@ namespace build2
     // proxied through these functions.
     //
     bool (*adhoc_match) (const adhoc_rule&,
-                         action, target&, const string&, match_extra&,
-                         optional<action>);
+                         action, target&, const string&, match_extra&);
 
     recipe (*adhoc_apply) (const adhoc_rule&, action, target&, match_extra&);
   };
diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx
index 8b2b021..49da7cb 100644
--- a/libbuild2/rule.cxx
+++ b/libbuild2/rule.cxx
@@ -332,11 +332,10 @@ namespace build2
   //
   const dir_path adhoc_rule::recipes_build_dir ("recipes");
 
-  bool adhoc_rule::
-  match (action a, target& t, const string& h, match_extra& me,
-         optional<action> fallback) const
+  optional<action> adhoc_rule::
+  reverse_fallback (action, const target_type&) const
   {
-    return !fallback && match (a, t, h, me);
+    return nullopt;
   }
 
   bool adhoc_rule::
diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx
index 4b05379..af89124 100644
--- a/libbuild2/rule.hxx
+++ b/libbuild2/rule.hxx
@@ -24,13 +24,12 @@ namespace build2
   //
   // Note: match() is only called once but may not be followed by apply().
   //
-  // The match_extra argument is used to pass additional information that is
-  // only needed by some rule implementations. It is also a way for us to
-  // later pass more information without breaking source compatibility.
+  // The match_extra argument (the type is defined in target.hxx) is used to
+  // pass additional information that is only needed by some rule
+  // implementations. It is also a way for us to later pass more information
+  // without breaking source compatibility.
   //
-  struct match_extra
-  {
-  };
+  struct match_extra;
 
   class LIBBUILD2_SYMEXPORT rule
   {
@@ -172,18 +171,15 @@ namespace build2
   public:
     // Some of the operations come in compensating pairs, such as update and
     // clean, install and uninstall. An ad hoc rule implementation may choose
-    // to provide a fallback implementation of a compensating operation if it
-    // is providing the other half (passed in the fallback argument).
-    //
-    // The default implementation calls rule::match() if fallback is absent
-    // and returns false if fallback is present. So an implementation that
-    // doesn't care about this semantics can implement the straight rule
-    // interface.
+    // to provide a fallback implementation of a reverse operation if it is
+    // providing the other half.
     //
-    virtual bool
-    match (action, target&, const string&, match_extra&,
-           optional<action> fallback) const;
+    virtual optional<action>
+    reverse_fallback (action, const target_type&) const;
 
+    // The default implementation forwards to the pattern's match() if there
+    // is a pattern and returns true otherwise.
+    //
     virtual bool
     match (action, target&, const string&, match_extra&) const override;
 
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index 56b10c0..45f285c 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -97,6 +97,16 @@ namespace build2
   //
   using rule_match = pair<const string, reference_wrapper<const rule>>;
 
+  // Additional information about a rule match (see rule.hxx for details).
+  //
+  // @@ TODO: will probably want to clear it after apply() if add anything
+  //    dynamically-allocated here (see apply_impl()).
+  //
+  struct match_extra
+  {
+    bool fallback; // True if matching a fallback rule.
+  };
+
   // Target.
   //
 
@@ -495,6 +505,10 @@ namespace build2
       //
       mutable atomic_count dependents {0};
 
+      // Match state storage between the match() and apply() calls.
+      //
+      build2::match_extra match_extra;
+
       // Matched rule (pointer to hint_rule_map element). Note that in case of
       // a direct recipe assignment we may not have a rule (NULL).
       //