Rework meta/operation registration

We now have global tables for meta/operation. Plus each can
then be enabled on the per-project basis.

16 files changed, 186 insertions, 115 deletions

diff --git a/build/algorithm.ixx b/build/algorithm.ixx
index e0b9364..ac2b1fc 100644
--- a/build/algorithm.ixx
+++ b/build/algorithm.ixx
@@ -68,11 +68,11 @@ namespace build
   inline void
   unmatch (action, target& t)
   {
+    // text << "U " << t << ": " << t.dependents << " " << dependency_count;
+
     assert (t.dependents != 0 && dependency_count != 0);
     t.dependents--;
     dependency_count--;
-
-    // text << "U " << t << ": " << t.dependents << " " << dependency_count;
   }
 
   inline void
@@ -131,6 +131,8 @@ namespace build
   inline target_state
   execute (action a, target& t)
   {
+    // text << "E " << t << ": " << t.dependents << " " << dependency_count;
+
     if (dependency_count != 0) // Re-examination of a postponed target?
     {
       assert (t.dependents != 0);
@@ -138,8 +140,6 @@ namespace build
       dependency_count--;
     }
 
-    // text << "E " << t << ": " << t.dependents << " " << dependency_count;
-
     switch (target_state ts = t.state ())
     {
     case target_state::unchanged:
diff --git a/build/b.cxx b/build/b.cxx
index 5500c9e..11280e1 100644
--- a/build/b.cxx
+++ b/build/b.cxx
@@ -492,8 +492,8 @@ main (int argc, char* argv[])
           // all be known. We store the combined action id in uint8_t;
           // see <operation> for details.
           //
-          assert (rs.operations.size () <= 128);
-          assert (rs.meta_operations.size () <= 128);
+          assert (operation_table.size () <= 128);
+          assert (meta_operation_table.size () <= 128);
 
           // Since we now know all the names of meta-operations and
           // operations, "lift" names that we assumed (from buildspec
@@ -511,7 +511,7 @@ main (int argc, char* argv[])
 
             if (!on.empty ())
             {
-              m = rs.meta_operations.find (on);
+              m = meta_operation_table.find (on);
 
               if (m != 0)
               {
@@ -543,7 +543,7 @@ main (int argc, char* argv[])
               }
               else
               {
-                o = rs.operations.find (on);
+                o = operation_table.find (on);
 
                 if (o == 0)
                 {
@@ -564,7 +564,7 @@ main (int argc, char* argv[])
 
             if (!mn.empty ())
             {
-              m = rs.meta_operations.find (mn);
+              m = meta_operation_table.find (mn);
 
               if (m == 0)
               {
@@ -591,7 +591,11 @@ main (int argc, char* argv[])
             if (mid == 0)
             {
               mid = m;
-              mif = &rs.meta_operations[mid].get ();
+              mif = rs.meta_operations[m];
+
+              if (mif == nullptr)
+                fail (l) << "target " << tn << " does not support meta-"
+                         << "operation " << meta_operation_table[m];
 
               level4 ([&]{trace << "start meta-operation batch " << mif->name
                                 << ", id " << static_cast<uint16_t> (mid);});
@@ -607,10 +611,15 @@ main (int argc, char* argv[])
             //
             else
             {
-              if (mid > rs.meta_operations.size () ||     // Not a valid index.
-                  mif != &rs.meta_operations[mid].get ()) // Not the same impl.
-                fail (l) << "different meta-operation implementations "
-                         << "in a meta-operation batch";
+              const meta_operation_info* mi (rs.meta_operations[mid]);
+
+              if (mi == nullptr)
+                fail (l) << "target " << tn << " does not support meta-"
+                         << "operation " << meta_operation_table[mid];
+
+              if (mi != mif)
+                fail (l) << "different implementations of meta-operation "
+                         << mif->name << " in the same meta-operation batch";
             }
 
             // If this is the first target in the operation batch, then set
@@ -618,10 +627,21 @@ main (int argc, char* argv[])
             //
             if (oid == 0)
             {
+              auto lookup =
+                [&rs, &l, &tn] (operation_id o) -> const operation_info*
+                {
+                  const operation_info* r (rs.operations[o]);
+
+                  if (r == nullptr)
+                    fail (l) << "target " << tn << " does not support "
+                             << "operation " << operation_table[o];
+                  return r;
+                };
+
               if (o == 0)
                 o = default_id;
 
-              oif = &rs.operations[o].get ();
+              oif = lookup (o);
 
               level4 ([&]{trace << "start operation batch " << oif->name
                                 << ", id " << static_cast<uint16_t> (o);});
@@ -635,7 +655,7 @@ main (int argc, char* argv[])
 
               if (o != oid)
               {
-                oif = &rs.operations[oid].get ();
+                oif = lookup (oid);
                 level4 ([&]{trace << "operation translated to " << oif->name
                                   << ", id " << static_cast<uint16_t> (oid);});
               }
@@ -645,13 +665,13 @@ main (int argc, char* argv[])
               if (oif->pre != nullptr && (pre_oid = oif->pre (mid)) != 0)
               {
                 assert (pre_oid != default_id);
-                pre_oif = &rs.operations[pre_oid].get ();
+                pre_oif = lookup (pre_oid);
               }
 
               if (oif->post != nullptr && (post_oid = oif->post (mid)) != 0)
               {
                 assert (post_oid != default_id);
-                post_oif = &rs.operations[post_oid].get ();
+                post_oif = lookup (post_oid);
               }
             }
             //
@@ -660,26 +680,27 @@ main (int argc, char* argv[])
             //
             else
             {
-              if (oid > rs.operations.size () ||     // Not a valid index.
-                  oif != &rs.operations[oid].get ()) // Not the same impl.
-                fail (l) << "different operation implementations "
-                         << "in an operation batch";
+              auto check =
+                [&rs, &l, &tn] (operation_id o, const operation_info* i)
+                {
+                  const operation_info* r (rs.operations[o]);
+
+                  if (r == nullptr)
+                    fail (l) << "target " << tn << " does not support "
+                             << "operation " << operation_table[o];
+
+                  if (r != i)
+                    fail (l) << "different implementations of operation "
+                             << i->name << " in the same operation batch";
+                };
+
+              check (oid, oif);
 
               if (pre_oid != 0)
-              {
-                if (pre_oid > rs.operations.size () ||
-                    pre_oif != &rs.operations[pre_oid].get ())
-                  fail (l) << "different pre-operation implementations "
-                           << "in an operation batch";
-              }
+                check (pre_oid, pre_oif);
 
               if (post_oid != 0)
-              {
-                if (post_oid > rs.operations.size () ||
-                    post_oif != &rs.operations[post_oid].get ())
-                  fail (l) << "different post-operation implementations "
-                           << "in an operation batch";
-              }
+                check (post_oid, post_oif);
             }
           }
 
diff --git a/build/bin/module.cxx b/build/bin/module.cxx
index 7dd5f00..e03c658 100644
--- a/build/bin/module.cxx
+++ b/build/bin/module.cxx
@@ -65,6 +65,13 @@ namespace build
         rs.insert<lib> (update_id, "bin.lib", lib_);
         rs.insert<lib> (clean_id, "bin.lib", lib_);
 
+        //@@ Should we check if the install module was loaded
+        //   (by checking if install operation is registered
+        //   for this project)? If we do that, then install
+        //   will have to be loaded before bin. Perhaps we
+        //   should enforce loading of all operation-defining
+        //   modules before all others?
+        //
         rs.insert<lib> (install_id, "bin.lib", lib_);
       }
 
diff --git a/build/config/module.cxx b/build/config/module.cxx
index 237547c..fbb51af 100644
--- a/build/config/module.cxx
+++ b/build/config/module.cxx
@@ -24,15 +24,15 @@ namespace build
     static const path config_file ("build/config.build");
 
     extern "C" void
-    config_init (scope& root,
-                 scope& base,
+    config_init (scope& r,
+                 scope& b,
                  const location& l,
                  std::unique_ptr<module>&,
                  bool first)
     {
       tracer trace ("config::init");
 
-      if (&root != &base)
+      if (&r != &b)
         fail (l) << "config module must be initialized in bootstrap.build";
 
       if (!first)
@@ -41,21 +41,20 @@ namespace build
         return;
       }
 
-      const dir_path& out_root (root.path ());
+      const dir_path& out_root (r.path ());
       level4 ([&]{trace << "for " << out_root;});
 
       // Register meta-operations.
       //
-      if (root.meta_operations.insert (configure) != configure_id ||
-          root.meta_operations.insert (disfigure) != disfigure_id)
-        fail (l) << "config module must be initialized before other modules";
+      r.meta_operations.insert (configure_id, configure);
+      r.meta_operations.insert (disfigure_id, disfigure);
 
       // Load config.build if one exists.
       //
       path f (out_root / config_file);
 
       if (file_exists (f))
-        source (f, root, root);
+        source (f, r, r);
     }
   }
 }
diff --git a/build/config/operation b/build/config/operation
index 514c118..ee5161d 100644
--- a/build/config/operation
+++ b/build/config/operation
@@ -11,9 +11,6 @@ namespace build
 {
   namespace config
   {
-    const meta_operation_id configure_id = 2;
-    const meta_operation_id disfigure_id = 3;
-
     extern meta_operation_info configure;
     extern meta_operation_info disfigure;
   }
diff --git a/build/context.cxx b/build/context.cxx
index e8ecf74..259c6ea 100644
--- a/build/context.cxx
+++ b/build/context.cxx
@@ -40,6 +40,21 @@ namespace build
     scopes.clear ();
     variable_pool.clear ();
 
+    // Reset meta/operation tables. Note that the order should match
+    // the id constants in <build/operation>.
+    //
+    meta_operation_table.clear ();
+    meta_operation_table.insert ("perform");
+    meta_operation_table.insert ("configure");
+    meta_operation_table.insert ("disfigure");
+
+    operation_table.clear ();
+    operation_table.insert ("default");
+    operation_table.insert ("update");
+    operation_table.insert ("clean");
+    operation_table.insert ("test");
+    operation_table.insert ("install");
+
     // Enter builtin variables.
     //
     variable_pool.insert (variable ("subprojects", '='));
diff --git a/build/cxx/module.cxx b/build/cxx/module.cxx
index 1e6ed8a..042fb53 100644
--- a/build/cxx/module.cxx
+++ b/build/cxx/module.cxx
@@ -84,6 +84,8 @@ namespace build
         rs.insert<libso> (update_id, "cxx.link", link::instance);
         rs.insert<libso> (clean_id, "cxx.link", link::instance);
 
+        //@@ Should we check if install module was loaded (see bin)?
+        //
         rs.insert<exe> (install_id, "cxx.install", install::instance);
         rs.insert<liba> (install_id, "cxx.install", install::instance);
         rs.insert<libso> (install_id, "cxx.install", install::instance);
diff --git a/build/file.cxx b/build/file.cxx
index 305c13a..e5deecf 100644
--- a/build/file.cxx
+++ b/build/file.cxx
@@ -118,19 +118,17 @@ namespace build
   {
     scope& rs (scopes.insert (out_root, true).first);
 
-    // Enter built-in meta-operation and operation names. Note that
-    // the order of registration should match the id constants; see
-    // <operation> for details. Loading of modules (via the src
-    // bootstrap; see below) can result in additional names being
-    // added.
+    // Enter built-in meta-operation and operation names. Loading of
+    // modules (via the src bootstrap; see below) can result in
+    // additional meta/operations being added.
     //
     if (rs.meta_operations.empty ())
     {
-      assert (rs.meta_operations.insert (perform) == perform_id);
+      rs.meta_operations.insert (perform_id, perform);
 
-      assert (rs.operations.insert (default_) == default_id);
-      assert (rs.operations.insert (update) == update_id);
-      assert (rs.operations.insert (clean) == clean_id);
+      rs.operations.insert (default_id, default_);
+      rs.operations.insert (update_id, update);
+      rs.operations.insert (clean_id, clean);
     }
 
     // If this is already a root scope, verify that things are
diff --git a/build/install/module.cxx b/build/install/module.cxx
index 1dd655e..834b0e8 100644
--- a/build/install/module.cxx
+++ b/build/install/module.cxx
@@ -104,8 +104,10 @@ namespace build
 
       // Register the install operation.
       //
-      assert (r.operations.insert (install) == install_id);
+      r.operations.insert (install_id, install);
 
+      // Register rules.
+      //
       {
         auto& rs (b.rules);
 
diff --git a/build/install/operation.cxx b/build/install/operation.cxx
index 1ba001f..1246671 100644
--- a/build/install/operation.cxx
+++ b/build/install/operation.cxx
@@ -4,8 +4,6 @@
 
 #include <build/install/operation>
 
-#include <build/config/operation>
-
 using namespace std;
 using namespace butl;
 
@@ -18,7 +16,7 @@ namespace build
     {
       // Run update as a pre-operation, unless we are disfiguring.
       //
-      return mo != config::disfigure_id ? update_id : 0;
+      return mo != disfigure_id ? update_id : 0;
     }
 
     operation_info install {
diff --git a/build/operation b/build/operation
index ad7b911..286e003 100644
--- a/build/operation
+++ b/build/operation
@@ -102,20 +102,26 @@ namespace build
   std::ostream&
   operator<< (std::ostream&, action);
 
-  // Id constants for build-in operations.
+  // Id constants for build-in and pre-defined meta/operations.
   //
   const meta_operation_id perform_id   = 1;
 
+  const meta_operation_id configure_id = 2;
+  const meta_operation_id disfigure_id = 3;
+
   // The default operation is a special marker that can be used to
   // indicate that no operation was explicitly specified by the user.
   //
   const operation_id default_id = 1;
   const operation_id update_id = 2;
   const operation_id clean_id  = 3;
-  const operation_id install_id = 4;
 
-  const action_id perform_update_id = (perform_id << 4) | update_id;
-  const action_id perform_clean_id  = (perform_id << 4) | clean_id;
+  const operation_id test_id = 4;
+  const operation_id install_id = 5;
+
+  const action_id perform_update_id   = (perform_id << 4) | update_id;
+  const action_id perform_clean_id    = (perform_id << 4) | clean_id;
+  const action_id perform_test_id     = (perform_id << 4) | test_id;
   const action_id perform_install_id  = (perform_id << 4) | install_id;
 
   // Recipe execution mode.
@@ -272,40 +278,76 @@ namespace build
     operation_id (*post) (meta_operation_id);
   };
 
-  // Build-in operations.
+  // Built-in operations.
   //
   extern operation_info default_;
   extern operation_info update;
   extern operation_info clean;
 
-  // Meta/operation tables.
+  // Global meta/operation tables. Each registered meta/operation
+  // is assigned an id which is used as an index in the per-project
+  // registered meta/operation lists.
   //
-  using meta_operation_table = butl::string_table<
-    meta_operation_id,
-    std::reference_wrapper<const meta_operation_info>>;
-
-  using operation_table = butl::string_table<
-    operation_id,
-    std::reference_wrapper<const operation_info>>;
-}
+  // We have three types of meta/operations: built-in (e.g., perform,
+  // update), pre-defined (e.g., configure, test), and dynamically-
+  // defined. For built-in ones, both the id and implementation are
+  // part of the build2 core. For pre-defined, the id is registered
+  // as part of the core but the implementation is loaded as part of
+  // a module. The idea with pre-defined operations is that they have
+  // common, well-established semantics but could still be optional.
+  // Another aspect of pre-defined operations is that often rules
+  // across multiple modules need to know their ids. Finally,
+  // dynamically-defined meta/operations have their ids registered
+  // as part of a module load. In this case, the meta/operation is
+  // normally (but not necessarily) fully implemented by this module.
+  //
+  // Note also that the name of a meta/operation in a sense defines
+  // its semantics. It would be strange to have an operation called
+  // test that does two very different things in different projects.
+  //
+  extern butl::string_table<meta_operation_id> meta_operation_table;
+  extern butl::string_table<operation_id> operation_table;
 
-namespace butl
-{
-  template <>
-  struct string_table_traits<
-    std::reference_wrapper<const build::meta_operation_info>>
+  // These are "sparse" in the sense that we may have "holes" that
+  // are represented as NULL pointers. Also, lookup out of bounds
+  // is treated as a hole.
+  //
+  template <typename T>
+  struct sparse_vector
   {
-    static const std::string&
-    key (const build::meta_operation_info& x) {return x.name;}
-  };
+    using base_type = std::vector<T*>;
+    using size_type = typename base_type::size_type;
+
+    void
+    insert (size_type i, T& x)
+    {
+      size_type n (v_.size ());
+
+      if (i < n)
+        v_[i] = &x;
+      else
+      {
+        if (n != i)
+          v_.resize (i, nullptr); // Add holes.
+        v_.push_back (&x);
+      }
+    }
+
+    T*
+    operator[] (size_type i) const
+    {
+      return i < v_.size () ? v_[i] : nullptr;
+    }
 
-  template <>
-  struct string_table_traits<
-    std::reference_wrapper<const build::operation_info>>
-  {
-    static const std::string&
-    key (const build::operation_info& x) {return x.name;}
+    bool
+    empty () const {return v_.empty ();}
+
+  private:
+    base_type v_;
   };
+
+  using meta_operations = sparse_vector<const meta_operation_info>;
+  using operations = sparse_vector<const operation_info>;
 }
 
 #endif // BUILD_OPERATION
diff --git a/build/operation.cxx b/build/operation.cxx
index 694ad1c..607658f 100644
--- a/build/operation.cxx
+++ b/build/operation.cxx
@@ -228,4 +228,9 @@ namespace build
     nullptr,
     nullptr
   };
+
+  // Tables.
+  //
+  string_table<meta_operation_id> meta_operation_table;
+  string_table<operation_id> operation_table;
 }
diff --git a/build/scope b/build/scope
index 9fb8061..6edb43a 100644
--- a/build/scope
+++ b/build/scope
@@ -116,8 +116,8 @@ namespace build
     // Meta/operations supported by this project (set on the root
     // scope only).
     //
-    meta_operation_table meta_operations;
-    operation_table operations;
+    build::meta_operations meta_operations;
+    build::operations operations;
 
     typedef build::path path_type;
 
diff --git a/build/test/module.cxx b/build/test/module.cxx
index d5ba754..cba930f 100644
--- a/build/test/module.cxx
+++ b/build/test/module.cxx
@@ -19,24 +19,18 @@ namespace build
 {
   namespace test
   {
-    class module: public build::module
-    {
-    public:
-      module (operation_id test_id): rule (test_id) {}
-
-      test::rule rule;
-    };
+    static rule rule_;
 
     extern "C" void
-    test_init (scope& root,
-               scope& base,
+    test_init (scope& r,
+               scope& b,
                const location& l,
-               unique_ptr<build::module>& r,
+               unique_ptr<build::module>&,
                bool first)
     {
       tracer trace ("test::init");
 
-      if (&root != &base)
+      if (&r != &b)
         fail (l) << "test module must be initialized in bootstrap.build";
 
       if (!first)
@@ -45,17 +39,17 @@ namespace build
         return;
       }
 
-      const dir_path& out_root (root.path ());
+      const dir_path& out_root (r.path ());
       level4 ([&]{trace << "for " << out_root;});
 
       // Register the test operation.
       //
-      operation_id test_id (root.operations.insert (test));
-
-      unique_ptr<module> m (new module (test_id));
+      r.operations.insert (test_id, test);
 
+      // Register rules.
+      //
       {
-        auto& rs (base.rules);
+        auto& rs (r.rules);
 
         // Register the standard alias rule for the test operation.
         //
@@ -63,10 +57,8 @@ namespace build
 
         // Register our test running rule.
         //
-        rs.insert<target> (test_id, "test", m->rule);
+        rs.insert<target> (test_id, "test", rule_);
       }
-
-      r = move (m);
     }
   }
 }
diff --git a/build/test/operation.cxx b/build/test/operation.cxx
index da98e7d..40c5b86 100644
--- a/build/test/operation.cxx
+++ b/build/test/operation.cxx
@@ -4,8 +4,6 @@
 
 #include <build/test/operation>
 
-#include <build/config/operation>
-
 using namespace std;
 using namespace butl;
 
@@ -18,7 +16,7 @@ namespace build
     {
       // Run update as a pre-operation, unless we are disfiguring.
       //
-      return mo != config::disfigure_id ? update_id : 0;
+      return mo != disfigure_id ? update_id : 0;
     }
 
     operation_info test {
diff --git a/build/test/rule b/build/test/rule
index 259b72b..17f32ce 100644
--- a/build/test/rule
+++ b/build/test/rule
@@ -15,8 +15,6 @@ namespace build
     class rule: public build::rule
     {
     public:
-      rule (operation_id o): test_id (o) {}
-
       virtual match_result
       match (action, target&, const std::string&) const;
 
@@ -25,9 +23,6 @@ namespace build
 
       static target_state
       perform_test (action, target&);
-
-    private:
-      operation_id test_id;
     };
   }
 }