From 0d5234f4aefd3cc5b5948cc1b9dd009e50046f5e Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Mon, 17 Aug 2015 11:18:10 +0200
Subject: Tighten variable_map interface

---
 build/scope.cxx     |  8 +++----
 build/target.cxx    | 16 +++++---------
 build/test/rule.cxx |  4 +++-
 build/variable      | 61 +++++++++++++++++++++++++++++++++++++----------------
 4 files changed, 54 insertions(+), 35 deletions(-)

(limited to 'build')

diff --git a/build/scope.cxx b/build/scope.cxx
index 600c46e..d878726 100644
--- a/build/scope.cxx
+++ b/build/scope.cxx
@@ -17,13 +17,11 @@ namespace build
   {
     for (const scope* s (this); s != nullptr; s = s->parent_scope ())
     {
-      auto i (s->vars.find (var));
-      if (i != s->vars.end ())
-        // @@ Same issue as in variable_map: need ro_value_proxy.
-        return value_proxy (&const_cast<value_ptr&> (i->second), &s->vars);
+      if (const value_ptr* v = s->vars.find (var))
+        return value_proxy (v, &s->vars);
     }
 
-    return value_proxy (nullptr, nullptr);
+    return value_proxy ();
   }
 
   value_proxy scope::
diff --git a/build/target.cxx b/build/target.cxx
index 37cec8c..aaf8a5e 100644
--- a/build/target.cxx
+++ b/build/target.cxx
@@ -119,18 +119,12 @@ namespace build
   value_proxy target::
   operator[] (const variable& var) const
   {
-    auto find = [&var] (const variable_map& vars)
-    {
-      auto i (vars.find (var));
-      return i != vars.end () ? &const_cast<value_ptr&> (i->second) : nullptr;
-    };
-
-    if (auto p = find (vars))
+    if (auto p = vars.find (var))
       return value_proxy (p, &vars);
 
     if (group != nullptr)
     {
-      if (auto p = find (group->vars))
+      if (auto p = group->vars.find (var))
         return value_proxy (p, &group->vars);
     }
 
@@ -141,7 +135,7 @@ namespace build
     {
       if (!s->target_vars.empty ())
       {
-        auto find_specific = [&find, s] (const target& t) -> value_proxy
+        auto find_specific = [&var, s] (const target& t) -> value_proxy
         {
           // Search across target type hierarchy.
           //
@@ -159,7 +153,7 @@ namespace build
             if (j == i->second.end ())
               continue;
 
-            if (auto p = find (j->second))
+            if (auto p = j->second.find (var))
               return value_proxy (p, &j->second);
           }
 
@@ -176,7 +170,7 @@ namespace build
         }
       }
 
-      if (auto p = find (s->vars))
+      if (auto p = s->vars.find (var))
         return value_proxy (p, &s->vars);
     }
 
diff --git a/build/test/rule.cxx b/build/test/rule.cxx
index 30de93c..10b628d 100644
--- a/build/test/rule.cxx
+++ b/build/test/rule.cxx
@@ -33,12 +33,14 @@ namespace build
       // @@ This logic doesn't take into account target type/pattern-
       // specific variables.
       //
+      // @@ Perhaps a find_any(<list-of-vars>)?
+      //
       for (auto p (t.vars.find_namespace ("test"));
            p.first != p.second;
            ++p.first)
       {
         const variable& var (p.first->first);
-        value_ptr& val (p.first->second);
+        const value_ptr& val (p.first->second);
 
         // If we have test, then always use that.
         //
diff --git a/build/variable b/build/variable
index ed8d65c..bc78289 100644
--- a/build/variable
+++ b/build/variable
@@ -177,10 +177,21 @@ namespace build
     const variable_map* vars; // Variable map to which this value belongs.
 
     value_proxy (): vars (nullptr), p (nullptr) {}
-    value_proxy (value_ptr* p, const variable_map* v): vars (v), p (p) {}
+    value_proxy (value_ptr* p, const variable_map* v)
+        : vars (p != nullptr ? v : nullptr), p (p) {}
 
     template <typename T>
-    value_proxy (value_ptr& p, const T& x): vars (&x.vars), p (&p) {}
+    value_proxy (value_ptr& p, const T& x)
+        : value_proxy (&p, &x.vars) {}
+
+    // @@ To do this properly we seem to need ro_value_proxy?
+    //
+    value_proxy (const value_ptr* p, const variable_map* v)
+        : value_proxy (const_cast<value_ptr*> (p), v) {}
+
+    template <typename T>
+    value_proxy (const value_ptr& p, const T& x)
+        : value_proxy (const_cast<value_ptr&> (p), x) {}
 
     void
     rebind (const value_proxy& x) {vars = x.vars; p = x.p;}
@@ -305,18 +316,23 @@ namespace build
 
   // variable_map
   //
-  using variable_map_base = butl::prefix_map<variable_cref, value_ptr, '.'>;
-  struct variable_map: variable_map_base
+  struct variable_map
   {
+    using map_type = butl::prefix_map<variable_cref, value_ptr, '.'>;
+    using size_type = map_type::size_type;
+    using const_iterator = map_type::const_iterator;
+
+    const value_ptr*
+    find (const variable& var) const
+    {
+      auto i (m_.find (var));
+      return i != m_.end () ? &i->second : nullptr;
+    }
+
     value_proxy
     operator[] (const variable& var) const
     {
-      auto i (find (var));
-      return i != end ()
-        // @@ To do this properly we seem to need ro_value_proxy.
-        //
-        ? value_proxy (&const_cast<value_ptr&> (i->second), this)
-        : value_proxy (nullptr, nullptr);
+      return value_proxy (find (var), this);
     }
 
     value_proxy
@@ -331,7 +347,7 @@ namespace build
     std::pair<value_proxy, bool>
     assign (const variable& var)
     {
-      auto r (emplace (var, value_ptr ()));
+      auto r (m_.emplace (var, value_ptr ()));
       return std::make_pair (value_proxy (&r.first->second, this), r.second);
     }
 
@@ -341,17 +357,26 @@ namespace build
       return assign (variable_pool.find (name));
     }
 
-    std::pair<iterator, iterator>
-    find_namespace (const std::string& ns)
-    {
-      return find_prefix (variable_pool.find (ns));
-    }
-
     std::pair<const_iterator, const_iterator>
     find_namespace (const std::string& ns) const
     {
-      return find_prefix (variable_pool.find (ns));
+      return m_.find_prefix (variable_pool.find (ns));
     }
+
+    const_iterator
+    begin () const {return m_.begin ();}
+
+    const_iterator
+    end () const {return m_.end ();}
+
+    bool
+    empty () const {return m_.empty ();}
+
+    size_type
+    size () const {return m_.size ();}
+
+  private:
+    map_type m_;
   };
 
   // Target type/pattern-specific variables.
-- 
cgit v1.1