Redo modules map as vector

4 files changed, 84 insertions, 53 deletions

diff --git a/build2/cli/init.cxx b/build2/cli/init.cxx
index 8f9df4a..eadf32c 100644
--- a/build2/cli/init.cxx
+++ b/build2/cli/init.cxx
@@ -55,7 +55,7 @@ namespace build2
                 scope& bs,
                 const location& loc,
                 bool,
-                bool optional,
+                bool opt,
                 module_init_extra& extra)
     {
       tracer trace ("cli::guess_init");
@@ -90,7 +90,7 @@ namespace build2
           rs,
           name ("cli", dir_path (), "exe", "cli"), // cli%exe{cli}
           true      /* phase2 */,
-          optional,
+          opt,
           true      /* metadata */,
           loc,
           "module load"));
@@ -150,7 +150,7 @@ namespace build2
                  scope& bs,
                  const location& loc,
                  bool,
-                 bool optional,
+                 bool opt,
                  module_init_extra& extra)
     {
       tracer trace ("cli::config_init");
@@ -163,8 +163,8 @@ namespace build2
 
       // Load cli.guess and share its module instance as ours.
       //
-      if (const shared_ptr<build2::module>* r = load_module (
-            rs, rs, "cli.guess", loc, optional, extra.hints))
+      if (optional<shared_ptr<build2::module>> r = load_module (
+            rs, rs, "cli.guess", loc, opt, extra.hints))
       {
         extra.module = *r;
       }
@@ -173,7 +173,7 @@ namespace build2
         // This can happen if someone already optionally loaded cli.guess
         // and it has failed to configure.
         //
-        if (!optional)
+        if (!opt)
           fail (loc) << "cli could not be configured" <<
             info << "re-run with -V for more information";
 
@@ -198,7 +198,7 @@ namespace build2
           scope& bs,
           const location& loc,
           bool,
-          bool optional,
+          bool opt,
           module_init_extra& extra)
     {
       tracer trace ("cli::init");
@@ -220,8 +220,8 @@ namespace build2
 
       // Load cli.config and get its module instance.
       //
-      if (const shared_ptr<build2::module>* r = load_module (
-            rs, rs, "cli.config", loc, optional, extra.hints))
+      if (optional<shared_ptr<build2::module>> r = load_module (
+            rs, rs, "cli.config", loc, opt, extra.hints))
       {
         extra.module = *r;
       }
@@ -230,7 +230,7 @@ namespace build2
         // This can happen if someone already optionally loaded cli.config
         // and it has failed to configure.
         //
-        if (!optional)
+        if (!opt)
           fail (loc) << "cli could not be configured" <<
             info << "re-run with -V for more information";
 
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index 81fca32..171d136 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -1448,20 +1448,16 @@ namespace build2
 
     // Finish off initializing bootstrapped modules.
     //
-    for (auto& p: root.root_extra->modules)
+    for (auto& s: root.root_extra->modules)
     {
-      module_state& s (p.second);
-
       if (s.boot && s.first)
-        init_module (root, root, p.first, s.loc);
+        init_module (root, root, s.name, s.loc);
     }
 
-    for (auto& p: root.root_extra->modules)
+    for (auto& s: root.root_extra->modules)
     {
-      module_state& s (p.second);
-
       if (s.boot && !s.first)
-        init_module (root, root, p.first, s.loc);
+        init_module (root, root, s.name, s.loc);
     }
 
     // Load hooks and root.build.
diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx
index d45ceed..bb877d9 100644
--- a/libbuild2/module.cxx
+++ b/libbuild2/module.cxx
@@ -578,12 +578,10 @@ namespace build2
 
     if (i != lm.end ())
     {
-      module_state& s (i->second);
-
       // The only valid situation here is if the module has already been
       // bootstrapped.
       //
-      assert (s.boot);
+      assert (i->boot);
       return;
     }
 
@@ -596,12 +594,22 @@ namespace build2
       fail (loc) << "build system module " << mod << " should not be loaded "
                  << "during bootstrap";
 
-    i = lm.emplace (mod,
-                    module_state {true, false, mf.init, nullptr, loc}).first;
+    lm.push_back (module_state {true, false, mod, mf.init, nullptr, loc});
+    i = lm.end () - 1;
 
     {
-      module_boot_extra extra {i->second.module};
-      i->second.first = mf.boot (rs, loc, extra);
+      module_boot_extra e;
+
+      // Note: boot() can load additional modules invalidating the iterator.
+      //
+      size_t j (i - lm.begin ());
+      bool f (mf.boot (rs, loc, e));
+      i = lm.begin () + j;
+
+      i->first = f;
+
+      if (e.module != nullptr)
+        i->module = move (e.module);
     }
 
     rs.assign (rs.var_pool ().insert (mod + ".booted")) = true;
@@ -632,14 +640,13 @@ namespace build2
           fail (loc) << "build system module " << mod << " should be loaded "
                      << "during bootstrap";
 
-        i = lm.emplace (
-          mod,
-          module_state {false, false, mf->init, nullptr, loc}).first;
+        lm.push_back (module_state {false, false, mod, mf->init, nullptr, loc});
+        i = lm.end () - 1;
       }
     }
     else
     {
-      module_state& s (i->second);
+      module_state& s (*i);
 
       if (s.boot)
       {
@@ -692,15 +699,26 @@ namespace build2
 
       if ((c = l))
       {
-        module_init_extra extra {i->second.module, hints};
-        c = i->second.init (rs, bs, loc, f, opt, extra);
+        module_init_extra e {i->module, hints};
+
+        // Note: init() can load additional modules invalidating the iterator.
+        //
+        size_t j (i - lm.begin ());
+        c = i->init (rs, bs, loc, f, opt, e);
+        i = lm.begin () + j;
+
+        if (e.module != i->module)
+        {
+          assert (i->module == nullptr);
+          i->module = move (e.module);
+        }
       }
 
       lv = l;
       cv = c;
     }
 
-    return l && c ? &i->second : nullptr;
+    return l && c ? &*i : nullptr;
   }
 
   // @@ TODO: This is a bit of a fuzzy mess:
@@ -715,7 +733,7 @@ namespace build2
   // Note that it would have been nice to keep these inline but we need the
   // definition of scope for the variable lookup.
   //
-  const shared_ptr<module>*
+  optional<shared_ptr<module>>
   load_module (scope& rs,
                scope& bs,
                const string& name,
@@ -726,18 +744,18 @@ namespace build2
     if (cast_false<bool> (bs[name + ".loaded"]))
     {
       if (cast_false<bool> (bs[name + ".configured"]))
-        return &rs.root_extra->modules.find (name)->second.module;
+        return rs.root_extra->modules.find (name)->module;
     }
     else
     {
       if (module_state* ms = init_module (rs, bs, name, loc, opt, hints))
-        return &ms->module;
+        return ms->module;
     }
 
-    return nullptr;
+    return nullopt;
   }
 
-  const shared_ptr<module>&
+  shared_ptr<module>
   load_module (scope& rs,
                scope& bs,
                const string& name,
@@ -748,7 +766,7 @@ namespace build2
     //   attempt to load it was optional?
 
     return cast_false<bool> (bs[name + ".loaded"])
-      ? rs.root_extra->modules.find (name)->second.module
+      ? rs.root_extra->modules.find (name)->module
       : init_module (rs, bs, name, loc, false /* optional */, hints)->module;
   }
 }
diff --git a/libbuild2/module.hxx b/libbuild2/module.hxx
index f97bc60..0305e2c 100644
--- a/libbuild2/module.hxx
+++ b/libbuild2/module.hxx
@@ -42,7 +42,7 @@ namespace build2
   //
   struct module_boot_extra
   {
-    shared_ptr<build2::module>& module; // Module instance (out).
+    shared_ptr<build2::module> module; // Module instance (out).
 
     // Convenience functions.
     //
@@ -53,9 +53,8 @@ namespace build2
     T& module_as () {assert (module); return static_cast<T&> (*module);}
   };
 
-  // Return true if the module should be initialized first (the order of
-  // initialization within the resulting two groups of modules is
-  // unspecified).
+  // Return true if the module should be initialized first (within the
+  // resulting two groups the modules are initializated in the order loaded).
   //
   using module_boot_function =
     bool (scope& root,
@@ -66,8 +65,8 @@ namespace build2
   //
   struct module_init_extra
   {
-    shared_ptr<build2::module>& module; // Module instance (in/out).
-    const variable_map&         hints;  // Configuration hints (see below).
+    shared_ptr<build2::module> module; // Module instance (in/out).
+    const variable_map&        hints;  // Configuration hints (see below).
 
     // Convenience functions.
     //
@@ -125,20 +124,37 @@ namespace build2
   {
     bool boot;  // True if the module boot'ed but not yet init'ed.
     bool first; // True if the boot'ed module must be init'ed first.
+    const string name;
     module_init_function* init;
     shared_ptr<build2::module> module;
     location_value loc; // Boot location.
   };
 
-  struct module_map: std::map<string, module_state>
+  struct module_map: vector<module_state>
   {
+    iterator
+    find (const string& name)
+    {
+      return find_if (
+        begin (), end (),
+        [&name] (const module_state& s) {return s.name == name;});
+    }
+
+    const_iterator
+    find (const string& name) const
+    {
+      return find_if (
+        begin (), end (),
+        [&name] (const module_state& s) {return s.name == name;});
+    }
+
     template <typename T>
     T*
     find_module (const string& name) const
     {
       auto i (find (name));
       return i != end ()
-        ? static_cast<T*> (i->second.module.get ())
+        ? static_cast<T*> (i->module.get ())
         : nullptr;
     }
   };
@@ -152,7 +168,8 @@ namespace build2
   // parser but also by some modules to init prerequisite modules. Return a
   // pointer to the corresponding module state if the module was both
   // successfully loaded and configured and NULL otherwise (which can only
-  // happen if optional is true). Note that the return can be used as a bool.
+  // happen if optional is true). Note that the result can be used as a bool
+  // but should not be assumed stable (vector resize).
   //
   // The config_hints variable map can be used to pass configuration hints
   // from one module to another. For example, the cxx modude may pass the
@@ -170,18 +187,18 @@ namespace build2
 
   // A wrapper over init_module() to use from other modules that incorporates
   // the <name>.loaded variable check (use init_module() directly to sidestep
-  // this check). Return a pointer to the pointer to the module instance if it
-  // was both successfully loaded and configured and NULL otherwise (so can be
-  // used as bool).
+  // this check). Return an optional pointer to the module instance that is
+  // present if the module was both successfully loaded and configured and
+  // absent otherwise (so can be used as bool).
   //
-  // Note also that NULL can be returned even of optional is false which
+  // Note also that absent can be returned even of optional is false which
   // happens if the requested module has already been loaded but failed to
   // configure. The function could have issued diagnostics but the caller can
   // normally provide additional information.
   //
   // Note: for non-optional modules use the versions below.
   //
-  LIBBUILD2_SYMEXPORT const shared_ptr<module>*
+  LIBBUILD2_SYMEXPORT optional<shared_ptr<module>>
   load_module (scope& root,
                scope& base,
                const string& name,
@@ -191,7 +208,7 @@ namespace build2
 
   // As above but always load and return a pointer to the module instance.
   //
-  LIBBUILD2_SYMEXPORT const shared_ptr<module>&
+  LIBBUILD2_SYMEXPORT shared_ptr<module>
   load_module (scope& root,
                scope& base,
                const string& name,