From 19af3f6b0873a92582e4787a87a6f14df53ff6ae Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Mon, 10 Oct 2022 18:22:31 +0200
Subject: Preparatory work for public/private variable distinction

We still always use the public var_pool from context but where required,
all access now goes through scope::var_pool().
---
 libbuild2/adhoc-rule-buildscript.cxx   |  40 +++++-----
 libbuild2/bin/init.cxx                 |   4 +
 libbuild2/build/script/parser.cxx      |   6 +-
 libbuild2/build/script/parser.test.cxx |   8 +-
 libbuild2/build/script/script.cxx      |   6 +-
 libbuild2/build/script/script.hxx      |   7 +-
 libbuild2/cc/common.cxx                |   7 +-
 libbuild2/cc/compile-rule.cxx          |   4 +
 libbuild2/cc/init.cxx                  |   7 +-
 libbuild2/cc/link-rule.cxx             |   8 +-
 libbuild2/cc/module.cxx                |   8 +-
 libbuild2/cc/pkgconfig.cxx             |   8 +-
 libbuild2/config/operation.cxx         |   8 ++
 libbuild2/config/utility.cxx           |   6 +-
 libbuild2/config/utility.hxx           |   8 ++
 libbuild2/context.cxx                  |   8 +-
 libbuild2/context.hxx                  |   2 +-
 libbuild2/dist/operation.cxx           |   2 +-
 libbuild2/file.cxx                     |  13 ++-
 libbuild2/functions-builtin.cxx        |   2 +-
 libbuild2/install/utility.cxx          |   2 +
 libbuild2/parser.cxx                   |   9 +--
 libbuild2/prerequisite.cxx             |   2 +-
 libbuild2/prerequisite.hxx             |   6 +-
 libbuild2/scope.cxx                    |   2 +-
 libbuild2/scope.hxx                    |  15 +++-
 libbuild2/target.cxx                   |   2 +
 libbuild2/target.hxx                   |  30 +++----
 libbuild2/test/script/script.cxx       |   4 +-
 libbuild2/variable.cxx                 |  96 ++++++++++++++++++++++-
 libbuild2/variable.hxx                 | 139 +++++++++++++++++++++++++--------
 31 files changed, 357 insertions(+), 112 deletions(-)

diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index 505be1b..a32b3bb 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -27,10 +27,11 @@ namespace build2
   static inline void
   hash_script_vars (sha256& cs,
                     const build::script::script& s,
+                    const scope& bs,
                     const target& t,
                     names& storage)
   {
-    context& ctx (t.ctx);
+    auto& vp (bs.var_pool ());
 
     for (const string& n: s.vars)
     {
@@ -38,7 +39,7 @@ namespace build2
 
       lookup l;
 
-      if (const variable* var = ctx.var_pool.find (n))
+      if (const variable* var = vp.find (n))
         l = t[var];
 
       cs.append (!l.defined () ? '\x1' : l->null ? '\x2' : '\x3');
@@ -221,8 +222,8 @@ namespace build2
 
   struct adhoc_buildscript_rule::match_data
   {
-    match_data (action a, const target& t, bool temp_dir)
-        : env (a, t, temp_dir) {}
+    match_data (action a, const target& t, const scope& bs, bool temp_dir)
+        : env (a, t, bs, temp_dir) {}
 
     build::script::environment env;
     build::script::default_runner run;
@@ -236,8 +237,10 @@ namespace build2
 
   struct adhoc_buildscript_rule::match_data_byproduct
   {
-    match_data_byproduct (action a, const target& t, bool temp_dir)
-        : env (a, t, temp_dir) {}
+    match_data_byproduct (action a, const target& t,
+                          const scope& bs,
+                          bool temp_dir)
+        : env (a, t, bs, temp_dir) {}
 
     build::script::environment env;
     build::script::default_runner run;
@@ -608,7 +611,7 @@ namespace build2
 
       {
         sha256 cs;
-        hash_script_vars (cs, script, t, storage);
+        hash_script_vars (cs, script, bs, t, storage);
 
         if (dd.expect (cs.string ()) != nullptr)
           l4 ([&]{trace << "recipe variable change forcing update of " << t;});
@@ -645,10 +648,10 @@ namespace build2
     if (script.depdb_dyndep_byproduct)
     {
       mdb.reset (new match_data_byproduct (
-                   a, t, script.depdb_preamble_temp_dir));
+                   a, t, bs, script.depdb_preamble_temp_dir));
     }
     else
-      md.reset (new match_data (a, t, script.depdb_preamble_temp_dir));
+      md.reset (new match_data (a, t, bs, script.depdb_preamble_temp_dir));
 
 
     build::script::environment& env (mdb != nullptr ? mdb->env : md->env);
@@ -1176,6 +1179,8 @@ namespace build2
     const file& t (xt.as<file> ());
     const path& tp (t.path ());
 
+    const scope& bs (t.base_scope ());
+
     // Update prerequisites and determine if any of them render this target
     // out-of-date.
     //
@@ -1263,7 +1268,7 @@ namespace build2
       //
       {
         sha256 cs;
-        hash_script_vars (cs, script, t, storage);
+        hash_script_vars (cs, script, bs, t, storage);
 
         if (dd.expect (cs.string ()) != nullptr)
           l4 ([&]{trace << "recipe variable change forcing update of " << t;});
@@ -1294,8 +1299,6 @@ namespace build2
       }
     }
 
-    const scope* bs (nullptr);
-
     // Execute the custom dependency change tracking commands, if present.
     //
     // Note that we share the environment between the execute_depdb_preamble()
@@ -1320,20 +1323,18 @@ namespace build2
       }
     }
 
-    build::script::environment env (a, t, false /* temp_dir */);
+    build::script::environment env (a, t, bs, false /* temp_dir */);
     build::script::default_runner run;
 
     if (depdb_preamble)
     {
-      bs = &t.base_scope ();
-
       if (script.depdb_preamble_temp_dir)
         env.set_temp_dir_variable ();
 
       build::script::parser p (ctx);
 
       run.enter (env, script.start_loc);
-      p.execute_depdb_preamble (a, *bs, t, env, script, run, dd);
+      p.execute_depdb_preamble (a, bs, t, env, script, run, dd);
     }
 
     // Update if depdb mismatch.
@@ -1362,10 +1363,7 @@ namespace build2
     {
       // Prepare to execute the script diag line and/or body.
       //
-      if (bs == nullptr)
-        bs = &t.base_scope ();
-
-      if ((r = execute_update_file (*bs, a, t, env, run)))
+      if ((r = execute_update_file (bs, a, t, env, run)))
       {
         if (!ctx.dry_run)
           dd.check_mtime (tp);
@@ -1604,7 +1602,7 @@ namespace build2
       const scope& bs (t.base_scope ());
       const scope& rs (*bs.root_scope ());
 
-      build::script::environment e (a, t, script.body_temp_dir, deadline);
+      build::script::environment e (a, t, bs, script.body_temp_dir, deadline);
       build::script::parser p (ctx);
 
       if (verb == 1)
diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx
index 2b1df97..265521e 100644
--- a/libbuild2/bin/init.cxx
+++ b/libbuild2/bin/init.cxx
@@ -195,6 +195,8 @@ namespace build2
       //
       const target_triplet* tgt (nullptr);
       {
+        // Note: go straight for the public variable pool.
+        //
         const variable& var (ctx.var_pool["config.bin.target"]);
 
         // We first see if the value was specified via the configuration
@@ -272,6 +274,8 @@ namespace build2
       //
       const string* pat (nullptr);
       {
+        // Note: go straight for the public variable pool.
+        //
         const variable& var (ctx.var_pool["config.bin.pattern"]);
 
         // We first see if the value was specified via the configuration
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 788a782..bcea7e0 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -2526,10 +2526,8 @@ namespace build2
           {
             if (pre_parse_suspended_)
             {
-              const variable* pvar (scope_->ctx.var_pool.find (name));
-
-              if (pvar != nullptr)
-                r = (*scope_)[*pvar];
+              if (const variable* var = scope_->var_pool ().find (name))
+                r = (*scope_)[*var];
             }
 
             if (!depdb_clear_)
diff --git a/libbuild2/build/script/parser.test.cxx b/libbuild2/build/script/parser.test.cxx
index f8c2696..061f3f8 100644
--- a/libbuild2/build/script/parser.test.cxx
+++ b/libbuild2/build/script/parser.test.cxx
@@ -232,6 +232,8 @@ namespace build2
 
           tt.path (path ("driver"));
 
+          const scope& bs (tt.base_scope ());
+
           small_vector<action, 1> acts {perform_update_id};
 
           // Parse and run.
@@ -239,7 +241,7 @@ namespace build2
           parser p (ctx);
           path_name nm ("buildfile");
 
-          script s (p.pre_parse (tt.base_scope (), tt.type (), acts,
+          script s (p.pre_parse (bs, tt.type (), acts,
                                  cin, nm,
                                  11 /* line */,
                                  (m != mode::diag
@@ -251,7 +253,7 @@ namespace build2
           {
           case mode::run:
             {
-              environment e (perform_update_id, tt, s.body_temp_dir);
+              environment e (perform_update_id, tt, bs, s.body_temp_dir);
               print_runner r (print_line, print_iterations);
               p.execute_body (ctx.global_scope, ctx.global_scope, e, s, r);
               break;
@@ -266,7 +268,7 @@ namespace build2
               {
                 assert (s.diag_line);
 
-                environment e (perform_update_id, tt, false /* temp_dir */);
+                environment e (perform_update_id, tt, bs, false /* temp_dir */);
 
                 cout << "diag: " << p.execute_special (ctx.global_scope,
                                                        ctx.global_scope,
diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx
index a557065..2e777b4 100644
--- a/libbuild2/build/script/script.cxx
+++ b/libbuild2/build/script/script.cxx
@@ -28,6 +28,7 @@ namespace build2
       environment::
       environment (action a,
                    const target_type& t,
+                   const scope_type& s,
                    bool temp,
                    const optional<timestamp>& dl)
           : build2::script::environment (
@@ -39,7 +40,8 @@ namespace build2
               redirect (redirect_type::merge, 2),
               redirect (redirect_type::pass)),
             target (t),
-            vars (context, false /* global */),
+            scope (s),
+            vars (context, false /* shared */), // Note: managed.
             var_ts (var_pool.insert (">")),
             var_ps (var_pool.insert ("<")),
             script_deadline (to_deadline (dl, false /* success */))
@@ -233,7 +235,7 @@ namespace build2
         // in parallel). Plus, if there is no such variable, then we cannot
         // possibly find any value.
         //
-        const variable* pvar (context.var_pool.find (n));
+        const variable* pvar (scope.var_pool ().find (n));
 
         if (pvar == nullptr)
           return lookup_type ();
diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx
index f8df204..2c5e6e0 100644
--- a/libbuild2/build/script/script.hxx
+++ b/libbuild2/build/script/script.hxx
@@ -89,10 +89,12 @@ namespace build2
       class environment: public build2::script::environment
       {
       public:
+        using scope_type = build2::scope;
         using target_type = build2::target;
 
         environment (action,
                      const target_type&,
+                     const scope_type&,
                      bool temp_dir,
                      const optional<timestamp>& deadline = nullopt);
 
@@ -113,11 +115,12 @@ namespace build2
         environment& operator= (const environment&) = delete;
 
       public:
-        // Primary target this environment is for.
+        // Primary target this environment is for and its base scope;
         //
         const target_type& target;
+        const scope_type&  scope;
 
-        // Script-local variable pool and map.
+        // Script-private variable pool and map.
         //
         // Note that it may be tempting to reuse the rule-specific variables
         // for this but they should not be modified during execution (i.e.,
diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx
index 4a0c4b1..2d344f1 100644
--- a/libbuild2/cc/common.cxx
+++ b/libbuild2/cc/common.cxx
@@ -164,6 +164,9 @@ namespace build2
       if (self && proc_lib)
         chain->push_back (&l);
 
+      // We only lookup public variables so go straight for the public
+      // variable pool.
+      //
       auto& vp (top_bs.ctx.var_pool);
 
       do // Breakout loop.
@@ -347,7 +350,7 @@ namespace build2
         // Find system search directories corresponding to this library, i.e.,
         // from its project and for its type (C, C++, etc).
         //
-        auto find_sysd = [&top_sysd, t, cc, same, &bs, &sysd, this] ()
+        auto find_sysd = [&top_sysd, &vp, t, cc, same, &bs, &sysd, this] ()
         {
           // Use the search dirs corresponding to this library scope/type.
           //
@@ -356,7 +359,7 @@ namespace build2
           : &cast<dir_paths> (
             bs.root_scope ()->vars[same
                                    ? x_sys_lib_dirs
-                                   : bs.ctx.var_pool[t + ".sys_lib_dirs"]]);
+                                   : vp[t + ".sys_lib_dirs"]]);
         };
 
         auto find_linfo = [top_li, t, cc, &bs, &l, &li] ()
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index fdc1416..fda97a0 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -530,6 +530,8 @@ namespace build2
         if (find (d.ls.begin (), d.ls.end (), &l) != d.ls.end ())
           return false;
 
+        // Note: go straight for the public variable pool.
+        //
         const variable& var (
           com
           ? c_export_poptions
@@ -781,6 +783,8 @@ namespace build2
         //
         if (const scope* rs = l.base_scope ().root_scope ())
         {
+          // Note: go straight for the public variable pool.
+          //
           const variable& var (
             com
             ? c_export_poptions
diff --git a/libbuild2/cc/init.cxx b/libbuild2/cc/init.cxx
index 062e750..b2f35aa 100644
--- a/libbuild2/cc/init.cxx
+++ b/libbuild2/cc/init.cxx
@@ -338,10 +338,11 @@ namespace build2
       //
       if (!cast_false<bool> (rs["bin.config.loaded"]))
       {
-        // Prepare configuration hints. They are only used on the first load
-        // of bin.config so we only populate them on our first load.
+        // Prepare configuration hints (pretend it belongs to root scope).
+        // They are only used on the first load of bin.config so we only
+        // populate them on our first load.
         //
-        variable_map h (rs.ctx);
+        variable_map h (rs);
 
         if (first)
         {
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 8bc073a..94e885f 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -1914,6 +1914,8 @@ namespace build2
           bool u;
           if ((u = pt->is_a<libux> ()) || pt->is_a<liba> ())
           {
+            // Note: go straight for the public variable pool.
+            //
             const variable& var (ctx.var_pool["bin.whole"]); // @@ Cache.
 
             // See the bin module for the lookup semantics discussion. Note
@@ -2364,6 +2366,8 @@ namespace build2
         //
         if (const target* g = exp && l.is_a<libs> () ? l.group : &l)
         {
+          // Note: go straight for the public variable pool.
+          //
           const variable& var (
             com
             ? (exp ? c_export_loptions : c_loptions)
@@ -2751,7 +2755,7 @@ namespace build2
       // those that don't match. Note that we have to do it after updating
       // prerequisites to keep the dependency counts straight.
       //
-      if (const variable* var_fi = ctx.var_pool.find ("for_install"))
+      if (const variable* var_fi = rs.var_pool ().find ("for_install"))
       {
         // Parallel prerequisites/prerequisite_targets loop.
         //
@@ -3007,6 +3011,8 @@ namespace build2
       {
         // For VC we use link.exe directly.
         //
+        // Note: go straight for the public variable pool.
+        //
         const string& cs (
           cast<string> (
             rs[tsys == "win32-msvc"
diff --git a/libbuild2/cc/module.cxx b/libbuild2/cc/module.cxx
index c930d49..27f9d9e 100644
--- a/libbuild2/cc/module.cxx
+++ b/libbuild2/cc/module.cxx
@@ -265,9 +265,9 @@ namespace build2
       //
       if (!cc_loaded)
       {
-        // Prepare configuration hints.
+        // Prepare configuration hints (pretend it belongs to root scope).
         //
-        variable_map h (rs.ctx);
+        variable_map h (rs);
 
         // Note that all these variables have already been registered.
         //
@@ -376,7 +376,9 @@ namespace build2
       //
       if (!cast_false<bool> (rs["cc.core.config.loaded"]))
       {
-        variable_map h (rs.ctx);
+        // Prepare configuration hints (pretend it belongs to root scope).
+        //
+        variable_map h (rs);
 
         if (!xi.bin_pattern.empty ())
           h.assign ("config.bin.pattern") = xi.bin_pattern;
diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx
index 5efab0d..6023b45 100644
--- a/libbuild2/cc/pkgconfig.cxx
+++ b/libbuild2/cc/pkgconfig.cxx
@@ -802,6 +802,11 @@ namespace build2
 
         context& ctx (t.ctx);
 
+        // These should be public (qualified) variables so go straight for
+        // the public variable pool.
+        //
+        auto& vp (ctx.var_pool.rw ()); // Load phase.
+
         optional<uint64_t> ver;
         optional<string> pfx;
 
@@ -865,7 +870,6 @@ namespace build2
                           : name (move (s)));
           }
 
-          auto& vp (ctx.var_pool.rw ()); // Load phase.
           const variable& var (vp.insert (move (vn)));
 
           value& v (t.assign (var));
@@ -1974,6 +1978,8 @@ namespace build2
         //
         if (la)
         {
+          // Note: go straight for the public variable pool.
+          //
           if (cast_false<bool> (l.lookup_original (
                                   ctx.var_pool["bin.whole"],
                                   true /* target_only */).first))
diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx
index 5edb610..c6fea07 100644
--- a/libbuild2/config/operation.cxx
+++ b/libbuild2/config/operation.cxx
@@ -207,6 +207,8 @@ namespace build2
         // saved according to config.config.persist potentially warning if the
         // variable would otherwise be dropped.
         //
+        // Note: go straight for the public variable pool.
+        //
         auto& vp (ctx.var_pool);
 
         for (auto p (rs.vars.lookup_namespace ("config"));
@@ -636,6 +638,8 @@ namespace build2
         }
       }
 
+      // Note: go straight for the public variable pool.
+      //
       value& v (rs.assign (*rs.ctx.var_pool.find ("config.config.environment")));
 
       // Note that setting new config.config.environment value invalidates the
@@ -910,6 +914,8 @@ namespace build2
 
       context& ctx (fwd ? ts[0].as<scope> ().ctx : ts[0].as<target> ().ctx);
 
+      // Note: go straight for the public variable pool.
+      //
       const variable* c_s (ctx.var_pool.find ("config.config.save"));
 
       if (c_s->overrides == nullptr)
@@ -1276,6 +1282,8 @@ namespace build2
       // Add the default config.config.persist value unless there is a custom
       // one (specified as a command line override).
       //
+      // Note: go straight for the public variable pool.
+      //
       const variable& var (*ctx.var_pool.find ("config.config.persist"));
 
       if (!rs[var].defined ())
diff --git a/libbuild2/config/utility.cxx b/libbuild2/config/utility.cxx
index 2ce94ff..314de84 100644
--- a/libbuild2/config/utility.cxx
+++ b/libbuild2/config/utility.cxx
@@ -81,7 +81,9 @@ namespace build2
                       const string& n,
                       initializer_list<const char*> ig)
     {
-      auto& vp (rs.var_pool ());
+      // Note: go straight for the public variable pool.
+      //
+      auto& vp (rs.ctx.var_pool);
 
       // Search all outer scopes for any value in this namespace.
       //
@@ -160,6 +162,8 @@ namespace build2
     pair<variable_origin, lookup>
     origin (const scope& rs, const string& n)
     {
+      // Note: go straight for the public variable pool.
+      //
       const variable* var (rs.ctx.var_pool.find (n));
 
       if (var == nullptr)
diff --git a/libbuild2/config/utility.hxx b/libbuild2/config/utility.hxx
index 72fe520..98d7ec0 100644
--- a/libbuild2/config/utility.hxx
+++ b/libbuild2/config/utility.hxx
@@ -271,6 +271,8 @@ namespace build2
                    const string& var,
                    uint64_t save_flags = 0)
     {
+      // Note: go straight for the public variable pool.
+      //
       return lookup_config (rs, rs.ctx.var_pool[var], save_flags);
     }
 
@@ -280,6 +282,8 @@ namespace build2
                    const string& var,
                    uint64_t save_flags = 0)
     {
+      // Note: go straight for the public variable pool.
+      //
       return lookup_config (new_value, rs, rs.ctx.var_pool[var], save_flags);
     }
 
@@ -367,6 +371,8 @@ namespace build2
                    uint64_t save_flags = 0,
                    bool override = false)
     {
+      // Note: go straight for the public variable pool.
+      //
       return lookup_config (rs,
                             rs.ctx.var_pool[var],
                             std::forward<T> (default_value), // VC14
@@ -383,6 +389,8 @@ namespace build2
                    uint64_t save_flags = 0,
                    bool override = false)
     {
+      // Note: go straight for the public variable pool.
+      //
       return lookup_config (new_value,
                             rs,
                             rs.ctx.var_pool[var],
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index a7df959..e44d79f 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -52,7 +52,10 @@ namespace build2
     variable_override_cache global_override_cache;
     strings global_var_overrides;
 
-    data (context& c): scopes (c), targets (c), var_pool (&c /* global */) {}
+    data (context& c)
+        : scopes (c),
+          targets (c),
+          var_pool (&c /* shared */, nullptr /* outer */) {}
   };
 
   context::
@@ -479,6 +482,7 @@ namespace build2
         unique_ptr<variable> p (
           new variable {
             n + '.' + to_string (i + 1) + '.' + k,
+            &vp     /* owner */,
             nullptr /* aliases   */,
             nullptr /* type      */,
             nullptr /* overrides */,
@@ -708,7 +712,7 @@ namespace build2
 
     auto find_ovar = [this] (const char* n)
     {
-      const variable* v (var_pool.find (n));
+      const variable* v (var_pool.find (n)); // @@ TMP: pub/prv vars
 
       // The operation variable should have prerequisite or target visibility.
       //
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 25a0672..0747350 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -380,7 +380,7 @@ namespace build2
     //
     const scope_map& scopes;
     target_set& targets;
-    const variable_pool& var_pool;
+    const variable_pool& var_pool;           // Public variables.
     const variable_overrides& var_overrides; // Project and relative scope.
     function_map& functions;
 
diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx
index 468f7bd..91d2321 100644
--- a/libbuild2/dist/operation.cxx
+++ b/libbuild2/dist/operation.cxx
@@ -259,7 +259,7 @@ namespace build2
       {
         l5 ([&]{trace << "load dist " << rs;});
 
-        dist_var = ctx.var_pool.find ("dist");
+        dist_var = rs.var_pool ().find ("dist");
 
         // Match a rule for every operation supported by this project. Skip
         // default_id.
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index 3374791..a2fe906 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -582,12 +582,16 @@ namespace build2
       fail << "variable out_root expected as first line in " << f << endf;
   }
 
-  static void
+  // Note: not static due to being a friend of variable_pool.
+  //
+  void
   setup_root_extra (scope& root, optional<bool>& altn)
   {
     assert (altn && root.root_extra == nullptr);
     bool a (*altn);
 
+    context& ctx (root.ctx);
+
     root.root_extra.reset (
       new scope::root_extra_type {
         nullopt /* project */,
@@ -607,6 +611,7 @@ namespace build2
         a ? alt_export_file      : std_export_file,
         a ? alt_src_root_file    : std_src_root_file,
         a ? alt_out_root_file    : std_out_root_file,
+        {&ctx, &ctx.var_pool.rw (root)}, /* var_pool */
         {}, /* meta_operations */
         {}, /* operations */
         {}, /* modules */
@@ -3197,13 +3202,15 @@ namespace build2
 
         const string& pfx (ns[1].value);
 
-        auto& vp (ctx.var_pool.rw ()); // Load phase.
-
         // See if we have the stable program name in the <var-prefix>.name
         // variable. If its missing, set it to the metadata key (i.e., target
         // name as imported) by default.
         //
         {
+          // Note: go straight for the public variable pool.
+          //
+          auto& vp (ctx.var_pool.rw ()); // Load phase.
+
           value& nv (t.assign (vp.insert (pfx + ".name")));
           if (!nv)
             nv = *meta;
diff --git a/libbuild2/functions-builtin.cxx b/libbuild2/functions-builtin.cxx
index 9785f3a..f85c342 100644
--- a/libbuild2/functions-builtin.cxx
+++ b/libbuild2/functions-builtin.cxx
@@ -60,7 +60,7 @@ namespace build2
         fail << "visibility() called out of scope" << endf;
 
       const variable* var (
-        s->ctx.var_pool.find (convert<string> (move (name))));
+        s->var_pool ().find (convert<string> (move (name))));
 
       return (var != nullptr
               ? optional<string> (to_string (var->visibility))
diff --git a/libbuild2/install/utility.cxx b/libbuild2/install/utility.cxx
index 17b1365..c2a581e 100644
--- a/libbuild2/install/utility.cxx
+++ b/libbuild2/install/utility.cxx
@@ -12,6 +12,8 @@ namespace build2
     {
       context& ctx (t.ctx);
 
+      // Note: go straight for the public variable pool.
+      //
       const variable& var (*ctx.var_pool.find ("config.install.scope"));
 
       if (const string* s = cast_null<string> (ctx.global_scope[var]))
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 1cffb85..ec33d6e 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -4952,11 +4952,10 @@ namespace build2
     //         attributes).
 
     if (type || vis || ovr)
-      ctx->var_pool.update (const_cast<variable&> (var),
-                            type,
-                            vis ? &*vis : nullptr,
-                            ovr ? &*ovr : nullptr);
-
+      var.owner->update (const_cast<variable&> (var),
+                         type,
+                         vis ? &*vis : nullptr,
+                         ovr ? &*ovr : nullptr);
   }
 
   void parser::
diff --git a/libbuild2/prerequisite.cxx b/libbuild2/prerequisite.cxx
index cc41708..7e14c76 100644
--- a/libbuild2/prerequisite.cxx
+++ b/libbuild2/prerequisite.cxx
@@ -63,7 +63,7 @@ namespace build2
         ext (to_ext (t.ext ())),
         scope (t.base_scope ()),
         target (&t),
-        vars (t.ctx, false /* global */)
+        vars (*this, false /* shared */)
   {
   }
 
diff --git a/libbuild2/prerequisite.hxx b/libbuild2/prerequisite.hxx
index 3b64eae..33efed0 100644
--- a/libbuild2/prerequisite.hxx
+++ b/libbuild2/prerequisite.hxx
@@ -93,7 +93,7 @@ namespace build2
           name (move (n)),
           ext (move (e)),
           scope (s),
-          vars (s.ctx, false /* global */) {}
+          vars (*this, false /* shared */) {}
 
     // Make a prerequisite from a target.
     //
@@ -147,7 +147,7 @@ namespace build2
           ext (move (x.ext)),
           scope (x.scope),
           target (x.target.load (memory_order_relaxed)),
-          vars (move (x.vars)) {}
+          vars (move (x.vars), *this, false /* shared */) {}
 
     prerequisite (const prerequisite& x, memory_order o = memory_order_consume)
         : proj (x.proj),
@@ -158,7 +158,7 @@ namespace build2
           ext (x.ext),
           scope (x.scope),
           target (x.target.load (o)),
-          vars (x.vars) {}
+          vars (x.vars, *this, false /* shared */) {}
   };
 
   inline ostream&
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index 74f212e..5811bd1 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -33,7 +33,7 @@ namespace build2
   //
   scope::
   scope (context& c, bool shared)
-      : ctx (c), vars (c, shared), target_vars (c, shared)
+      : ctx (c), vars (*this, shared), target_vars (c, shared)
   {
   }
 
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 2105512..598d7e8 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -136,7 +136,7 @@ namespace build2
     lookup_type
     operator[] (const string& name) const
     {
-      const variable* var (ctx.var_pool.find (name));
+      const variable* var (var_pool ().find (name));
       return var != nullptr ? operator[] (*var) : lookup_type ();
     }
 
@@ -508,6 +508,10 @@ namespace build2
       const path&     src_root_file;  // build[2]/bootstrap/src-root.build[2]
       const path&     out_root_file;  // build[2]/bootstrap/src-root.build[2]
 
+      // Project-private variable pool.
+      //
+      variable_pool var_pool;
+
       // Meta/operations supported by this project.
       //
       build2::meta_operations meta_operations;
@@ -589,12 +593,21 @@ namespace build2
       return const_cast<scope&> (*this);
     }
 
+    // @@ TODO: find root scope and return its var_pool falling back to
+    //          ctx.var_pool if no root scope.
+    //
     variable_pool&
     var_pool ()
     {
       return ctx.var_pool.rw (*this);
     }
 
+    const variable_pool&
+    var_pool () const
+    {
+      return ctx.var_pool;
+    }
+
   private:
     friend class parser;
     friend class scope_map;
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index 768fc82..14b6496 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -755,6 +755,8 @@ namespace build2
         t->decl = decl;
         t->state.inner.target_ = t;
         t->state.outer.target_ = t;
+        t->state.inner.vars.target_ = t;
+        t->state.outer.vars.target_ = t;
 
         if (ctx.phase != run_phase::load && !need_lock)
           ul.unlock ();
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index f745f59..6387b8f 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -619,8 +619,9 @@ namespace build2
     lookup_type
     operator[] (const string& name) const
     {
-      const variable* var (ctx.var_pool.find (name));
-      return var != nullptr ? operator[] (*var) : lookup_type ();
+      const scope& bs (base_scope ());
+      const variable* var (bs.var_pool ().find (name));
+      return var != nullptr ? lookup (*var, &bs).first : lookup_type ();
     }
 
     // As above but also return the depth at which the value is found. The
@@ -632,12 +633,14 @@ namespace build2
     // earlier. If no value is found, then the depth is set to ~0.
     //
     pair<lookup_type, size_t>
-    lookup (const variable& var) const
+    lookup (const variable& var, const scope* bs = nullptr) const
     {
-      auto p (lookup_original (var));
+      auto p (lookup_original (var, false, bs));
       return var.overrides == nullptr
         ? p
-        : base_scope ().lookup_override (var, move (p), true);
+        : (bs != nullptr
+           ? *bs
+           : base_scope ()).lookup_override (var, move (p), true);
     }
 
     // If target_only is true, then only look in target and its target group
@@ -781,13 +784,6 @@ namespace build2
         return operator[] (*var);
       }
 
-      lookup_type
-      operator[] (const string& name) const
-      {
-        const variable* var (target_->ctx.var_pool.find (name));
-        return var != nullptr ? operator[] (*var) : lookup_type ();
-      }
-
       // As above but also return the depth at which the value is found. The
       // depth is calculated by adding 1 for each test performed. So a value
       // that is from the rule will have depth 1. That from the target - 2,
@@ -816,14 +812,18 @@ namespace build2
       value&
       assign (const variable* var) {return vars.assign (var);} // For cached.
 
+      // Implementation details.
+      //
     public:
       explicit
-      opstate (context& c): vars (c, false /* global */) {}
+      opstate (context& c): vars (variable_map::owner::target, &c) {}
 
     private:
       friend class target_set;
 
-      const target* target_ = nullptr; // Back-pointer, set by target_set.
+      // Back-pointer, set by target_set along with vars.target_.
+      //
+      const target* target_ = nullptr;
     };
 
     action_state<opstate> state;
@@ -1163,7 +1163,7 @@ namespace build2
     target (context& c, dir_path d, dir_path o, string n)
         : ctx (c),
           dir (move (d)), out (move (o)), name (move (n)),
-          vars (c, false /* global */),
+          vars (*this, false /* shared */),
           state (c)
     {
       dynamic_type = &static_type;
diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx
index be86117..e10afec 100644
--- a/libbuild2/test/script/script.cxx
+++ b/libbuild2/test/script/script.cxx
@@ -30,7 +30,7 @@ namespace build2
       scope_base::
       scope_base (script& s)
           : root (s),
-            vars (s.test_target.ctx, false /* global */)
+            vars (s.test_target.ctx, false /* shared */) // Note: managed.
       {
         vars.assign (root.wd_var) = dir_path ();
       }
@@ -355,7 +355,7 @@ namespace build2
         // in parallel). Plus, if there is no such variable, then we cannot
         // possibly find any value.
         //
-        const variable* pvar (context.var_pool.find (n));
+        const variable* pvar (root.target_scope.var_pool ().find (n));
 
         if (pvar == nullptr)
           return lookup_type ();
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index 9e98bb6..5c32d89 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -1740,6 +1740,7 @@ namespace build2
         variable {
           move (n),
           nullptr,
+          nullptr,
           pt,
           nullptr,
           pv != nullptr ? *pv : variable_visibility::project}));
@@ -1747,7 +1748,10 @@ namespace build2
     variable& var (r.first->second);
 
     if (r.second)
+    {
+      var.owner = this;
       var.aliases = &var;
+    }
     else // Note: overridden variable will always exist.
     {
       // This is tricky: if the pattern does not require a match, then we
@@ -1868,7 +1872,66 @@ namespace build2
 
   // variable_map
   //
-  const variable_map empty_variable_map (nullptr /* context */);
+  const variable_map empty_variable_map (variable_map::owner::empty);
+
+  // Need scope/target definition thus not inline.
+  //
+  variable_map::
+  variable_map (const scope& s, bool shared)
+    : shared_ (shared), owner_ (owner::scope), scope_ (&s), ctx (&s.ctx)
+  {
+  }
+
+  variable_map::
+  variable_map (const target& t, bool shared)
+      : shared_ (shared), owner_ (owner::target), target_ (&t), ctx (&t.ctx)
+  {
+  }
+
+  variable_map::
+  variable_map (const prerequisite& p, bool shared)
+    : shared_ (shared),
+      owner_ (owner::prereq), prereq_ (&p),
+      ctx (&p.scope.ctx)
+  {
+  }
+
+  variable_map::
+  variable_map (variable_map&& v, const prerequisite& p, bool shared)
+      : shared_ (shared),
+        owner_ (owner::scope), prereq_ (&p),
+        ctx (&p.scope.ctx),
+        m_ (move (v.m_))
+  {
+  }
+
+  variable_map::
+  variable_map (const variable_map& v, const prerequisite& p, bool shared)
+      : shared_ (shared),
+        owner_ (owner::scope), prereq_ (&p),
+        ctx (&p.scope.ctx),
+        m_ (v.m_)
+  {
+  }
+
+  lookup variable_map::
+  lookup (const string& name) const
+  {
+    lookup_type r;
+
+    const scope* bs (owner_ == owner::scope  ? scope_                  :
+                     owner_ == owner::target ? &target_->base_scope () :
+                     owner_ == owner::prereq ? &prereq_->scope         :
+                     nullptr);
+
+    if (const variable* var = bs->var_pool ().find (name))
+    {
+      auto p (lookup (*var));
+      r = lookup_type (p.first, &p.second, this);
+    }
+
+    return r;
+  }
 
   auto variable_map::
   lookup (const variable& var, bool typed, bool aliased) const ->
@@ -1919,6 +1982,19 @@ namespace build2
     return pair<value_data*, const variable&> (r, p.second);
   }
 
+  value& variable_map::
+  assign (const string& name)
+  {
+    assert (owner_ != owner::context);
+
+    const scope* bs (owner_ == owner::scope  ? scope_                  :
+                     owner_ == owner::target ? &target_->base_scope () :
+                     owner_ == owner::prereq ? &prereq_->scope         :
+                     nullptr);
+
+    return insert (bs->var_pool ()[name]).first;
+  }
+
   pair<value&, bool> variable_map::
   insert (const variable& var, bool typed, bool reset_extra)
   {
@@ -1945,6 +2021,21 @@ namespace build2
     return pair<value&, bool> (r, p.second);
   }
 
+  auto variable_map::
+  find (const string& name) const -> const_iterator
+  {
+    assert (owner_ != owner::context);
+
+    const scope* bs (owner_ == owner::scope  ? scope_                  :
+                     owner_ == owner::target ? &target_->base_scope () :
+                     owner_ == owner::prereq ? &prereq_->scope         :
+                     nullptr);
+
+
+    const variable* var (bs->var_pool ().find (name));
+    return var != nullptr ? find (*var) : end ();
+  }
+
   bool variable_map::
   erase (const variable& var)
   {
@@ -1966,6 +2057,9 @@ namespace build2
   variable_map& variable_pattern_map::
   insert (pattern_type type, string&& text)
   {
+    // Note that this variable map is special and we use context as its owner
+    // (see variable_map for details).
+    //
     auto r (map_.emplace (pattern {type, false, move (text), {}},
                           variable_map (ctx, shared_)));
 
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index 4f87818..9f1eee6 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -145,13 +145,27 @@ namespace build2
     return o << to_string (v);
   }
 
-  // variable
+  // A variable.
   //
-  // The two variables are considered the same if they have the same name.
+  // A variable can be public, project-private, or script-private, which
+  // corresponds to the variable pool it belongs to. The two variables from
+  // the same pool are considered the same if they have the same name. The
+  // variable access (public/private) rules are:
+  //
+  // - Qualified variable are by default public while unqualified -- private.
+  //
+  // - Private must have project or lesser visibility and not be overridable.
+  //
+  // - An unqualified public variable can only be pre-entered during the
+  //   context construction (to make sure it is not entered as private).
+  //
+  // - There is no scope-private variables in our model due to side-loading,
+  //   target type/pattern-specific append, etc.
   //
   // Variables can be aliases of each other in which case they form a circular
   // linked list (the aliases pointer for variable without any aliases points
-  // to the variable itself).
+  // to the variable itself). This mechanism should only be used for variables
+  // of the same access (normally public).
   //
   // If the variable is overridden on the command line, then override is the
   // linked list of the special override variables. Their names are derived
@@ -198,6 +212,7 @@ namespace build2
   struct variable
   {
     string name;
+    const variable_pool* owner;
     const variable* aliases;               // Circular linked list.
     const value_type* type;                // If NULL, then not (yet) typed.
     unique_ptr<const variable> overrides;
@@ -467,7 +482,6 @@ namespace build2
   template <typename T> T cast_true (const value&);
   template <typename T> T cast_true (const lookup&);
 
-
   // Assign value type to the value. The variable is optional and is only used
   // for diagnostics.
   //
@@ -492,7 +506,7 @@ namespace build2
   vector_view<name>
   reverse (value&, names& storage);
 
-  // Variable lookup result, AKA, binding of a name to a value.
+  // Variable lookup result, AKA, binding of a variable to a value.
   //
   // A variable can be undefined, NULL, or contain a (potentially empty)
   // value.
@@ -1404,7 +1418,17 @@ namespace build2
     }
 
   public:
-    variable_pool (): variable_pool (nullptr) {}
+    // Create a private pool.
+    //
+    explicit
+    variable_pool (variable_pool* outer = nullptr)
+        : variable_pool (nullptr /* shared */, outer) {}
+
+    variable_pool (variable_pool&&) = delete;
+    variable_pool& operator= (variable_pool&&) = delete;
+
+    variable_pool (const variable_pool&) = delete;
+    variable_pool& operator= (const variable_pool&) = delete;
 
     // RW access (only for shared pools).
     //
@@ -1497,11 +1521,13 @@ namespace build2
     //
   private:
     friend class context;
+    friend void setup_root_extra (scope&, optional<bool>&);
 
-    explicit
-    variable_pool (context* shared): shared_ (shared) {}
+    variable_pool (context* shared, variable_pool* outer)
+        : shared_ (shared), outer_ (outer) {}
 
-    context* shared_;
+    context*      shared_;
+    variable_pool* outer_;
   };
 }
 
@@ -1587,8 +1613,13 @@ namespace build2
     lookup_type
     operator[] (const variable& var) const
     {
-      auto p (lookup (var));
-      return lookup_type (p.first, &p.second, this);
+      lookup_type r;
+      if (!empty ())
+      {
+        auto p (lookup (var));
+        r = lookup_type (p.first, &p.second, this);
+      }
+      return r;
     }
 
     lookup_type
@@ -1601,12 +1632,17 @@ namespace build2
     lookup_type
     operator[] (const string& name) const
     {
-      const variable* var (ctx != nullptr
-                           ? ctx->var_pool.find (name)
-                           : nullptr);
-      return var != nullptr ? operator[] (*var) : lookup_type ();
+      assert (owner_ != owner::context);
+
+      lookup_type r;
+      if (!empty ())
+        r = lookup (name);
+      return r;
     }
 
+    lookup_type
+    lookup (const string& name) const;
+
     // If typed is false, leave the value untyped even if the variable is. If
     // aliased is false, then don't consider aliases (used by the variable
     // override machinery where the aliases chain is repurrposed for something
@@ -1633,7 +1669,9 @@ namespace build2
       // insert anything.
       //
       return lookup_namespace (variable {
-          move (ns), nullptr, nullptr, nullptr, variable_visibility::project});
+          move (ns),
+          nullptr, nullptr, nullptr, nullptr,
+          variable_visibility::project});
     }
 
     // Convert a lookup pointing to a value belonging to this variable map
@@ -1662,10 +1700,10 @@ namespace build2
       return assign (*var);
     }
 
-    // Note that the variable is expected to have already been registered.
+    // Note that the variable is expected to have already been inserted.
     //
     value&
-    assign (const string& name) {return insert (ctx->var_pool[name]).first;}
+    assign (const string& name);
 
     // As above but also return an indication of whether the new value (which
     // will be NULL) was actually inserted. Similar to find(), if typed is
@@ -1684,13 +1722,7 @@ namespace build2
     }
 
     const_iterator
-    find (const string& name) const
-    {
-      const variable* var (ctx != nullptr
-                           ? ctx->var_pool.find (name)
-                           : nullptr);
-      return var != nullptr ? find (*var) : end ();
-    }
+    find (const string& name) const;
 
     bool
     erase (const variable&);
@@ -1712,21 +1744,55 @@ namespace build2
 
   public:
     // Shared should be true if this map is part of the shared build state
-    // (e.g., scopes, etc) and thus should only be modified during the load
-    // phase.
+    // (e.g., scopes) and thus should only be modified during the load phase.
     //
     explicit
-    variable_map (context& c, bool shared = false)
-      : ctx (&c), shared_ (shared) {}
+    variable_map (const scope& owner, bool shared = false);
+
+    explicit
+    variable_map (const target& owner, bool shared = false);
+
+    explicit
+    variable_map (const prerequisite& owner, bool shared = false);
+
+    variable_map (variable_map&&, const prerequisite&, bool shared = false);
+    variable_map (const variable_map&, const prerequisite&, bool shared = false);
+
+    variable_map&
+    operator= (variable_map&& v) {m_ = move (v.m_); return *this;}
+
+    variable_map&
+    operator= (const variable_map& v) {m_ = v.m_; return *this;}
+
+    // The context owner is for special "managed" variable maps. Note that
+    // such maps cannot lookup/insert variable names specified as strings.
+    //
+    variable_map (context& c, bool shared)
+      : shared_ (shared), owner_ (owner::context), ctx (&c) {}
+
+    variable_map (variable_map&& v)
+      : shared_ (v.shared_), owner_ (v.owner_), ctx (v.ctx), m_ (move (v.m_))
+    {
+      assert (owner_ == owner::context);
+    }
+
+    variable_map (const variable_map& v)
+      : shared_ (v.shared_), owner_ (v.owner_), ctx (v.ctx), m_ (v.m_)
+    {
+      assert (v.owner_ == owner::context);
+    }
 
     void
     clear () {m_.clear ();}
 
-    // Implementation details (only used for empty_variable_map).
+    // Implementation details.
     //
   public:
+    enum class owner {empty, context, scope, target, prereq};
+
     explicit
-    variable_map (context* c): ctx (c) {}
+    variable_map (owner o, context* c = nullptr, bool shared = false)
+      : shared_ (shared), owner_ (o), ctx (c) {}
 
   private:
     friend class variable_type_map;
@@ -1735,9 +1801,18 @@ namespace build2
     typify (const value_data&, const variable&) const;
 
   private:
+    friend class target_set;
+
+    bool shared_;
+    owner owner_;
+    union
+    {
+      const scope*        scope_;
+      const target*       target_;
+      const prerequisite* prereq_;
+    };
     context* ctx;
     map_type m_;
-    bool shared_;
   };
 
   LIBBUILD2_SYMEXPORT extern const variable_map empty_variable_map;
-- 
cgit v1.1