Switch to public/private variables model

Now unqualified variables are project-private and can be typified.

18 files changed, 275 insertions, 128 deletions

diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index 4cbdecb..f46a3eb 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -464,6 +464,18 @@ namespace build2
 
       string n (t.value, c == '!' || c == '%' || c == '/' ? 1 : 0);
 
+      // Make sure it is qualified.
+      //
+      // We can support overridable public unqualified variables (which must
+      // all be pre-entered by the end of this constructor) but we will need
+      // to detect their names here in an ad hoc manner (we cannot enter them
+      // before this logic because of the "untyped override" requirement).
+      //
+      // Note: issue the same diagnostics as in variable_pool::update().
+      //
+      if (n.find ('.') == string::npos)
+        fail << "variable " << n << " cannot be overridden";
+
       if (c == '!' && dir)
         fail << "scope-qualified global override of variable " << n;
 
@@ -620,6 +632,10 @@ namespace build2
       r.insert<mtime_target> (perform_update_id, "build.file", file_rule::instance);
       r.insert<mtime_target> (perform_clean_id,  "build.file", file_rule::instance);
     }
+
+    // End of initialization.
+    //
+    load_generation = 1;
   }
 
   context::
@@ -716,29 +732,6 @@ namespace build2
     current_mode = inner_oif.mode;
     current_diag_noise = diag_noise;
 
-    auto find_ovar = [this] (const char* n)
-    {
-      const variable* v (var_pool.find (n)); // @@ TMP: pub/prv vars
-
-      // The operation variable should have prerequisite or target visibility.
-      //
-      assert (v != nullptr &&
-              (v->visibility == variable_visibility::prereq ||
-               v->visibility == variable_visibility::target));
-
-      return v;
-    };
-
-    current_inner_ovar =
-      inner_oif.var_name != nullptr
-      ? find_ovar (inner_oif.var_name)
-      : nullptr;
-
-    current_outer_ovar =
-      outer_oif != nullptr && outer_oif->var_name != nullptr
-      ? find_ovar (outer_oif->var_name)
-      : nullptr;
-
     // Reset counters (serial execution).
     //
     dependency_count.store (0, memory_order_relaxed);
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 9cb4fb4..b426ea0 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -253,7 +253,7 @@ namespace build2
     // change already existing nodes or invalidate any references to such (the
     // idea here is that one should be able to load additional buildfiles as
     // long as they don't interfere with the existing build state). The
-    // "islands" are identified by the load_generation number (0 for the
+    // "islands" are identified by the load_generation number (1 for the
     // initial/serial load). It is incremented in case of a phase switch and
     // can be stored in various "nodes" to verify modifications are only done
     // "within the islands". Another example of invalidation would be
@@ -263,6 +263,10 @@ namespace build2
     // for example, we may have queried a variable which in the new hierarchy
     // would "see" a new value from the newly inserted scope.
     //
+    // The special load_generation value 0 indicates initialization before
+    // anything has been loaded. Currently, it is changed to 1 at the end
+    // of the context constructor.
+    //
     run_phase phase = run_phase::load;
     size_t load_generation = 0;
 
@@ -309,11 +313,6 @@ namespace build2
     const operation_info* current_inner_oif;
     const operation_info* current_outer_oif;
 
-    // Current operation-specific variables.
-    //
-    const variable* current_inner_ovar;
-    const variable* current_outer_ovar;
-
     action
     current_action () const
     {
@@ -549,7 +548,7 @@ namespace build2
     // operations. The initial idea was to extend this value to allow
     // specifying the operation (e.g., clean@false). However, later we
     // realized that we could reuse the "operation-specific variables"
-    // (update, clean, install, test; see context::current_ovar) with a more
+    // (update, clean, install, test; see project_operation_info) with a more
     // natural-looking and composable result. Plus, this allows for
     // operation-specific "modifiers", for example, "unmatch" and "update
     // during match" logic for update (see var_update for details) or
diff --git a/libbuild2/dump.cxx b/libbuild2/dump.cxx
index 14078da..b8ec74e 100644
--- a/libbuild2/dump.cxx
+++ b/libbuild2/dump.cxx
@@ -208,7 +208,7 @@ namespace build2
 
     for (action a: r.actions)
       os << ' ' << re.meta_operations[a.meta_operation ()]->name <<
-        '(' << re.operations[a.operation ()]->name << ')';
+        '(' << re.operations[a.operation ()].info->name << ')';
 
     os << endl;
     r.dump_text (os, ind);
@@ -289,7 +289,7 @@ namespace build2
         os << "rule_hint=";
 
         if (v.operation != default_id)
-          os << s.root_scope ()->root_extra->operations[v.operation]->name
+          os << s.root_scope ()->root_extra->operations[v.operation].info->name
              << '@';
 
         os << v.hint;
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index a3962d7..cb57f70 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -582,7 +582,7 @@ namespace build2
       fail << "variable out_root expected as first line in " << f << endf;
   }
 
-  // Note: not static due to being a friend of variable_pool.
+  // Note: not static due to being a friend of scope and variable_pool.
   //
   void
   setup_root_extra (scope& root, optional<bool>& altn)
@@ -620,6 +620,8 @@ namespace build2
         {}, /* environment */
         ""} /* environment_checksum */);
 
+    root.var_pool_ = &root.root_extra->var_pool;
+
     // Enter built-in meta-operation and operation names. Loading of
     // modules (via the src bootstrap; see below) can result in
     // additional meta/operations being added.
@@ -628,9 +630,9 @@ namespace build2
     root.insert_meta_operation (perform_id, mo_perform);
     root.insert_meta_operation (info_id,    mo_info);
 
-    root.insert_operation (default_id, op_default);
-    root.insert_operation (update_id,  op_update);
-    root.insert_operation (clean_id,   op_clean);
+    root.insert_operation (default_id, op_default, nullptr);
+    root.insert_operation (update_id,  op_update,  ctx.var_update);
+    root.insert_operation (clean_id,   op_clean,   ctx.var_clean);
   }
 
   value&
diff --git a/libbuild2/function.test.cxx b/libbuild2/function.test.cxx
index f711d2f..7ce7ad3 100644
--- a/libbuild2/function.test.cxx
+++ b/libbuild2/function.test.cxx
@@ -124,7 +124,9 @@ namespace build2
 
     try
     {
-      scope& s (ctx.global_scope.rw ());
+      // Use temp scope for the private variable pool.
+      //
+      temp_scope s (ctx.global_scope.rw ());
 
       parser p (ctx);
       p.parse_buildfile (cin, path_name ("buildfile"), &s, s);
diff --git a/libbuild2/install/init.cxx b/libbuild2/install/init.cxx
index ef9de05..35c2d13 100644
--- a/libbuild2/install/init.cxx
+++ b/libbuild2/install/init.cxx
@@ -250,6 +250,21 @@ namespace build2
 
       context& ctx (rs.ctx);
 
+      // Enter module variables (note that init() below enters some more).
+      //
+      auto& vp (rs.var_pool ());
+
+      // The install variable is a path, not dir_path, since it can be used
+      // to both specify the target directory (to install with the same file
+      // name) or target file (to install with a different name). And the
+      // way we distinguish between the two is via the presence/absence of
+      // the trailing directory separator.
+      //
+      // Plus it can have the special true/false values when acting as a
+      // operation variable.
+      //
+      auto& ovar (vp.insert<path> ("install", variable_visibility::target));
+
       // Register the install function family if this is the first instance of
       // the install modules.
       //
@@ -258,9 +273,9 @@ namespace build2
 
       // Register our operations.
       //
-      rs.insert_operation (install_id,            op_install);
-      rs.insert_operation (uninstall_id,          op_uninstall);
-      rs.insert_operation (update_for_install_id, op_update_for_install);
+      rs.insert_operation (install_id,            op_install,            &ovar);
+      rs.insert_operation (uninstall_id,          op_uninstall,          &ovar);
+      rs.insert_operation (update_for_install_id, op_update_for_install, &ovar);
     }
 
     static const path cmd ("install");
@@ -317,13 +332,6 @@ namespace build2
       // Note that the set_dir() calls below enter some more.
       //
       {
-        // The install variable is a path, not dir_path, since it can be used
-        // to both specify the target directory (to install with the same file
-        // name) or target file (to install with a different name). And the
-        // way we distinguish between the two is via the presence/absence of
-        // the trailing directory separator.
-        //
-        vp.insert<path>   ("install",     variable_visibility::target);
         vp.insert<bool>   ("for_install", variable_visibility::prereq);
         vp.insert<string> ("install.mode");
         vp.insert<bool>   ("install.subdirs");
diff --git a/libbuild2/install/operation.cxx b/libbuild2/install/operation.cxx
index 8c59ac1..52e8c94 100644
--- a/libbuild2/install/operation.cxx
+++ b/libbuild2/install/operation.cxx
@@ -37,7 +37,6 @@ namespace build2
       0,
       "install",
       "install",
-      "install",
       "installing",
       "installed",
       "has nothing to install", // We cannot "be installed".
@@ -62,7 +61,6 @@ namespace build2
       uninstall_id,
       0,
       "uninstall",
-      "install",
       "uninstall",
       "uninstalling",
       "uninstalled",
@@ -81,7 +79,6 @@ namespace build2
       update_id, // Note: not update_for_install_id.
       install_id,
       op_update.name,
-      op_update.var_name,
       op_update.name_do,
       op_update.name_doing,
       op_update.name_did,
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index 6458a54..756d5a8 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -40,12 +40,17 @@ namespace build2
     // Note that the below rules are called for both install and
     // update-for-install.
     //
+    // @@ TODO: we clearly need a module class.
+    //
     static inline const variable&
-    var_install (context& ctx)
+    var_install (const scope& rs)
     {
-      return ctx.current_outer_ovar != nullptr
-        ? *ctx.current_outer_ovar
-        : *ctx.current_inner_ovar;
+      context& ctx (rs.ctx);
+
+      return *rs.root_extra->operations[
+        (ctx.current_outer_oif != nullptr
+         ? ctx.current_outer_oif
+         : ctx.current_inner_oif)->id].ovar;
     }
 
     // alias_rule
@@ -85,8 +90,6 @@ namespace build2
     {
       tracer trace ("install::alias_rule::apply");
 
-      context& ctx (t.ctx);
-
       // Pass-through to our installable prerequisites.
       //
       // @@ Shouldn't we do match in parallel (here and below)?
@@ -138,7 +141,7 @@ namespace build2
         //
         // Note: not the same as lookup_install() above.
         //
-        auto l ((*pt)[var_install (ctx)]);
+        auto l ((*pt)[var_install (*p.scope.root_scope ())]);
         if (l && cast<path> (l).string () == "false")
         {
           l5 ([&]{trace << "ignoring " << *pt << " (not installable)";});
@@ -223,8 +226,10 @@ namespace build2
       //
       if (p.is_a<exe> ())
       {
+        const scope& rs (*p.scope.root_scope ());
+
         if (p.vars.empty () ||
-            cast_empty<path> (p.vars[var_install (t.ctx)]).string () != "true")
+            cast_empty<path> (p.vars[var_install (rs)]).string () != "true")
           return nullptr;
       }
 
@@ -237,8 +242,6 @@ namespace build2
     {
       tracer trace ("install::group_rule::apply");
 
-      context& ctx (t.ctx);
-
       // Resolve group members.
       //
       // Remember that we are called twice: first during update for install
@@ -253,8 +256,10 @@ namespace build2
                      ? resolve_members (a, t)
                      : t.group_members (a));
 
-      if (gv.members != nullptr)
+      if (gv.members != nullptr && gv.count != 0)
       {
+        const scope& rs (t.root_scope ());
+
         auto& pts (t.prerequisite_targets[a]);
         for (size_t i (0); i != gv.count; ++i)
         {
@@ -277,7 +282,7 @@ namespace build2
           //
           // Note: not the same as lookup_install() above.
           //
-          auto l ((*mt)[var_install (ctx)]);
+          auto l ((*mt)[var_install (rs)]);
           if (l && cast<path> (l).string () == "false")
           {
             l5 ([&]{trace << "ignoring " << *mt << " (not installable)";});
@@ -324,13 +329,15 @@ namespace build2
       //
       if (p.is_a<exe> ())
       {
+        const scope& rs (*p.scope.root_scope ());
+
         // Note that while include() checks for install=false, here we need to
         // check for explicit install=true. We could have re-used the lookup
         // performed by include(), but then we would have had to drag it
         // through and also diagnose any invalid values.
         //
         if (p.vars.empty () ||
-            cast_empty<path> (p.vars[var_install (t.ctx)]).string () != "true")
+            cast_empty<path> (p.vars[var_install (rs)]).string () != "true")
           return nullptr;
       }
 
@@ -350,8 +357,6 @@ namespace build2
     {
       tracer trace ("install::file_rule::apply");
 
-      context& ctx (t.ctx);
-
       // Note that we are called both as the outer part during the update-for-
       // un/install pre-operation and as the inner part during the un/install
       // operation itself.
@@ -419,7 +424,7 @@ namespace build2
         //
         // Note: not the same as lookup_install() above.
         //
-        auto l ((*pt)[var_install (ctx)]);
+        auto l ((*pt)[var_install (*p.scope.root_scope ())]);
         if (l && cast<path> (l).string () == "false")
         {
           l5 ([&]{trace << "ignoring " << *pt << " (not installable)";});
@@ -974,8 +979,6 @@ namespace build2
     target_state file_rule::
     perform_install (action a, const target& xt) const
     {
-      context& ctx (xt.ctx);
-
       const file& t (xt.as<file> ());
       const path& tp (t.path ());
 
@@ -1073,7 +1076,7 @@ namespace build2
       //
       if (!tp.empty ())
       {
-        install_target (t, cast<path> (t[var_install (ctx)]), 1);
+        install_target (t, cast<path> (t[var_install (rs)]), 1);
         r |= target_state::changed;
       }
 
@@ -1277,8 +1280,6 @@ namespace build2
     target_state file_rule::
     perform_uninstall (action a, const target& xt) const
     {
-      context& ctx (xt.ctx);
-
       const file& t (xt.as<file> ());
       const path& tp (t.path ());
 
@@ -1342,7 +1343,7 @@ namespace build2
       target_state r (target_state::unchanged);
 
       if (!tp.empty ())
-        r |= uninstall_target (t, cast<path> (t[var_install (ctx)]), 1);
+        r |= uninstall_target (t, cast<path> (t[var_install (rs)]), 1);
 
       // Then installable ad hoc group members, if any. To be anally precise,
       // we would have to do it in reverse, but that's not easy (it's a
diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx
index 412e07f..fc569a9 100644
--- a/libbuild2/operation.cxx
+++ b/libbuild2/operation.cxx
@@ -978,7 +978,6 @@ namespace build2
     default_id,
     0,
     "<default>",
-    nullptr,
     "",
     "",
     "",
@@ -1006,7 +1005,6 @@ namespace build2
     0,
     "update",
     "update",
-    "update",
     "updating",
     "updated",
     "is up to date",
@@ -1023,7 +1021,6 @@ namespace build2
     0,
     "clean",
     "clean",
-    "clean",
     "cleaning",
     "cleaned",
     "is clean",
diff --git a/libbuild2/operation.hxx b/libbuild2/operation.hxx
index ce3cd79..4eb2658 100644
--- a/libbuild2/operation.hxx
+++ b/libbuild2/operation.hxx
@@ -196,7 +196,6 @@ namespace build2
     const operation_id id;
     const operation_id outer_id;
     const char* name;
-    const char* var_name; // Operation variable or NULL if not used.
 
     // Name derivatives for diagnostics. Note that unlike meta-operations,
     // these can only be empty for the default operation (id 1), And
@@ -308,35 +307,36 @@ namespace build2
 
   using operation_table = butl::string_table<operation_id>;
 
-  // These are "sparse" in the sense that we may have "holes" that
-  // are represented as NULL pointers. Also, lookup out of bounds
-  // is treated as a hole.
+  // This is a "sparse" vector in the sense that we may have "holes" that are
+  // represented as default-initialized empty instances (for example, NULL if
+  // T is a pointer). Also, lookup out of bounds is treated as a hole.
   //
   template <typename T, size_t N>
   struct sparse_vector
   {
-    using base_type = small_vector<T*, N>;
+    using base_type = small_vector<T, N>;
     using size_type = typename base_type::size_type;
 
     void
-    insert (size_type i, T& x)
+    insert (size_type i, T x)
     {
       size_type n (v_.size ());
 
       if (i < n)
-        v_[i] = &x;
+        v_[i] = x;
       else
       {
         if (n != i)
-          v_.resize (i, nullptr); // Add holes.
-        v_.push_back (&x);
+          v_.resize (i, T ()); // Add holes.
+
+        v_.push_back (move (x));
       }
     }
 
-    T*
+    T
     operator[] (size_type i) const
     {
-      return i < v_.size () ? v_[i] : nullptr;
+      return i < v_.size () ? v_[i] : T ();
     }
 
     bool
@@ -351,8 +351,28 @@ namespace build2
     base_type v_;
   };
 
-  using meta_operations = sparse_vector<const meta_operation_info, 8>;
-  using operations = sparse_vector<const operation_info, 10>;
+  // For operations we keep both the pointer to its description as well
+  // as to its operation variable (see var_include) which may belong to
+  // the project-private variable pool.
+  //
+  struct project_operation_info
+  {
+    const operation_info* info = nullptr;
+    const variable*       ovar = nullptr; // Operation variable.
+
+    // Allow treating it as pointer to operation_info in most contexts.
+    //
+    operator const operation_info*() const {return info;}
+    bool operator== (nullptr_t) {return info == nullptr;}
+    bool operator!= (nullptr_t) {return info != nullptr;}
+
+    project_operation_info (const operation_info* i = nullptr, // VC14
+                            const variable* v = nullptr)
+        : info (i), ovar (v) {}
+  };
+
+  using meta_operations = sparse_vector<const meta_operation_info*, 8>;
+  using operations = sparse_vector<project_operation_info, 10>;
 }
 
 namespace butl
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index ec33d6e..301acf9 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -3462,9 +3462,12 @@ namespace build2
         // In a somewhat hackish way we pass the variable in an undefined
         // lookup.
         //
+        // Note: consistent with parse_variable_name() wrt overridability.
+        //
         l = lookup ();
         l.var = &root_->var_pool ().insert (
-          move (report_var), true /* overridable */);
+          move (report_var),
+          report_var.find ('.') != string::npos /* overridable */);
       }
 
       if (l.var != nullptr)
@@ -4680,10 +4683,12 @@ namespace build2
   {
     // Enter a variable name for assignment (as opposed to lookup).
 
+    // If the variable is qualified (and thus public), make it overridable.
+    //
     // Note that the overridability can still be restricted (e.g., by a module
     // that enters this variable or by a pattern).
     //
-    bool ovr (true);
+    bool ovr (on.find ('.') != string::npos);
     auto r (scope_->var_pool ().insert (move (on), nullptr, nullptr, &ovr));
 
     if (!r.second)
@@ -4951,6 +4956,9 @@ namespace build2
     //@@ TODO: the same checks for vis and ovr (when we have the corresponding
     //         attributes).
 
+    //@@ TODO: need to verify/diagnose ovr and visibility for project-private
+    //         variables.
+
     if (type || vis || ovr)
       var.owner->update (const_cast<variable&> (var),
                          type,
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 598d7e8..a02d865 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -510,6 +510,8 @@ namespace build2
 
       // Project-private variable pool.
       //
+      // Note: see scope::var_pool_ and use scope::var_pool().
+      //
       variable_pool var_pool;
 
       // Meta/operations supported by this project.
@@ -551,16 +553,27 @@ namespace build2
 
     unique_ptr<root_extra_type> root_extra;
 
+    // The last argument is the operation variable (see var_include) or NULL
+    // if not used.
+    //
     void
-    insert_operation (operation_id id, const operation_info& in)
+    insert_operation (operation_id id,
+                      const operation_info& in,
+                      const variable* ovar)
     {
-      root_extra->operations.insert (id, in);
+      // The operation variable should have prerequisite or target visibility.
+      //
+      assert (ovar == nullptr ||
+              (ovar->visibility == variable_visibility::prereq ||
+               ovar->visibility == variable_visibility::target));
+
+      root_extra->operations.insert (id, project_operation_info {&in, ovar});
     }
 
     void
     insert_meta_operation (meta_operation_id id, const meta_operation_info& in)
     {
-      root_extra->meta_operations.insert (id, in);
+      root_extra->meta_operations.insert (id, &in);
     }
 
     bool
@@ -593,19 +606,29 @@ 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.
+    // Return the project-private variable pool (which is chained to the
+    // public pool) unless pub is true, in which case return the public pool.
+    //
+    // You would normally go for the public pool directly as an optimization
+    // (for example, in the module's init()) if you know all your variables
+    // are qualified and thus public.
     //
     variable_pool&
-    var_pool ()
+    var_pool (bool pub = false)
     {
-      return ctx.var_pool.rw (*this);
+      return (pub                  ? ctx.var_pool      :
+              var_pool_ != nullptr ? *var_pool_        :
+              root_     != nullptr ? *root_->var_pool_ :
+              ctx.var_pool).rw (*this);
     }
 
     const variable_pool&
-    var_pool () const
+    var_pool (bool pub = false) const
     {
-      return ctx.var_pool;
+      return (pub                  ? ctx.var_pool      :
+              var_pool_ != nullptr ? *var_pool_        :
+              root_     != nullptr ? *root_->var_pool_ :
+              ctx.var_pool);
     }
 
   private:
@@ -615,6 +638,7 @@ namespace build2
 
     // These two from <libbuild2/file.hxx> set strong_.
     //
+    friend void setup_root_extra (scope&, optional<bool>&);
     friend LIBBUILD2_SYMEXPORT void create_bootstrap_outer (scope&, bool);
     friend LIBBUILD2_SYMEXPORT scope& create_bootstrap_inner (scope&,
                                                               const dir_path&);
@@ -634,6 +658,8 @@ namespace build2
     scope* root_;
     scope* strong_ = nullptr; // Only set on root scopes.
                               // NULL means no strong amalgamtion.
+
+    variable_pool* var_pool_ = nullptr; // For temp_scope override.
   };
 
   inline bool
@@ -701,24 +727,28 @@ namespace build2
 
   // Temporary scope. The idea is to be able to create a temporary scope in
   // order not to change the variables in the current scope. Such a scope is
-  // not entered in to the scope map. As a result it can only be used as a
-  // temporary set of variables. In particular, defining targets directly in
-  // such a scope will surely end up badly. Defining any nested scopes will be
-  // as if defining such a scope in the parent (since path() returns parent's
-  // path).
+  // not entered in to the scope map and its parent is the global scope. As a
+  // result it can only be used as a temporary set of variables. In
+  // particular, defining targets directly in such a scope will surely end up
+  // badly.
   //
   class temp_scope: public scope
   {
   public:
-    temp_scope (scope& p)
-        : scope (p.ctx, false /* shared */)
+    temp_scope (scope& gs)
+        : scope (gs.ctx, false /* shared */),
+          var_pool_ (nullptr /* shared */, &gs.ctx.var_pool.rw (gs), nullptr)
     {
-      out_path_ = p.out_path_;
-      src_path_ = p.src_path_;
-      parent_ = &p;
-      root_ = p.root_;
-      // No need to copy strong_ since we are never root scope.
+      // Note that making this scope its own root is a bad idea.
+      //
+      root_ = nullptr;
+      parent_ = &gs;
+      out_path_ = gs.out_path_;
+      scope::var_pool_ = &var_pool_;
     }
+
+  private:
+    variable_pool var_pool_;
   };
 
   // Scope map. Protected by the phase mutex.
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index 14b6496..a466951 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -582,15 +582,22 @@ namespace build2
 
     names storage;
     names_view ns;
-    const variable* current_ovar (nullptr);
+    const variable* ovar (nullptr);
 
     if (r != include_type::excluded)
     {
-      current_ovar = a.outer ()
-        ? ctx.current_outer_ovar
-        : ctx.current_inner_ovar;
+      // Instead of going via potentially expensive target::base_scope(), use
+      // the prerequisite's scope; while it may not be the same as the
+      // targets's base scope, they must have the same root scope.
+      //
+      const scope& rs (*p.scope.root_scope ());
+
+      ovar = rs.root_extra->operations[
+        (a.outer ()
+         ? ctx.current_outer_oif
+         : ctx.current_inner_oif)->id].ovar;
 
-      if (current_ovar != nullptr && (l = p.vars[*current_ovar]))
+      if (ovar != nullptr && (l = p.vars[*ovar]))
       {
         // Maybe we should optimize this for the common cases (bool, path,
         // name)? But then again we don't expect many such overrides. Plus
@@ -635,7 +642,7 @@ namespace build2
         // Note: we have to delay this until the meta-operation callback above
         // had a chance to override it.
         //
-        fail << "unrecognized " << *current_ovar << " variable value "
+        fail << "unrecognized " << *ovar << " variable value "
              << "'" << ns << "' specified for prerequisite " << p;
       }
     }
diff --git a/libbuild2/test/init.cxx b/libbuild2/test/init.cxx
index c80d3f0..f62ddcc 100644
--- a/libbuild2/test/init.cxx
+++ b/libbuild2/test/init.cxx
@@ -30,11 +30,6 @@ namespace build2
 
       l5 ([&]{trace << "for " << rs;});
 
-      // Register our operations.
-      //
-      rs.insert_operation (test_id, op_test);
-      rs.insert_operation (update_for_test_id, op_update_for_test);
-
       // Enter module variables. Do it during boot in case they get assigned
       // in bootstrap.build.
       //
@@ -128,6 +123,11 @@ namespace build2
           v = *rs.ctx.build_host;
       }
 
+      // Register our operations.
+      //
+      rs.insert_operation (test_id,            op_test,            &d.var_test);
+      rs.insert_operation (update_for_test_id, op_update_for_test, &d.var_test);
+
       extra.set_module (new module (move (d)));
     }
 
diff --git a/libbuild2/test/operation.cxx b/libbuild2/test/operation.cxx
index b7ec357..841abb5 100644
--- a/libbuild2/test/operation.cxx
+++ b/libbuild2/test/operation.cxx
@@ -65,7 +65,6 @@ namespace build2
       0,
       "test",
       "test",
-      "test",
       "testing",
       "tested",
       "has nothing to test", // We cannot "be tested".
@@ -83,7 +82,6 @@ namespace build2
       update_id, // Note: not update_for_test_id.
       test_id,
       op_update.name,
-      op_update.var_name,
       op_update.name_do,
       op_update.name_doing,
       op_update.name_did,
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index 54bdcf6..8b5bc24 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -1578,6 +1578,17 @@ namespace build2
           const variable_visibility* v,
           const bool* o) const
   {
+    assert (var.owner == this);
+
+    if (outer_ != nullptr)
+    {
+      // Project-private variable. Assert visibility/overridability, the same
+      // as in insert().
+      //
+      assert ((o == nullptr || !*o) &&
+              (v == nullptr || *v >= variable_visibility::project));
+    }
+
     // Check overridability (all overrides, if any, should already have
     // been entered; see context ctor for details).
     //
@@ -1710,6 +1721,52 @@ namespace build2
           const bool* o,
           bool pat)
   {
+    if (outer_ != nullptr)
+    {
+      // Project-private pool.
+      //
+      if (n.find ('.') != string::npos) // Qualified.
+        return outer_->insert (move (n), t, v, o, pat);
+
+      // Unqualified.
+      //
+      // The pool chaining semantics for insertion: first check the outer pool
+      // then, if not found, insert in own pool.
+      //
+      if (const variable* var = outer_->find (n))
+      {
+        // Verify type/visibility/overridability.
+        //
+        // Should we assert or fail? Currently the buildfile parser goes
+        // through update() to set these so let's do assert for now. We also
+        // require equality (these are a handful of special variables).
+        //
+        assert ((t == nullptr ||  t == var->type) &&
+                (v == nullptr || *v == var->visibility) &&
+                (o == nullptr || *o || var->overrides == nullptr));
+
+        return pair<variable&, bool> (const_cast<variable&> (*var), false);
+      }
+
+      // Project-private variable. Assert visibility/overridability and fall
+      // through. Again, we expect the buildfile parser to verify and diagnose
+      // these.
+      //
+      // Note: similar code in update().
+      //
+      assert ((o == nullptr || !*o) &&
+              (v == nullptr || *v >= variable_visibility::project));
+    }
+    else if (shared_)
+    {
+      // Public pool.
+      //
+      // Make sure all the unqualified variables are pre-entered during
+      // initialization.
+      //
+      assert (shared_->load_generation == 0 || n.find ('.') != string::npos);
+    }
+
     assert (!shared_ || shared_->phase == run_phase::load);
 
     // Apply pattern.
@@ -1781,7 +1838,15 @@ namespace build2
   const variable& variable_pool::
   insert_alias (const variable& var, string n)
   {
-    assert (var.aliases != nullptr && var.overrides == nullptr);
+    if (outer_ != nullptr)
+    {
+      assert (n.find ('.') != string::npos); // Qualified.
+      return outer_->insert_alias (var, move (n));
+    }
+
+    assert (var.owner == this      &&
+            var.aliases != nullptr &&
+            var.overrides == nullptr);
 
     variable& a (insert (move (n),
                          var.type,
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index bfe3b87..9206cbb 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -1270,7 +1270,7 @@ namespace build2
     //
     // Note also that a pattern and later insertions may restrict (but not
     // relax) visibility and overridability.
-
+    //
     const variable&
     insert (string name)
     {
@@ -1357,6 +1357,8 @@ namespace build2
     // don't allow this (manually handle multiple names by merging their
     // values instead).
     //
+    // Note: currently only public variables can be aliased.
+    //
     const variable&
     insert_alias (const variable& var, string name);
 
@@ -1380,6 +1382,8 @@ namespace build2
     // true        null  --  public variable pool in context
     // true    not null  --  project-private pool in scope::root_extra
     //                       with outer pointing to context::var_pool
+    // false   not null  --  temporary scope-private pool in temp_scope
+    //                       with outer pointing to context::var_pool
     // false       null  --  script-private pool in script::environment
     //
     // Notice that the script-private pool doesn't rely on outer and does
@@ -1388,6 +1392,7 @@ namespace build2
     //
   private:
     friend class context;
+    friend class temp_scope;
     friend void setup_root_extra (scope&, optional<bool>&);
 
     // Shared pool (public or project-private). The shared argument is
@@ -1440,6 +1445,8 @@ namespace build2
             const bool* overridable,
             bool pattern = true);
 
+    // Note: the variable must belong to this pool.
+    //
     void
     update (variable&,
             const value_type*,
@@ -1457,7 +1464,7 @@ namespace build2
       // 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);
+      string n (var.name); // @@ PERF (maybe keep reuse buffer at least?)
       auto r (map_.insert (map::value_type (&n, move (var))));
 
       if (r.second)
diff --git a/libbuild2/variable.ixx b/libbuild2/variable.ixx
index a84c012..7cafd89 100644
--- a/libbuild2/variable.ixx
+++ b/libbuild2/variable.ixx
@@ -916,8 +916,21 @@ namespace build2
   inline const variable* variable_pool::
   find (const string& n) const
   {
+    // The pool chaining semantics for lookup: first check own pool then, if
+    // not found, check the outer pool.
+    //
     auto i (map_.find (&n));
-    return i != map_.end () ? &i->second : nullptr;
+    if (i != map_.end ())
+      return &i->second;
+
+    if (outer_ != nullptr)
+    {
+      i = outer_->map_.find (&n);
+      if (i != outer_->map_.end ())
+        return &i->second;
+    }
+
+    return nullptr;
   }
 
   // variable_map