Some ground work for proper variable cache implementation

2 files changed, 37 insertions, 27 deletions

diff --git a/build2/scope.cxx b/build2/scope.cxx
index aaa0077..6f21636 100644
--- a/build2/scope.cxx
+++ b/build2/scope.cxx
@@ -53,7 +53,8 @@ namespace build2
       //
       // @@ MT
       //
-      value& cache (s->target_vars.cache[make_tuple (&v, tt, *tn)]);
+      variable_map::value_data& cache (
+        s->target_vars.cache[make_tuple (&v, tt, *tn)]);
 
       // Un-typify the cache. This can be necessary, for example, if we are
       // changing from one value-typed stem to another.
@@ -185,7 +186,7 @@ namespace build2
     //
     // We also keep track of the root scope of the project from which this
     // innermost value comes. This is used to decide whether a non-recursive
-    // project-wise override applies.
+    // project-wise override applies. And also where our variable cache is.
     //
     const variable_map* inner_vars (nullptr);
     const scope* inner_proj (nullptr);
@@ -303,7 +304,7 @@ namespace build2
           if (inner_vars == nullptr)
           {
             inner_vars = l.vars;
-            inner_proj = s->root_scope (); // Not actually used.
+            inner_proj = s->root_scope ();
           }
 
           apply = true;
@@ -322,6 +323,12 @@ namespace build2
 
     assert (inner_vars != nullptr);
 
+    // If for some reason we are not in a project, use the cache from the
+    // global scope.
+    //
+    if (inner_proj == nullptr)
+      inner_proj = global_scope;
+
     // Implementing proper caching is tricky so for now we are going to re-
     // calculate the value every time. Later, the plan is to use value
     // versioning (incremented on every update) to detect stem value changes.
diff --git a/build2/variable b/build2/variable
index 40470fa..6c579dd 100644
--- a/build2/variable
+++ b/build2/variable
@@ -330,7 +330,7 @@ namespace build2
     using value_type = build2::value;
 
     const value_type* value;  // NULL if undefined.
-    const variable_map* vars;
+    const variable_map* vars; // value is variable_map::value_data if not NULL.
 
     bool
     defined () const {return value != nullptr;}
@@ -355,11 +355,11 @@ namespace build2
     template <typename T>
     lookup (const value_type& v, const T& x): lookup (&v, &x.vars) {}
 
-    lookup (const value_type& v, const variable_map& vs)
-        : value (&v), vars (&vs) {}
+    lookup (const value_type& v, const variable_map& vm)
+        : value (&v), vars (&vm) {}
 
-    lookup (const value_type* v, const variable_map* vs)
-        : value (v), vars (v != nullptr ? vs : nullptr) {}
+    lookup (const value_type* v, const variable_map* vm)
+        : value (v), vars (v != nullptr ? vm : nullptr) {}
   };
 
   // Two lookups are equal if they point to the same variable.
@@ -757,18 +757,6 @@ namespace build2
     static const value_type_ex value_type;
   };
 
-  // Variable override cache.
-  //
-  struct variable_override_value
-  {
-    build2::value value;
-    const variable_map* stem_vars = nullptr; // NULL means there is no stem.
-  };
-
-  using variable_override_cache = std::map<pair<const variable_map*,
-                                                const variable*>,
-                                           variable_override_value>;
-
   // Project-wide (as opposed to global) variable overrides. Returned by
   // context.cxx:reset().
   //
@@ -1046,6 +1034,7 @@ namespace build2
     struct value_data: value
     {
       using value::value;
+      using value::operator=;
 
       size_t version = 0; // Incremented on each modification (override cache).
       size_t generation;  // load_generation of this value (global state only).
@@ -1223,9 +1212,9 @@ namespace build2
       return map_.emplace (t, variable_pattern_map (global_)).first->second;
     }
 
-    const_iterator begin ()                    const {return map_.begin ();}
-    const_iterator end ()                      const {return map_.end ();}
-    bool           empty ()                    const {return map_.empty ();}
+    const_iterator begin () const {return map_.begin ();}
+    const_iterator end ()   const {return map_.end ();}
+    bool           empty () const {return map_.empty ();}
 
     lookup
     find (const target_type&, const string& tname, const variable&) const;
@@ -1237,18 +1226,32 @@ namespace build2
     // guarantee the references remain valid).
     //
     // The key is the combination of the "original value identity" (as a
-    // pointer to the value in variable_pattern_map) and the "target identity"
-    // (as target type and target name).
+    // pointer to the value in one of the variable_pattern_map's) and the
+    // "target identity" (as target type and target name). Note that while at
+    // first it may seem like we don't need the target identity, we actually
+    // do since the stem may itself be target-type/pattern-specific.
     //
     // @@ MT
     //
-    mutable
-    std::map<tuple<const value*, const target_type*, string>, value> cache;
+    mutable std::map<tuple<const value*, const target_type*, string>,
+                     variable_map::value_data> cache;
 
   private:
     bool global_;
     map_type map_;
   };
+
+  // Variable override cache.
+  //
+  struct variable_override_value
+  {
+    variable_map::value_data value;
+    const variable_map* stem_vars = nullptr; // NULL means there is no stem.
+  };
+
+  using variable_override_cache = std::map<pair<const variable_map*,
+                                                const variable*>,
+                                           variable_override_value>;
 }
 
 #include <build2/variable.ixx>