Redo variable pattern-typing to match in more specific order

4 files changed, 123 insertions, 41 deletions

diff --git a/build2/config/init.cxx b/build2/config/init.cxx
index cfb69e0..a73c394 100644
--- a/build2/config/init.cxx
+++ b/build2/config/init.cxx
@@ -40,9 +40,10 @@ namespace build2
 
       auto& vp (var_pool.rw (rs));
 
-      // utility.cxx:unconfigured()
+      // utility.cxx:unconfigured() (note: not overridable).
       //
-      vp.insert_pattern<bool> ("config.*.configured");
+      vp.insert_pattern<bool> (
+        "config.*.configured", false, variable_visibility::normal);
 
       // Load config.build if one exists.
       //
@@ -51,7 +52,7 @@ namespace build2
       // possible that some module which needs the configuration will get
       // called first.
       //
-      const variable& c_v (vp.insert<uint64_t> ("config.version"));
+      const variable& c_v (vp.insert<uint64_t> ("config.version", false));
 
       // Don't load it if we are disfiguring. This is a bit tricky since the
       // build2 core may not yet know it is disfiguring. But we know.
diff --git a/build2/context.cxx b/build2/context.cxx
index a326002..466db0d 100644
--- a/build2/context.cxx
+++ b/build2/context.cxx
@@ -237,19 +237,23 @@ namespace build2
     // Enter builtin variables and patterns.
     //
 
-    // file.cxx:import()
+    // All config. variables are by default overridable.
     //
-    // Note that the order is important (reverse application).
+    vp.insert_pattern ("config.**", nullopt, true, nullopt, true, false);
+
+    // file.cxx:import() (note that order is important; see insert_pattern()).
     //
-    vp.insert_pattern<path> ("config.import.**", true);
-    vp.insert_pattern<abs_dir_path> ("config.import.*", true);
+    vp.insert_pattern<abs_dir_path> (
+      "config.import.*", true, variable_visibility::normal, true);
+    vp.insert_pattern<path> (
+      "config.import.**", true, variable_visibility::normal, true);
 
     // module.cxx:load_module().
     //
-    vp.insert_pattern<bool> ("**.loaded", false, variable_visibility::project);
-    vp.insert_pattern<bool> ("**.configured",
-                             false,
-                             variable_visibility::project);
+    vp.insert_pattern<bool> (
+      "**.loaded", false, variable_visibility::project);
+    vp.insert_pattern<bool> (
+      "**.configured", false, variable_visibility::project);
 
     var_src_root = &vp.insert<dir_path> ("src_root");
     var_out_root = &vp.insert<dir_path> ("out_root");
diff --git a/build2/variable b/build2/variable
index 6e20a9c..33d33a7 100644
--- a/build2/variable
+++ b/build2/variable
@@ -6,6 +6,7 @@
 #define BUILD2_VARIABLE
 
 #include <map>
+#include <set>
 #include <functional>    // hash
 #include <type_traits>   // aligned_storage
 #include <unordered_map>
@@ -858,7 +859,12 @@ namespace build2
     }
 
     // Insert a variable pattern. Any variable that matches this pattern
-    // will have the specified type, visibility, and overridability.
+    // will have the specified type, visibility, and overridability. If
+    // match is true, then individual insertions of the matching variable
+    // must match the specified type/visibility/overridability. Otherwise,
+    // individual insertions can provide alternative values and the pattern
+    // values are a fallback (if you specify false you better be very clear
+    // about what you are trying to achieve).
     //
     // The pattern must be in the form [<prefix>.](*|**)[.<suffix>] where
     // '*' matches single component stems (i.e., 'foo' but not 'foo.bar')
@@ -866,25 +872,37 @@ namespace build2
     // multi-component variables are considered for pattern matching (so
     // just '*' won't match anything).
     //
-    // Note that patterns are matched in the reverse order of insertion (as
-    // opposed to, say, more specific first) with the first match used. A
-    // newly inserted pattern is also applied retrospectively to all the
-    // existing variables that match.
+    // The patterns are matched in the more-specific-first order where the
+    // pattern is considered more specific if it has a greater sum of its
+    // prefix and suffix lengths. If the prefix and suffix are equal, then the
+    // '*' pattern is considered more specific than '**'. If neither is more
+    // specific, then they are matched in the reverse order of insertion.
+    //
+    // If retro is true then a newly inserted pattern is also applied
+    // retrospectively to all the existing variables that match but only
+    // if no more specific pattern already exists (which is then assumed
+    // to have been applied). So if you use this functionality, watch out
+    // for the insertion order (you probably want more specific first).
     //
   public:
     void
     insert_pattern (const string& pattern,
-                    const build2::value_type* type,
-                    bool overridable,
-                    variable_visibility);
+                    optional<const value_type*> type,
+                    optional<bool> overridable,
+                    optional<variable_visibility>,
+                    bool retro = false,
+                    bool match = true);
 
     template <typename T>
     void
     insert_pattern (const string& p,
-                    bool overridable = false,
-                    variable_visibility v = variable_visibility::normal)
+                    optional<bool> overridable,
+                    optional<variable_visibility> v,
+                    bool retro = false,
+                    bool match = true)
     {
-      insert_pattern (p, &value_traits<T>::value_type, overridable, v);
+      insert_pattern (
+        p, &value_traits<T>::value_type, overridable, v, retro, match);
     }
 
   public:
@@ -906,13 +924,13 @@ namespace build2
 
     const variable&
     insert (string name,
-            const build2::value_type*,
+            const value_type*,
             const variable_visibility* = nullptr,
             const bool* overridable = nullptr);
 
     void
     update (variable&,
-            const build2::value_type*,
+            const value_type*,
             const variable_visibility* = nullptr,
             const bool* = nullptr) const;
 
@@ -957,15 +975,29 @@ namespace build2
     {
       string prefix;
       string suffix;
-      bool   multi;  // Match multi-component stems.
+      bool   multi; // Match multi-component stems.
+      bool   match; // Must match individual variable insersions.
+
+      optional<const value_type*>   type;
+      optional<variable_visibility> visibility;
+      optional<bool>                overridable;
+
+      friend bool
+      operator< (const pattern& x, const pattern& y)
+      {
+        if (x.prefix.size () + x.suffix.size () <
+            y.prefix.size () + y.suffix.size ())
+          return true;
+
+        if (x.prefix == y.prefix && x.suffix == y.suffix)
+          return x.multi && !y.multi;
 
-      const build2::value_type* type;
-      variable_visibility       visibility;
-      bool                      overridable;
+        return false;
+      }
     };
 
   private:
-    vector<pattern> patterns_;
+    std::multiset<pattern> patterns_;
 
     // Global pool flag.
     //
diff --git a/build2/variable.cxx b/build2/variable.cxx
index 963eb0e..78f7c04 100644
--- a/build2/variable.cxx
+++ b/build2/variable.cxx
@@ -982,9 +982,29 @@ namespace build2
                  const variable_visibility*& v,
                  const bool*& o)
   {
-    if (t == nullptr) t =  p.type;        else assert ( t == p.type);
-    if (v == nullptr) v = &p.visibility;  else assert (*v == p.visibility);
-    if (o == nullptr) o = &p.overridable; else assert (*o == p.overridable);
+    if (p.type)
+    {
+      if (t == nullptr)
+        t = *p.type;
+      else if (p.match)
+        assert (t == *p.type);
+    }
+
+    if (p.visibility)
+    {
+      if (v == nullptr)
+        v = &*p.visibility;
+      else if (p.match)
+        assert (*v == *p.visibility);
+    }
+
+    if (p.overridable)
+    {
+      if (o == nullptr)
+        o = &*p.overridable;
+      else if (p.match)
+        assert (*o == *p.overridable);
+    }
   }
 
   const variable& variable_pool::
@@ -999,6 +1019,8 @@ namespace build2
     //
     if (n.find ('.') != string::npos)
     {
+      // Reverse means from the "largest" (most specific).
+      //
       for (const pattern& p: reverse_iterate (patterns_))
       {
         if (match_pattern (n, p.prefix, p.suffix, p.multi))
@@ -1034,9 +1056,11 @@ namespace build2
 
   void variable_pool::
   insert_pattern (const string& p,
-                  const build2::value_type* t,
-                  bool o,
-                  variable_visibility v)
+                  optional<const value_type*> t,
+                  optional<bool> o,
+                  optional<variable_visibility> v,
+                  bool retro,
+                  bool match)
   {
     assert (!global_ || phase == run_phase::load);
 
@@ -1066,17 +1090,38 @@ namespace build2
       sfx.assign (p, w, sn);
     }
 
+    auto i (
+      patterns_.insert (
+        pattern {move (pfx), move (sfx), multi, match, t, v, o}));
+
     // Apply retrospectively to existing variables.
     //
-    for (auto& p: map_)
+    if (retro)
     {
-      variable& var (p.second);
+      for (auto& p: map_)
+      {
+        variable& var (p.second);
 
-      if (match_pattern (var.name, pfx, sfx, multi))
-        update (var, t, &v, &o); // Not changing the key.
-    }
+        if (match_pattern (var.name, i->prefix, i->suffix, i->multi))
+        {
+          // Make sure that none of the existing more specific patterns
+          // match.
+          //
+          auto j (i), e (patterns_.end ());
+          for (++j; j != e; ++j)
+          {
+            if (match_pattern (var.name, j->prefix, j->suffix, j->multi))
+              break;
+          }
 
-    patterns_.push_back (pattern {move (pfx), move (sfx), multi, t, v, o});
+          if (j == e)
+            update (var,
+                    t ?  *t : nullptr,
+                    v ? &*v : nullptr,
+                    o ? &*o : nullptr); // Not changing the key.
+        }
+      }
+    }
   }
 
   variable_pool variable_pool::instance (true);