From 6535bf6175af32e2514faf75d2742424751a783b Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Wed, 15 Apr 2015 14:10:50 +0200
Subject: New variables architecture

Now operator[] is only used for lookup.
---
 build/b.cxx                |  4 ++--
 build/config/operation.cxx | 11 +++++-----
 build/context.cxx          |  4 ++--
 build/cxx/module.cxx       | 30 +++++++++++++-------------
 build/cxx/rule.cxx         | 12 +----------
 build/dump.cxx             |  2 +-
 build/file.cxx             | 22 +++++++++----------
 build/operation.cxx        |  4 ++--
 build/parser.cxx           | 22 +++++++++----------
 build/scope                | 52 ++++++++++++++++++++++++++++++++++----------
 build/scope.cxx            | 27 ++++++++++++++++++-----
 build/target               | 42 ++++++++++++++++++++++++++++--------
 build/target.cxx           | 25 +++++++++++++++++----
 build/variable             | 54 ++++++++++++++++++++++++----------------------
 build/variable.ixx         |  7 ++++++
 15 files changed, 202 insertions(+), 116 deletions(-)

diff --git a/build/b.cxx b/build/b.cxx
index 3fe9d20..2e36003 100644
--- a/build/b.cxx
+++ b/build/b.cxx
@@ -468,7 +468,7 @@ main (int argc, char* argv[])
           // See if the bootstrap process set/changed src_root.
           //
           {
-            auto v (rs.variables["src_root"]);
+            auto v (rs.assign ("src_root"));
 
             if (v)
             {
@@ -528,7 +528,7 @@ main (int argc, char* argv[])
           // Why don't we support it? Because things are already complex
           // enough here.
           //
-          if (auto v = rs.ro_variables ()["subprojects"])
+          if (auto v = rs.vars["subprojects"])
           {
             for (const name& n: v.as<const list_value&> ())
             {
diff --git a/build/config/operation.cxx b/build/config/operation.cxx
index 14f2a0a..ec9e4e6 100644
--- a/build/config/operation.cxx
+++ b/build/config/operation.cxx
@@ -88,7 +88,7 @@ namespace build
             << "# feel free to edit." << endl
             << "#" << endl;
 
-        if (auto v = root.ro_variables ()["amalgamation"])
+        if (auto v = root.vars["amalgamation"])
         {
           const dir_path& d (v.as<const dir_path&> ());
 
@@ -99,7 +99,7 @@ namespace build
         // Save all the variables in the config namespace that are set
         // on the project's root scope.
         //
-        for (auto p (root.variables.find_namespace ("config"));
+        for (auto p (root.vars.find_namespace ("config"));
              p.first != p.second;
              ++p.first)
         {
@@ -178,7 +178,7 @@ namespace build
 
       // Configure subprojects that have been loaded.
       //
-      if (auto v = root.ro_variables ()["subprojects"])
+      if (auto v = root.vars["subprojects"])
       {
         for (const name& n: v.as<const list_value&> ())
         {
@@ -272,7 +272,7 @@ namespace build
       // Disfigure subprojects. Since we don't load buildfiles during
       // disfigure, we do it for all known subprojects.
       //
-      if (auto v = root.ro_variables ()["subprojects"])
+      if (auto v = root.vars["subprojects"])
       {
         for (const name& n: v.as<const list_value&> ())
         {
@@ -286,8 +286,7 @@ namespace build
 
           // Check if the bootstrap process changed src_root.
           //
-          const dir_path& p (
-            nroot.variables["src_root"].as<const dir_path&> ());
+          const dir_path& p (nroot.vars["src_root"].as<const dir_path&> ());
 
           if (src_nroot != p)
             fail << "bootstrapped src_root " << p << " does not match "
diff --git a/build/context.cxx b/build/context.cxx
index d87ce11..958fb98 100644
--- a/build/context.cxx
+++ b/build/context.cxx
@@ -40,8 +40,8 @@ namespace build
     global_scope = &scopes[dir_path ("/")];
 #endif
 
-    global_scope->variables["work"] = work;
-    global_scope->variables["home"] = home;
+    global_scope->assign ("work") = work;
+    global_scope->assign ("home") = home;
   }
 
   fs_status<mkdir_status>
diff --git a/build/cxx/module.cxx b/build/cxx/module.cxx
index 3c9a4b6..e5c71e7 100644
--- a/build/cxx/module.cxx
+++ b/build/cxx/module.cxx
@@ -49,8 +49,8 @@ namespace build
 
         if (val)
         {
-          if (val.scope != global_scope)
-            break; // A value from config.build.
+          if (!val.belongs (*global_scope))
+            break; // A value from (some) config.build.
 
           v = val.as<const string&> ();
         }
@@ -96,7 +96,7 @@ namespace build
 
         // Set on the project root.
         //
-        root.variables["config.cxx"] = move (v);
+        root.assign ("config.cxx") = move (v);
       }
 
       // config.cxx.{p,c,l}options
@@ -113,11 +113,11 @@ namespace build
 
         if (v.defined ())
         {
-          if (v.scope == global_scope)
-            root.variables["config.cxx.poptions"] = v;
+          if (v.belongs (*global_scope))
+            root.assign ("config.cxx.poptions") = v;
         }
         else
-          root.variables["config.cxx.poptions"]; // Set to NULL.
+          root.assign ("config.cxx.poptions") = nullptr;
       }
 
       {
@@ -125,11 +125,11 @@ namespace build
 
         if (v.defined ())
         {
-          if (v.scope == global_scope)
-            root.variables["config.cxx.coptions"] = v;
+          if (v.belongs (*global_scope))
+            root.assign ("config.cxx.coptions") = v;
         }
         else
-          root.variables["config.cxx.coptions"]; // Set to NULL.
+          root.assign ("config.cxx.coptions") = nullptr;
       }
 
       {
@@ -137,11 +137,11 @@ namespace build
 
         if (v.defined ())
         {
-          if (v.scope == global_scope)
-            root.variables["config.cxx.loptions"] = v;
+          if (v.belongs (*global_scope))
+            root.assign ("config.cxx.loptions") = v;
         }
         else
-          root.variables["config.cxx.loptions"]; // Set to NULL.
+          root.assign ("config.cxx.loptions") = nullptr;
       }
 
       {
@@ -149,11 +149,11 @@ namespace build
 
         if (v.defined ())
         {
-          if (v.scope == global_scope)
-            root.variables["config.cxx.libs"] = v;
+          if (v.belongs (*global_scope))
+            root.assign ("config.cxx.libs") = v;
         }
         else
-          root.variables["config.cxx.libs"]; // Set to NULL.
+          root.assign ("config.cxx.libs") = nullptr;
       }
     }
   }
diff --git a/build/cxx/rule.cxx b/build/cxx/rule.cxx
index a7ec2a6..bc105cf 100644
--- a/build/cxx/rule.cxx
+++ b/build/cxx/rule.cxx
@@ -576,17 +576,7 @@ namespace build
         // Set the -fPIC option if we are building a shared object.
         //
         if (so)
-        {
-          auto var (ot.variables["cxx.coptions"]);
-
-          if (!var)
-          {
-            if (auto var1 = ot.base_scope ()["cxx.coptions"])
-              var = var1;
-          }
-
-          var += "-fPIC";
-        }
+          ot.append ("cxx.coptions") += "-fPIC";
 
         // If this target already exists, then it needs to be "compatible"
         // with what we are doing here.
diff --git a/build/dump.cxx b/build/dump.cxx
index b41e489..dbd9c19 100644
--- a/build/dump.cxx
+++ b/build/dump.cxx
@@ -57,7 +57,7 @@ namespace build
 
     // Variables.
     //
-    for (const auto& e: p.variables)
+    for (const auto& e: p.vars)
     {
       const variable& var (e.first);
       const value_ptr& val (e.second);
diff --git a/build/file.cxx b/build/file.cxx
index faffce5..72b8d37 100644
--- a/build/file.cxx
+++ b/build/file.cxx
@@ -90,7 +90,7 @@ namespace build
     // consistent.
     //
     {
-      auto v (rs.variables["out_root"]);
+      auto v (rs.assign ("out_root"));
 
       if (!v)
         v = out_root;
@@ -106,7 +106,7 @@ namespace build
 
     if (!src_root.empty ())
     {
-      auto v (rs.variables["src_root"]);
+      auto v (rs.assign ("src_root"));
 
       if (!v)
         v = src_root;
@@ -161,7 +161,7 @@ namespace build
   void
   create_bootstrap_outer (scope& root)
   {
-    auto v (root.ro_variables ()["amalgamation"]);
+    auto v (root.vars["amalgamation"]);
 
     if (!v)
       return;
@@ -178,7 +178,7 @@ namespace build
 
     // Check if the bootstrap process changed src_root.
     //
-    const dir_path& p (rs.variables["src_root"].as<const dir_path&> ());
+    const dir_path& p (rs.vars["src_root"].as<const dir_path&> ());
 
     if (src_root != p)
       fail << "bootstrapped src_root " << p << " does not match "
@@ -193,7 +193,7 @@ namespace build
   scope&
   create_bootstrap_inner (scope& root, const dir_path& out_base)
   {
-    if (auto v = root.ro_variables ()["subprojects"])
+    if (auto v = root.vars["subprojects"])
     {
       for (const name& n: v.as<const list_value&> ())
       {
@@ -215,7 +215,7 @@ namespace build
 
         // Check if the bootstrap process changed src_root.
         //
-        const dir_path& p (rs.variables["src_root"].as<const dir_path&> ());
+        const dir_path& p (rs.vars["src_root"].as<const dir_path&> ());
 
         if (src_root != p)
           fail << "bootstrapped src_root " << p << " does not match "
@@ -262,8 +262,8 @@ namespace build
 
     if (val)
     {
-      if (val.scope == global_scope)
-        iroot.variables[var] = val; // Copy into root scope.
+      if (val.belongs (*global_scope))
+        iroot.assign (var) = val; // Copy into root scope.
     }
     else
       fail (l) << "unable to find out_root for imported " << n <<
@@ -283,7 +283,7 @@ namespace build
 
     // Check that the bootstrap process set src_root.
     //
-    if (auto v = root.ro_variables ()["src_root"])
+    if (auto v = root.vars["src_root"])
     {
       const dir_path& p (v.as<const dir_path&> ());
 
@@ -315,8 +315,8 @@ namespace build
 
     // "Pass" the imported project's roots to the stub.
     //
-    ts.variables["out_root"] = out_root;
-    ts.variables["src_root"] = src_root;
+    ts.assign ("out_root") = out_root;
+    ts.assign ("src_root") = src_root;
 
     // Load the export stub. Note that it is loaded in the context
     // of the importing project, not the imported one. The export
diff --git a/build/operation.cxx b/build/operation.cxx
index bceede5..cabf73b 100644
--- a/build/operation.cxx
+++ b/build/operation.cxx
@@ -49,8 +49,8 @@ namespace build
     //
     scope& base (scopes[out_base]);
 
-    base.variables["out_base"] = out_base;
-    auto v (base.variables["src_base"] = src_base);
+    base.assign ("out_base") = out_base;
+    auto v (base.assign ("src_base") = src_base);
     base.src_path_ = &v.as<const dir_path&> ();
 
     // Load the buildfile unless it has already been loaded.
diff --git a/build/parser.cxx b/build/parser.cxx
index 7c326a6..e055bdb 100644
--- a/build/parser.cxx
+++ b/build/parser.cxx
@@ -440,7 +440,7 @@ namespace build
       //
       if (src_root_ == nullptr)
       {
-        auto v (root_->ro_variables ()["src_root"]);
+        auto v (root_->vars["src_root"]);
         src_root_ = v ? &v.as<const dir_path&> () : nullptr;
       }
     }
@@ -555,8 +555,8 @@ namespace build
       scope* os (scope_);
       scope_ = &scopes[out_base];
 
-      scope_->variables["out_base"] = move (out_base);
-      auto v (scope_->variables["src_base"] = move (src_base));
+      scope_->assign ("out_base") = move (out_base);
+      auto v (scope_->assign ("src_base") = move (src_base));
       scope_->src_path_ = &v.as<const dir_path&> ();
 
       target* odt (default_target_);
@@ -648,8 +648,8 @@ namespace build
 
       const auto& var (variable_pool.find (n.value));
 
-      if (auto val = scope_->ro_variables ()[var])
-        ps->variables[var] = val; //@@ Move?
+      if (auto val = scope_->vars[var])
+        ps->assign (var) = val; //@@ Move?
       else
         fail (l) << "undefined exported variable " << var.name;
     }
@@ -723,7 +723,7 @@ namespace build
 
     if (assign)
     {
-      value_ptr& val (scope_->variables[var]);
+      value_ptr& val (scope_->assign (var));
 
       if (val == nullptr) // Initialization.
       {
@@ -744,11 +744,11 @@ namespace build
         //
         list_value* lv (&val.as<list_value&> ());
 
-        if (val.scope != scope_) // Append to value from parent scope?
+        if (!val.belongs (*scope_)) // Append to value from parent scope?
         {
           list_value_ptr nval (new list_value (*lv));
           lv = nval.get (); // Append to.
-          scope_->variables.emplace (var, move (nval));
+          scope_->vars.emplace (var, move (nval));
         }
 
         lv->insert (lv->end (),
@@ -758,7 +758,7 @@ namespace build
       else // Initialization.
       {
         list_value_ptr nval (new list_value (move (vns)));
-        scope_->variables.emplace (var, move (nval));
+        scope_->vars.emplace (var, move (nval));
       }
     }
   }
@@ -1344,9 +1344,9 @@ namespace build
       // to their return values are not guaranteed to be stable (and,
       // in fact, path()'s is not).
       //
-      out_root_ = &root_->ro_variables ()["out_root"].as<const dir_path&> ();
+      out_root_ = &root_->vars["out_root"].as<const dir_path&> ();
 
-      auto v (root_->ro_variables ()["src_root"]);
+      auto v (root_->vars["src_root"]);
       src_root_ = v ? &v.as<const dir_path&> () : nullptr;
     }
 
diff --git a/build/scope b/build/scope
index e4a8a7d..e3c8776 100644
--- a/build/scope
+++ b/build/scope
@@ -26,6 +26,8 @@ namespace build
     const dir_path&
     src_path () const {return *src_path_;} // Corresponding src path.
 
+    const dir_path* src_path_ {nullptr}; // Cached src_{root,base} var value.
+
     scope*
     parent_scope () const {return parent_;}
 
@@ -40,28 +42,56 @@ namespace build
     bool
     root () const {return root_ == this;}
 
-    // Variable lookup. Note that this is find, not find or insert like
-    // in the variable_map, because we also search in outer scopes. For
-    // the latter use the variables map directly.
+    // Variables.
     //
   public:
+    variable_map vars;
+
+    // Lookup, including in outer scopes. If you only want to lookup
+    // in this scope, do it on the the variables map directly.
+    //
     value_proxy
-    operator[] (const variable&);
+    operator[] (const variable&) const;
 
     value_proxy
-    operator[] (const std::string& name)
+    operator[] (const std::string& name) const
     {
       return operator[] (variable_pool.find (name));
     }
 
-    const dir_path* src_path_ {nullptr}; // Cached src_{root,base} var value.
+    // Return a value_proxy suitable for assignment. If the variable
+    // does not exist in this scope's map, then a new one with the
+    // NULL value is added and returned. Otherwise the existing value
+    // if returned.
+    //
+    value_proxy
+    assign (const variable& var)
+    {
+      return vars.assign (var);
+    }
 
-  public:
-    variable_map variables;
+    value_proxy
+    assign (const std::string& name)
+    {
+      return assign (variable_pool.find (name));
+    }
+
+    // Return a value_proxy suitable for appending. If the variable
+    // does not exist in this scope's map, then outer scopes are
+    // searched for the same variable. If found then a new variable
+    // with the found value is added to this scope and returned.
+    // Otherwise this function proceeds as assign().
+    //
+    value_proxy
+    append (const variable&);
 
-    const variable_map&
-    ro_variables () const {return variables;}
+    value_proxy
+    append (const std::string& name)
+    {
+      return append (variable_pool.find (name));
+    }
 
+  public:
     prerequisite_set prerequisites;
 
     // Meta/operations supported by this project (set on the root
@@ -95,7 +125,7 @@ namespace build
 
     typedef dir_path_map<scope>::const_iterator iterator;
 
-    scope (): variables (this) {}
+    scope () = default;
 
     iterator i_;
     scope* parent_;
diff --git a/build/scope.cxx b/build/scope.cxx
index 6fb32b1..4503c6d 100644
--- a/build/scope.cxx
+++ b/build/scope.cxx
@@ -11,18 +11,35 @@ namespace build
   // scope
   //
   value_proxy scope::
-  operator[] (const variable& var)
+  operator[] (const variable& var) const
   {
-    for (scope* s (this); s != nullptr; s = s->parent_scope ())
+    for (const scope* s (this); s != nullptr; s = s->parent_scope ())
     {
-      auto i (s->variables.find (var));
-      if (i != s->variables.end ())
-        return value_proxy (&i->second, s);
+      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);
     }
 
     return value_proxy (nullptr, nullptr);
   }
 
+  value_proxy scope::
+  append (const variable& var)
+  {
+    value_proxy val (operator[] (var));
+
+    if (val && val.belongs (*this)) // Existing variable in this scope.
+      return val;
+
+    value_proxy r (assign (var));
+
+    if (val)
+      r = val; // Copy value from the outer scope.
+
+    return r;
+  }
+
   // scope_map
   //
   scope_map scopes;
diff --git a/build/target b/build/target
index 5495061..00be7b5 100644
--- a/build/target
+++ b/build/target
@@ -94,8 +94,7 @@ namespace build
     ~target () = default;
 
     target (dir_path d, std::string n, const std::string* e)
-        : dir (std::move (d)), name (std::move (n)), ext (e),
-          variables (nullptr) {}
+        : dir (std::move (d)), name (std::move (n)), ext (e) {}
 
     const dir_path dir;     // Absolute and normalized.
     const std::string name;
@@ -126,22 +125,47 @@ namespace build
     // Target-specific variables.
     //
   public:
-    variable_map variables;
+    variable_map vars;
 
-    const variable_map&
-    ro_variables () const {return variables;}
-
-    // Variable lookup in this target and all its outer scopes.
+    // Lookup, including in outer scopes. If you only want to lookup
+    // in this target, do it on the the variables map directly.
     //
     value_proxy
-    operator[] (const variable&);
+    operator[] (const variable&) const;
 
     value_proxy
-    operator[] (const std::string& name)
+    operator[] (const std::string& name) const
     {
       return operator[] (variable_pool.find (name));
     }
 
+    // Return a value_proxy suitable for assignment. See class scope
+    // for details.
+    //
+    value_proxy
+    assign (const variable& var)
+    {
+      return vars.assign (var);
+    }
+
+    value_proxy
+    assign (const std::string& name)
+    {
+      return assign (variable_pool.find (name));
+    }
+
+    // Return a value_proxy suitable for appending. See class scope
+    // for details.
+    //
+    value_proxy
+    append (const variable&);
+
+    value_proxy
+    append (const std::string& name)
+    {
+      return append (variable_pool.find (name));
+    }
+
   public:
     target_state state;
 
diff --git a/build/target.cxx b/build/target.cxx
index fbba1ed..5be0a8d 100644
--- a/build/target.cxx
+++ b/build/target.cxx
@@ -52,15 +52,32 @@ namespace build
   }
 
   value_proxy target::
-  operator[] (const variable& var)
+  operator[] (const variable& var) const
   {
-    auto i (variables.find (var));
+    auto i (vars.find (var));
 
-    return i != variables.end ()
-      ? value_proxy (&i->second, nullptr)
+    return i != vars.end ()
+      // @@ Same issue as in variable_map: need ro_value_proxy.
+      ? value_proxy (&const_cast<value_ptr&> (i->second), &vars)
       : base_scope ()[var];
   }
 
+  value_proxy target::
+  append (const variable& var)
+  {
+    value_proxy val (operator[] (var));
+
+    if (val && val.belongs (*this)) // Existing variable in this target.
+      return val;
+
+    value_proxy r (assign (var));
+
+    if (val)
+      r = val; // Copy value from the outer scope.
+
+    return r;
+  }
+
   ostream&
   operator<< (ostream& os, const target& t)
   {
diff --git a/build/variable b/build/variable
index 6c14477..48fc519 100644
--- a/build/variable
+++ b/build/variable
@@ -7,6 +7,7 @@
 
 #include <string>
 #include <memory>        // unique_ptr
+#include <cstddef>       // nullptr_t
 #include <utility>       // move()
 #include <cassert>
 #include <functional>    // hash
@@ -19,7 +20,6 @@
 
 namespace build
 {
-  class scope;
   struct value;
 
   struct value_type
@@ -87,10 +87,10 @@ namespace build
   //
   // A variable can be undefined, null, or contain some actual value.
   //
+  struct variable_map;
+
   struct value_proxy
   {
-    typedef build::scope scope_type;
-
     bool
     defined () const {return p != nullptr;}
 
@@ -100,8 +100,6 @@ namespace build
     explicit operator bool () const {return defined () && !null ();}
     explicit operator value_ptr& () const {return *p;}
 
-    scope_type* scope; // If NULL, then this is a target variable.
-
     // Get interface. See available specializations below.
     //
     template <typename T>
@@ -127,9 +125,20 @@ namespace build
     const value_proxy&
     operator= (dir_path) const;
 
+    const value_proxy&
+    operator= (nullptr_t) const;
+
+    // Return true if this value belongs to the specified scope or target.
+    //
+    template <typename T>
+    bool
+    belongs (const T& x) const {return map == &x.vars;}
+
     // Implementation details.
     //
-    value_proxy (value_ptr* p, scope_type* s): p (p), scope (s) {}
+    const variable_map* map; // Variable map to which this value belongs.
+
+    value_proxy (value_ptr* p, const variable_map* m): map (m), p (p) {}
 
   private:
     value_ptr* p;
@@ -215,32 +224,32 @@ namespace build
     typedef prefix_map<variable_cref, value_ptr, '.'> base;
 
     value_proxy
-    operator[] (const std::string& v)
+    operator[] (const variable& var) const
     {
-      return operator[] (variable_pool.find (v));
+      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);
     }
 
     value_proxy
-    operator[] (const variable& v)
+    operator[] (const std::string& name) const
     {
-      return value_proxy (&base::operator[] (v), scope_);
+      return operator[] (variable_pool.find (name));
     }
 
     value_proxy
-    operator[] (const std::string& v) const
+    assign (const variable& var)
     {
-      return operator[] (variable_pool.find (v));
+      return value_proxy (&base::operator[] (var), this);
     }
 
     value_proxy
-    operator[] (const variable& v) const
+    assign (const std::string& name)
     {
-      auto i (find (v));
-      return i != end ()
-        // @@ To do this properly we seem to need ro_value_proxy.
-        //
-        ? value_proxy (&const_cast<value_ptr&> (i->second), scope_)
-        : value_proxy (nullptr, nullptr);
+      return assign (variable_pool.find (name));
     }
 
     std::pair<iterator, iterator>
@@ -254,13 +263,6 @@ namespace build
     {
       return find_prefix (variable_pool.find (ns));
     }
-
-    explicit
-    variable_map (scope* s): scope_ (s) {}
-
-  private:
-    scope* scope_; // Scope to which this map belongs or NULL if this
-                   // is a target variable map.
   };
 }
 
diff --git a/build/variable.ixx b/build/variable.ixx
index c0059c5..2cbe519 100644
--- a/build/variable.ixx
+++ b/build/variable.ixx
@@ -53,4 +53,11 @@ namespace build
     p->reset (new list_value (std::move (v)));
     return *this;
   }
+
+  inline const value_proxy& value_proxy::
+  operator= (nullptr_t) const
+  {
+    p->reset ();
+    return *this;
+  }
 }
-- 
cgit v1.1