Cleanup match_result mess

19 files changed, 142 insertions, 141 deletions

diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index 2390b04..2a5b2d2 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -55,7 +55,7 @@ namespace build2
   pair<const rule*, match_result>
   match_impl (action a, target& t, bool apply, const rule* skip)
   {
-    pair<const rule*, match_result> r;
+    pair<const rule*, match_result> r (nullptr, false);
 
     // By default, clear the resolved targets list before calling
     // match(). The rule is free to modify this list in match()
@@ -143,7 +143,7 @@ namespace build2
               if (&ru == skip)
                 continue;
 
-              match_result m;
+              match_result m (false);
               {
                 auto g (
                   make_exception_guard (
@@ -212,7 +212,7 @@ namespace build2
                   // @@ We could also allow the rule to change the recipe
                   // action in apply(). Could be useful with delegates.
                   //
-                  t.recipe (ra, ru.apply (ra, t, m));
+                  t.recipe (ra, ru.apply (ra, t));
                 }
                 else
                 {
@@ -259,7 +259,7 @@ namespace build2
       // phase.
       //
       const match_result& mr (rp.second);
-      g.recipe (mr.recipe_action, rp.first->apply (mr.recipe_action, g, mr));
+      g.recipe (mr.recipe_action, rp.first->apply (mr.recipe_action, g));
     }
 
     // Note that we use execute_direct() rather than execute() here to
diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx
index ee4e338..b2c4941 100644
--- a/build2/algorithm.ixx
+++ b/build2/algorithm.ixx
@@ -86,8 +86,7 @@ namespace build2
   {
     auto rp (match_impl (a, t, false, &r));
     const match_result& mr (rp.second);
-    return make_pair (rp.first->apply (mr.recipe_action, t, mr),
-                      mr.recipe_action);
+    return make_pair (rp.first->apply (mr.recipe_action, t), mr.recipe_action);
   }
 
   group_view
diff --git a/build2/bin/rule b/build2/bin/rule
index 9bf1379..4072e4e 100644
--- a/build2/bin/rule
+++ b/build2/bin/rule
@@ -18,20 +18,20 @@ namespace build2
     {
     public:
       virtual match_result
-      match (action, target&, const string& hint) const;
+      match (action, target&, const string& hint) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
     };
 
     class lib_rule: public rule
     {
     public:
       virtual match_result
-      match (action, target&, const string& hint) const;
+      match (action, target&, const string& hint) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
 
       static target_state
       perform (action, target&);
diff --git a/build2/bin/rule.cxx b/build2/bin/rule.cxx
index 7710a53..3f16d71 100644
--- a/build2/bin/rule.cxx
+++ b/build2/bin/rule.cxx
@@ -25,17 +25,26 @@ namespace build2
       fail << diag_doing (a, t) << " target group" <<
         info << "explicitly select obje{}, obja{}, or objs{} member";
 
-      return nullptr;
+      return false;
     }
 
     recipe obj_rule::
-    apply (action, target&, const match_result&) const {return empty_recipe;}
+    apply (action, target&) const {return empty_recipe;}
 
     // lib
     //
     // The whole logic is pretty much as if we had our two group members as
     // our prerequisites.
     //
+
+    struct match_data
+    {
+      const string& type;
+    };
+
+    static_assert (sizeof (match_data) <= target::data_size,
+                   "insufficient space");
+
     match_result lib_rule::
     match (action act, target& xt, const string&) const
     {
@@ -79,7 +88,9 @@ namespace build2
         match_only (act, *t.s);
       }
 
-      match_result mr (t, &type);
+      t.data (match_data {type}); // Save in the target's auxilary storage.
+
+      match_result mr (true);
 
       // If there is an outer operation, indicate that we match
       // unconditionally so that we don't override ourselves.
@@ -91,11 +102,12 @@ namespace build2
     }
 
     recipe lib_rule::
-    apply (action act, target& xt, const match_result& mr) const
+    apply (action act, target& xt) const
     {
       lib& t (static_cast<lib&> (xt));
 
-      const string& type (*static_cast<const string*> (mr.cpvalue));
+      const match_data& md (t.data<match_data> ());
+      const string& type (md.type);
 
       bool a (type == "static" || type == "both");
       bool s (type == "shared" || type == "both");
@@ -116,12 +128,8 @@ namespace build2
     {
       lib& t (static_cast<lib&> (xt));
 
-      //@@ Not cool we have to do this again. Looks like we need
-      //   some kind of a cache vs resolved pointer, like in
-      //   prerequisite vs prerequisite_target.
-      //
-      //
-      const string& type (cast<string> (t["bin.lib"]));
+      const match_data& md (t.data<match_data> ());
+      const string& type (md.type);
 
       bool a (type == "static" || type == "both");
       bool s (type == "shared" || type == "both");
diff --git a/build2/cc/compile b/build2/cc/compile
index 2be2e86..584da7c 100644
--- a/build2/cc/compile
+++ b/build2/cc/compile
@@ -27,10 +27,10 @@ namespace build2
       compile (data&&);
 
       virtual match_result
-      match (action, target&, const string& hint) const;
+      match (action, target&, const string& hint) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
 
       target_state
       perform_update (action, target&) const;
diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx
index 6587d8c..e180798 100644
--- a/build2/cc/compile.cxx
+++ b/build2/cc/compile.cxx
@@ -35,6 +35,14 @@ namespace build2
     {
     }
 
+    struct match_data
+    {
+      prerequisite_member src;
+    };
+
+    static_assert (sizeof (match_data) <= target::data_size,
+                   "insufficient space");
+
     match_result compile::
     match (action a, target& t, const string&) const
     {
@@ -53,11 +61,14 @@ namespace build2
       for (prerequisite_member p: reverse_group_prerequisite_members (a, t))
       {
         if (p.is_a (x_src))
-          return p;
+        {
+          t.data (match_data {p}); // Save in the target's auxilary storage.
+          return true;
+        }
       }
 
       l4 ([&]{trace << "no " << x_lang << " source file for target " << t;});
-      return nullptr;
+      return false;
     }
 
     // Append or hash library options from a pair of *.export.* variables
@@ -180,11 +191,12 @@ namespace build2
     }
 
     recipe compile::
-    apply (action a, target& xt, const match_result& mr) const
+    apply (action a, target& xt) const
     {
       tracer trace (x, "compile::apply");
 
       file& t (static_cast<file&> (xt));
+      const match_data& md (t.data<match_data> ());
 
       scope& bs (t.base_scope ());
       scope& rs (*bs.root_scope ());
@@ -297,7 +309,7 @@ namespace build2
         // t.prerequisite_targets since we used standard search() and match()
         // above.
         //
-        file& src (mr.as_target<file> ());
+        file& src (*md.src.search ().is_a<file> ());
 
         // Make sure the output directory exists.
         //
diff --git a/build2/cc/link b/build2/cc/link
index cd8c10e..ea30320 100644
--- a/build2/cc/link
+++ b/build2/cc/link
@@ -25,10 +25,10 @@ namespace build2
       link (data&&);
 
       virtual match_result
-      match (action, target&, const string& hint) const;
+      match (action, target&, const string& hint) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
 
       target_state
       perform_update (action, target&) const;
diff --git a/build2/cc/link.cxx b/build2/cc/link.cxx
index 5ab9d91..7722c4f 100644
--- a/build2/cc/link.cxx
+++ b/build2/cc/link.cxx
@@ -109,11 +109,11 @@ namespace build2
         // it will most definitely need to be compiled but we can't do that.
         //
         else if (p.is_a<cc> ())
-          return nullptr;
+          return false;
       }
 
       if (!(seen_x || seen_c || seen_obj || seen_lib))
-        return nullptr;
+        return false;
 
       // We will only chain a C source if there is also an X source or we were
       // explicitly told to.
@@ -121,7 +121,7 @@ namespace build2
       if (seen_c && !seen_x && hint < x)
       {
         l4 ([&]{trace << "C prerequisite without " << x_lang << " or hint";});
-        return nullptr;
+        return false;
       }
 
       // Set the library type.
@@ -173,7 +173,7 @@ namespace build2
         }
       }
 
-      return &t;
+      return true;
     }
 
     auto link::
@@ -316,7 +316,7 @@ namespace build2
     }
 
     recipe link::
-    apply (action a, target& xt, const match_result&) const
+    apply (action a, target& xt) const
     {
       tracer trace (x, "link::apply");
 
diff --git a/build2/cli/rule b/build2/cli/rule
index 4f8ebc2..41ff8bf 100644
--- a/build2/cli/rule
+++ b/build2/cli/rule
@@ -18,10 +18,10 @@ namespace build2
     {
     public:
       virtual match_result
-      match (action, target&, const string& hint) const;
+      match (action, target&, const string& hint) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
 
       static target_state
       perform_update (action, target&);
diff --git a/build2/cli/rule.cxx b/build2/cli/rule.cxx
index cec8a60..bcc3a9f 100644
--- a/build2/cli/rule.cxx
+++ b/build2/cli/rule.cxx
@@ -56,7 +56,7 @@ namespace build2
 
         // See if we have a .cli source file.
         //
-        match_result r;
+        bool r (false);
         for (prerequisite_member p: group_prerequisite_members (a, t))
         {
           if (p.is_a<cli> ())
@@ -67,10 +67,10 @@ namespace build2
             {
               l4 ([&]{trace << ".cli file stem '" << p.name () << "' "
                             << "doesn't match target " << t;});
-              return r;
+              return false;
             }
 
-            r = p;
+            r = true;
             break;
           }
         }
@@ -113,7 +113,7 @@ namespace build2
         // it is some other group, then we are definitely not a match.
         //
         if (t.group != nullptr)
-          return t.group->is_a<cli_cxx> ();
+          return t.group->is_a<cli_cxx> () != nullptr;
 
         // Check if there is a corresponding cli.cxx{} group.
         //
@@ -164,12 +164,12 @@ namespace build2
         }
 
         assert (t.group == g);
-        return g;
+        return g != nullptr;
       }
     }
 
     recipe compile::
-    apply (action a, target& xt, const match_result& mr) const
+    apply (action a, target& xt) const
     {
       if (cli_cxx* pt = xt.is_a<cli_cxx> ())
       {
@@ -199,7 +199,7 @@ namespace build2
       }
       else
       {
-        cli_cxx& g (*static_cast<cli_cxx*> (mr.target));
+        cli_cxx& g (*static_cast<cli_cxx*> (xt.group));
         build2::match (a, g);
         return group_recipe; // Execute the group's recipe.
       }
diff --git a/build2/dist/rule b/build2/dist/rule
index f428ff2..0c40cf0 100644
--- a/build2/dist/rule
+++ b/build2/dist/rule
@@ -20,10 +20,10 @@ namespace build2
     {
     public:
       virtual match_result
-      match (action, target&, const string&) const;
+      match (action, target&, const string&) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
     };
   }
 }
diff --git a/build2/dist/rule.cxx b/build2/dist/rule.cxx
index e2bad9a..f6b50f7 100644
--- a/build2/dist/rule.cxx
+++ b/build2/dist/rule.cxx
@@ -16,13 +16,13 @@ namespace build2
   namespace dist
   {
     match_result rule::
-    match (action, target& t, const string&) const
+    match (action, target&, const string&) const
     {
-      return t; // We always match.
+      return true; // We always match.
     }
 
     recipe rule::
-    apply (action a, target& t, const match_result&) const
+    apply (action a, target& t) const
     {
       const dir_path& out_root (t.root_scope ().out_path ());
 
diff --git a/build2/install/rule b/build2/install/rule
index 4ad91bb..94ce65b 100644
--- a/build2/install/rule
+++ b/build2/install/rule
@@ -20,10 +20,10 @@ namespace build2
     {
     public:
       virtual match_result
-      match (action, target&, const string&) const;
+      match (action, target&, const string&) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
     };
 
     struct install_dir;
@@ -32,7 +32,10 @@ namespace build2
     {
     public:
       virtual match_result
-      match (action, target&, const string&) const;
+      match (action, target&, const string&) const override;
+
+      virtual recipe
+      apply (action, target&) const override;
 
       // Return NULL if this prerequisite should be ignored and pointer to its
       // target otherwise. The default implementation ignores prerequsites that
@@ -41,9 +44,6 @@ namespace build2
       virtual target*
       filter (action, target&, prerequisite_member) const;
 
-      virtual recipe
-      apply (action, target&, const match_result&) const;
-
       // Extra installation hooks.
       //
       using install_dir = install::install_dir;
diff --git a/build2/install/rule.cxx b/build2/install/rule.cxx
index 68000af..c97dbb0 100644
--- a/build2/install/rule.cxx
+++ b/build2/install/rule.cxx
@@ -39,13 +39,13 @@ namespace build2
     // alias_rule
     //
     match_result alias_rule::
-    match (action, target& t, const string&) const
+    match (action, target&, const string&) const
     {
-      return t;
+      return true;
     }
 
     recipe alias_rule::
-    apply (action a, target& t, const match_result&) const
+    apply (action a, target& t) const
     {
       tracer trace ("install::alias_rule::apply");
 
@@ -82,26 +82,39 @@ namespace build2
 
     // file_rule
     //
+    struct match_data
+    {
+      bool install;
+    };
+
+    static_assert (sizeof (match_data) <= target::data_size,
+                   "insufficient space");
+
     match_result file_rule::
     match (action a, target& t, const string&) const
     {
       // First determine if this target should be installed (called
       // "installable" for short).
       //
-      if (lookup_install<path> (t, "install") == nullptr)
-        // If this is the update pre-operation, signal that we don't match so
-        // that some other rule can take care of it.
-        //
-        return a.operation () == update_id ? nullptr : match_result (t, false);
+      match_data md {lookup_install<path> (t, "install") != nullptr};
+      match_result mr (true);
 
-      match_result mr (t, true);
-
-      // If this is the update pre-operation, change the recipe action
-      // to (update, 0) (i.e., "unconditional update").
-      //
       if (a.operation () == update_id)
-        mr.recipe_action = action (a.meta_operation (), update_id);
+      {
+        // If this is the update pre-operation and the target is installable,
+        // change the recipe action to (update, 0) (i.e., "unconditional
+        // update") so that we don't get matched for its prerequisites.
+        //
+        if (md.install)
+          mr.recipe_action = action (a.meta_operation (), update_id);
+        else
+          // Otherwise, signal that we don't match so that some other rule can
+          // take care of it.
+          //
+          return false;
+      }
 
+      t.data (md); // Save the data in the target's auxilary storage.
       return mr;
     }
 
@@ -113,9 +126,12 @@ namespace build2
     }
 
     recipe file_rule::
-    apply (action a, target& t, const match_result& mr) const
+    apply (action a, target& t) const
     {
-      if (!mr.bvalue) // Not installable.
+      match_data md (move (t.data<match_data> ()));
+      t.clear_data (); // In case delegated-to rule also uses aux storage.
+
+      if (!md.install) // Not installable.
         return noop_recipe;
 
       // Ok, if we are here, then this means:
diff --git a/build2/rule b/build2/rule
index 6d4a580..f104c4f 100644
--- a/build2/rule
+++ b/build2/rule
@@ -13,60 +13,20 @@
 
 namespace build2
 {
-  // @@ This is an ugly mess that is overdue for redesign. Perhaps
-  //    something similar to variable value storage?
-  //
   class match_result
   {
   public:
-    typedef build2::target target_type;
-    typedef build2::prerequisite prerequisite_type;
-
-    // Can contain neither (both are NULL), one of, or both. If both
-    // are NULL, then it is a "no match" indicator.
-    //
-    // Note that if the "payload" is stored in *value instead of
-    // prerequisite, then target must not be NULL.
-    //
-    union
-    {
-      prerequisite_type* prerequisite;
-
-      bool bvalue;
-      void* pvalue;
-      const void* cpvalue;
-    };
-
-    target_type* target;
-
+    bool result;
     action recipe_action = action (); // Used as recipe's action if set.
 
-    match_result (nullptr_t v = nullptr): prerequisite (v), target (v) {}
-    match_result (prerequisite_type& p): prerequisite (&p), target (nullptr) {}
-    match_result (prerequisite_type* p): prerequisite (p), target (nullptr) {}
-    match_result (target_type& t): prerequisite (nullptr), target (&t) {}
-    match_result (target_type* t): prerequisite (nullptr), target (t) {}
-    match_result (const prerequisite_member& pm)
-        : prerequisite (&pm.prerequisite.get ()), target (pm.target) {}
-
-    match_result (target_type& t, bool v): bvalue (v), target (&t) {}
-    match_result (target_type& t, void* v): pvalue (v), target (&t) {}
-    match_result (target_type& t, const void* v): cpvalue (v), target (&t) {}
-    match_result (target_type& t, nullptr_t v): pvalue (v), target (&t) {}
-
     explicit
-    operator bool () const
-    {
-      return target != nullptr || prerequisite != nullptr;
-    }
-
-    template <typename T>
-    T&
-    as_target () const
-    {
-      return static_cast<T&> (
-        target != nullptr ? *target : *prerequisite->target);
-    }
+    operator bool () const {return result;}
+
+    // Note that the from-bool constructor is intentionally implicit so that
+    // we can return true/false from match().
+    //
+    match_result (bool r): result (r) {}
+    match_result (bool r, action a): result (r), recipe_action (a) {}
   };
 
   class rule
@@ -76,7 +36,7 @@ namespace build2
     match (action, target&, const string& hint) const = 0;
 
     virtual recipe
-    apply (action, target&, const match_result&) const = 0;
+    apply (action, target&) const = 0;
   };
 
   // Fallback rule that on update verifies that the file exists and is
@@ -86,10 +46,10 @@ namespace build2
   {
   public:
     virtual match_result
-    match (action, target&, const string& hint) const;
+    match (action, target&, const string& hint) const override;
 
     virtual recipe
-    apply (action, target&, const match_result&) const;
+    apply (action, target&) const override;
 
     static target_state
     perform_update (action, target&);
@@ -101,10 +61,10 @@ namespace build2
   {
   public:
     virtual match_result
-    match (action, target&, const string& hint) const;
+    match (action, target&, const string& hint) const override;
 
     virtual recipe
-    apply (action, target&, const match_result&) const;
+    apply (action, target&) const override;
 
     static alias_rule instance;
   };
@@ -113,10 +73,10 @@ namespace build2
   {
   public:
     virtual match_result
-    match (action, target&, const string& hint) const;
+    match (action, target&, const string& hint) const override;
 
     virtual recipe
-    apply (action, target&, const match_result&) const;
+    apply (action, target&) const override;
 
     static target_state
     perform_update (action, target&);
@@ -133,10 +93,10 @@ namespace build2
   {
   public:
     virtual match_result
-    match (action, target& t, const string&) const {return t;}
+    match (action, target&, const string&) const override {return true;}
 
     virtual recipe
-    apply (action, target&, const match_result&) const {return noop_recipe;}
+    apply (action, target&) const override {return noop_recipe;}
 
     static fallback_rule instance;
   };
diff --git a/build2/rule.cxx b/build2/rule.cxx
index 58340b5..f9805db 100644
--- a/build2/rule.cxx
+++ b/build2/rule.cxx
@@ -63,18 +63,18 @@ namespace build2
         }
 
         if (ts != timestamp_unknown && ts != timestamp_nonexistent)
-          return t;
+          return true;
 
         l4 ([&]{trace << "no existing file for target " << t;});
-        return nullptr;
+        return false;
       }
     default:
-      return t;
+      return true;
     }
   }
 
   recipe file_rule::
-  apply (action a, target& t, const match_result&) const
+  apply (action a, target& t) const
   {
     // Update triggers the update of this target's prerequisites
     // so it would seem natural that we should also trigger their
@@ -143,13 +143,13 @@ namespace build2
   // alias_rule
   //
   match_result alias_rule::
-  match (action, target& t, const string&) const
+  match (action, target&, const string&) const
   {
-    return t;
+    return true;
   }
 
   recipe alias_rule::
-  apply (action a, target& t, const match_result&) const
+  apply (action a, target& t) const
   {
     // Inject dependency on our directory (note: not parent) so that it is
     // automatically created on update and removed on clean.
@@ -165,13 +165,13 @@ namespace build2
   // fsdir_rule
   //
   match_result fsdir_rule::
-  match (action, target& t, const string&) const
+  match (action, target&, const string&) const
   {
-    return t;
+    return true;
   }
 
   recipe fsdir_rule::
-  apply (action a, target& t, const match_result&) const
+  apply (action a, target& t) const
   {
     // Inject dependency on the parent directory. Note that we don't do it for
     // clean since we shouldn't (and can't possibly, since it's our parent) be
diff --git a/build2/target b/build2/target
index 8c2b2da..f824689 100644
--- a/build2/target
+++ b/build2/target
@@ -766,6 +766,12 @@ namespace build2
     as_prerequisite (tracer&) const;
   };
 
+  // It is often stored as the target's auxiliary data so make sure there is
+  // no destructor overhead.
+  //
+  static_assert (std::is_trivially_destructible<prerequisite_member>::value,
+                 "prerequisite_member is not trivially destructible");
+
   inline ostream&
   operator<< (ostream& os, const prerequisite_member& pm)
   {
diff --git a/build2/test/rule b/build2/test/rule
index e6cdc37..b915ea7 100644
--- a/build2/test/rule
+++ b/build2/test/rule
@@ -19,10 +19,10 @@ namespace build2
     {
     public:
       virtual match_result
-      match (action, target&, const string&) const;
+      match (action, target&, const string&) const override;
 
       virtual recipe
-      apply (action, target&, const match_result&) const;
+      apply (action, target&) const override;
 
       static target_state
       perform_script (action, target&);
diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx
index 975d19f..8621422 100644
--- a/build2/test/rule.cxx
+++ b/build2/test/rule.cxx
@@ -106,7 +106,7 @@ namespace build2
         }
       }
 
-      match_result mr (t);
+      match_result mr (true);
 
       // Theoretically if this target is testable and this is the update
       // pre-operation, then all we need to do is say we are not a match and
@@ -146,7 +146,7 @@ namespace build2
     }
 
     recipe rule::
-    apply (action a, target& t, const match_result&) const
+    apply (action a, target& t) const
     {
       tracer trace ("test::rule::apply");