Various design/implementation cleanups

1 files changed, 102 insertions, 64 deletions

diff --git a/build2/variable b/build2/variable
index 000ac9c..326c290 100644
--- a/build2/variable
+++ b/build2/variable
@@ -8,9 +8,10 @@
 #include <map>
 #include <functional>    // hash
 #include <type_traits>   // aligned_storage
-#include <unordered_set>
+#include <unordered_map>
 
 #include <butl/prefix-map>
+#include <butl/multi-index> // map_key
 
 #include <build2/types>
 #include <build2/utility>
@@ -654,107 +655,144 @@ namespace build2
     static const string type_name;
     static const build2::value_type value_type;
   };
-}
 
-// Variable map.
-//
-namespace std
-{
-  template <>
-  struct hash<build2::variable>: hash<string>
+  // variable_pool
+  //
+  class variable_pool
   {
-    size_t
-    operator() (const build2::variable& v) const noexcept
-    {
-      return hash<string>::operator() (v.name);
-    }
-  };
-}
+  public:
+    // Find existing or insert new. Find bias.
+    //
+    const variable&
+    operator[] (const string& name);
 
-namespace butl
-{
-  template <>
-  struct compare_prefix<std::reference_wrapper<const build2::variable>>:
-    compare_prefix<std::string>
-  {
-    typedef compare_prefix<std::string> base;
+    // Find existing or insert new. Insert bias.
+    //
+    const variable&
+    insert (string name);
 
-    explicit
-    compare_prefix (char d): base (d) {}
+    // Return NULL if there is no variable with this name.
+    //
+    const variable*
+    find (const string& name);
 
-    bool
-    operator() (const build2::variable& x, const build2::variable& y) const
+    // Insert or override.
+    //
+    template <typename T>
+    const variable&
+    insert (string name)
     {
-      return base::operator() (x.name, y.name);
+      return insert (
+        move (name), &value_traits<T>::value_type, nullptr, nullptr);
     }
 
-    bool
-    prefix (const build2::variable& p, const build2::variable& k) const
+    const variable&
+    insert (string name, variable_visibility v)
     {
-      return base::prefix (p.name, k.name);
+      return insert (move (name), nullptr, &v, nullptr);
     }
-  };
-}
 
-namespace build2
-{
-  // variable_pool
-  //
-  using variable_pool_base = std::unordered_set<variable>;
-  struct variable_pool: private variable_pool_base
-  {
     const variable&
-    insert (string name, variable_visibility v = variable_visibility::normal)
+    insert (string name, bool overridable)
     {
-      return insert (move (name), nullptr, v, false);
+      return insert (move (name), nullptr, nullptr, &overridable);
     }
 
     const variable&
-    insert (string name,
-            bool overridable,
-            variable_visibility v = variable_visibility::normal)
+    insert (string name, bool overridable, variable_visibility v)
     {
-      return insert (move (name), nullptr, v, overridable);
+      return insert (move (name), nullptr, &v, &overridable);
     }
 
     template <typename T>
     const variable&
-    insert (string name, variable_visibility v = variable_visibility::normal)
+    insert (string name, variable_visibility v)
     {
-      return insert (move (name), &value_traits<T>::value_type, v, false);
+      return insert (move (name), &value_traits<T>::value_type, &v, nullptr);
     }
 
     template <typename T>
     const variable&
-    insert (string name,
-            bool overridable,
-            variable_visibility v = variable_visibility::normal)
+    insert (string name, bool overridable)
     {
       return insert (
-        move (name), &value_traits<T>::value_type, v, overridable);
+        move (name), &value_traits<T>::value_type, nullptr, &overridable);
     }
 
+    template <typename T>
     const variable&
-    find (const string& name); //@@ TODO: Move to operator[], remove.
-                               //@@ ranmae var_pool to varpool or vpool?
-
-    const variable&
-    operator[] (const string& name) {return find (name);}
+    insert (string name, bool overridable, variable_visibility v)
+    {
+      return insert (
+        move (name), &value_traits<T>::value_type, &v, &overridable);
+    }
 
-    using variable_pool_base::clear;
+    void
+    clear () {map_.clear ();}
 
   private:
     const variable&
     insert (string name,
             const build2::value_type*,
-            variable_visibility,
-            bool overridable);
+            const variable_visibility*,
+            const bool* overridable);
+
+  private:
+    using key = butl::map_key<string>;
+    using map = std::unordered_map<key, variable>;
+
+    pair<map::iterator, bool>
+    insert (variable&& var)
+    {
+      // Keeping a pointer to the key while moving things during insertion is
+      // tricky. We could use a C-string instead of C++ for a key but that
+      // gets hairy very quickly (there is no std::hash for C-strings). So
+      // let's rely on small object-optimized std::string for now.
+      //
+      string n (var.name);
+      auto r (map_.insert (map::value_type (&n, move (var))));
+
+      if (r.second)
+        r.first->first.p = &r.first->second.name;
+
+      return r;
+    }
+
+    map map_;
   };
 
   extern variable_pool var_pool;
+}
 
-  // variable_map
-  //
+// variable_map
+//
+namespace butl
+{
+  template <>
+  struct compare_prefix<std::reference_wrapper<const build2::variable>>:
+    compare_prefix<std::string>
+  {
+    typedef compare_prefix<std::string> base;
+
+    explicit
+    compare_prefix (char d): base (d) {}
+
+    bool
+    operator() (const build2::variable& x, const build2::variable& y) const
+    {
+      return base::operator() (x.name, y.name);
+    }
+
+    bool
+    prefix (const build2::variable& p, const build2::variable& k) const
+    {
+      return base::prefix (p.name, k.name);
+    }
+  };
+}
+
+namespace build2
+{
   class variable_map
   {
   public:
@@ -794,7 +832,7 @@ namespace build2
     lookup
     operator[] (const string& name) const
     {
-      return operator[] (var_pool.find (name));
+      return operator[] (var_pool[name]);
     }
 
     // If typed is false, leave the value untyped even if the variable is.
@@ -833,13 +871,13 @@ namespace build2
     pair<reference_wrapper<value>, bool>
     insert (const string& name, bool typed = true)
     {
-      return insert (var_pool.find (name), typed);
+      return insert (var_pool[name], typed);
     }
 
     pair<const_iterator, const_iterator>
     find_namespace (const string& ns) const
     {
-      auto r (m_.find_prefix (var_pool.find (ns)));
+      auto r (m_.find_prefix (var_pool[ns]));
       return make_pair (const_iterator (r.first), const_iterator (r.second));
     }