Tie loose ends in target type/pattern-specific matching

7 files changed, 119 insertions, 42 deletions

diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 05a4da8..0482fcb 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -1522,12 +1522,14 @@ namespace build2
 
             if (!l.defined ())
             {
-              bool g (pt->group != nullptr);
+              const target* g (pt->group);
+
+              target_key tk (pt->key ());
+              target_key gk (g != nullptr ? g->key () : target_key {});
+
               l = bs.lookup_original (var,
-                                      &pt->type (),
-                                      &pt->name,
-                                      (g ? &pt->group->type () : nullptr),
-                                      (g ? &pt->group->name : nullptr)).first;
+                                      &tk,
+                                      g != nullptr ? &gk : nullptr).first;
             }
 
             if (l ? cast<bool> (*l) : u)
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index 3b612f8..46e3dcd 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -32,11 +32,11 @@ namespace build2
   //
   pair<lookup, size_t> scope::
   lookup_original (const variable& var,
-                   const target_type* tt, const string* tn,
-                   const target_type* gt, const string* gn,
+                   const target_key* tk,
+                   const target_key* gk,
                    size_t start_d) const
   {
-    assert (tt != nullptr || var.visibility != variable_visibility::target);
+    assert (tk != nullptr || var.visibility != variable_visibility::target);
 
     size_t d (0);
 
@@ -47,8 +47,9 @@ namespace build2
     //
     auto pre_app = [&var, this] (lookup_type& l,
                                  const scope* s,
-                                 const target_type* tt, const string* tn,
-                                 const target_type* gt, const string* gn)
+                                 const target_key* tk,
+                                 const target_key* gk,
+                                 optional<string> n)
     {
       const value& v (*l);
       assert ((v.extra == 1 || v.extra == 2) && v.type == nullptr);
@@ -62,14 +63,14 @@ namespace build2
       // group, then we shouldn't be looking for stem in the target's
       // variables. In other words, once we "jump" to group, we stay there.
       //
-      lookup_type stem (s->lookup_original (var, tt, tn, gt, gn, 2).first);
+      lookup_type stem (s->lookup_original (var, tk, gk, 2).first);
 
       // Check the cache.
       //
       pair<value&, ulock> entry (
         s->target_vars.cache.insert (
           ctx,
-          make_tuple (&v, tt, *tn),
+          make_tuple (&v, tk->type, n && !n->empty () ? move (*n) : *tk->name),
           stem,
           static_cast<const variable_map::value_data&> (v).version,
           var));
@@ -120,9 +121,17 @@ namespace build2
       l.value = &cv;
     };
 
+    // Most of the time we match against the target name directly but
+    // sometimes we may need to match against the directory leaf (dir{} or
+    // fsdir{}) or incorporate the extension. We therefore try hard to avoid
+    // the copy.
+    //
+    optional<string> tn;
+    optional<string> gn;
+
     for (const scope* s (this); s != nullptr; )
     {
-      if (tt != nullptr) // This started from the target.
+      if (tk != nullptr) // This started from the target.
       {
         bool f (!s->target_vars.empty ());
 
@@ -132,12 +141,12 @@ namespace build2
         {
           if (f)
           {
-            lookup_type l (s->target_vars.find (*tt, *tn, var));
+            lookup_type l (s->target_vars.find (*tk, var, tn));
 
             if (l.defined ())
             {
               if (l->extra != 0) // Prepend/append?
-                pre_app (l, s, tt, tn, gt, gn);
+                pre_app (l, s, tk, gk, move (tn));
 
               return make_pair (move (l), d);
             }
@@ -148,14 +157,14 @@ namespace build2
         //
         if (++d >= start_d)
         {
-          if (f && gt != nullptr)
+          if (f && gk != nullptr)
           {
-            lookup_type l (s->target_vars.find (*gt, *gn, var));
+            lookup_type l (s->target_vars.find (*gk, var, gn));
 
             if (l.defined ())
             {
               if (l->extra != 0) // Prepend/append?
-                pre_app (l, s, gt, gn, nullptr, nullptr);
+                pre_app (l, s, gk, nullptr, move (gn));
 
               return make_pair (move (l), d);
             }
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 5a853d9..3529495 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -132,17 +132,25 @@ namespace build2
     lookup_type
     lookup (const variable& var, const target_key& tk) const
     {
-      //@@ TODO: dir name
-      return lookup (var, tk.type, tk.name).first;
+      return lookup (var, &tk).first;
+    }
+
+    lookup_type
+    lookup (const variable& var,
+            const target_key& tk,
+            const target_key& gk) const
+    {
+      return lookup (var, &tk, &gk).first;
     }
 
     // Note for dir{} and fsdir{} target name is the directory leaf (without
-    // the trailing slash).
+    // the trailing slash). Also, if extension is to be matched (for this
+    // target type), then it should be included in the name.
     //
     lookup_type
     lookup (const variable& var, const target_type& tt, const string& tn) const
     {
-      return lookup (var, &tt, &tn).first;
+      return lookup (var, target_key {&tt, nullptr, nullptr, &tn, nullopt});
     }
 
     lookup_type
@@ -150,15 +158,20 @@ namespace build2
             const target_type& tt, const string& tn,
             const target_type& gt, const string& gn) const
     {
-      return lookup (var, &tt, &tn, &gt, &gn).first;
+      return lookup (var,
+                     target_key {&tt, nullptr, nullptr, &tn, nullopt},
+                     target_key {&gt, nullptr, nullptr, &gn, nullopt});
     }
 
+    // Note that target keys may be incomplete (only type and name must be
+    // present plus dir for dir{} and fsdir{} targets if name is empty).
+    //
     pair<lookup_type, size_t>
     lookup (const variable& var,
-            const target_type* tt = nullptr, const string* tn = nullptr,
-            const target_type* gt = nullptr, const string* gn = nullptr) const
+            const target_key* tk = nullptr,
+            const target_key* gk = nullptr) const
     {
-      auto p (lookup_original (var, tt, tn, gt, gn));
+      auto p (lookup_original (var, tk, gk));
       return var.overrides == nullptr ? p : lookup_override (var, move (p));
     }
 
@@ -166,11 +179,10 @@ namespace build2
     // can be used to skip a number of initial lookups.
     //
     pair<lookup_type, size_t>
-    lookup_original (
-      const variable&,
-      const target_type* tt = nullptr, const string* tn = nullptr,
-      const target_type* gt = nullptr, const string* gn = nullptr,
-      size_t start_depth = 1) const;
+    lookup_original (const variable&,
+                     const target_key* tk = nullptr,
+                     const target_key* gk = nullptr,
+                     size_t start_depth = 1) const;
 
     pair<lookup_type, size_t>
     lookup_override (const variable& var,
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index 6ba12b9..a686fc2 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -182,13 +182,13 @@ namespace build2
     {
       if (!target_only)
       {
-        //@@ TODO: dir name
+        target_key tk (key ());
+        target_key gk (g != nullptr ? g->key () : target_key {});
+
         auto p (base_scope ().lookup_original (
                   var,
-                  &type (),
-                  &name,
-                  g != nullptr ? &g->type () : nullptr,
-                  g != nullptr ? &g->name : nullptr));
+                  &tk,
+                  g != nullptr ? &gk : nullptr));
 
         r.first = move (p.first);
         r.second = r.first ? r.second + p.second : p.second;
diff --git a/libbuild2/target.txx b/libbuild2/target.txx
index fb9fe9c..c34adde 100644
--- a/libbuild2/target.txx
+++ b/libbuild2/target.txx
@@ -94,6 +94,9 @@ namespace build2
   {
     // Include target type/pattern-specific variables.
     //
+    // Note that we know this is not dir{} or fsdir{} and that the extension
+    // is not part of the match (see variable_type_map::find() for details).
+    //
     if (auto l = s.lookup (*s.ctx.var_extension, tt, tn))
     {
       // Help the user here and strip leading '.' from the extension.
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index 451cb76..6de0cfb 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -7,6 +7,7 @@
 
 #include <libbutl/path-pattern.mxx>
 
+#include <libbuild2/target.hxx>
 #include <libbuild2/diagnostics.hxx>
 
 using namespace std;
@@ -1791,13 +1792,57 @@ namespace build2
   // variable_type_map
   //
   lookup variable_type_map::
-  find (const target_type& type,
-        const string& name,
-        const variable& var) const
+  find (const target_key& tk,
+        const variable& var,
+        optional<string>& oname) const
   {
+    // Compute and cache "effective" name that we will be matching.
+    //
+    auto name = [&tk, &oname] () -> const string&
+    {
+      if (!oname)
+      {
+        const target_type& tt (*tk.type);
+
+        // Note that if the name is not empty, then we always use that, even
+        // if the type is dir/fsdir.
+        //
+        if (tk.name->empty () && (tt.is_a<dir> () || tt.is_a<fsdir> ()))
+        {
+          oname = tk.dir->leaf ().string ();
+        }
+        // If we have the extension and the type expects the extension to be
+        // always specified explicitly by the user, then add it to the name.
+        //
+        // Overall, we have the following cases:
+        //
+        // 1. Extension is fixed: man1{}.
+        //
+        // 2. Extension is always specified by the user: file{}.
+        //
+        // 3. Default extension that may be overridden by the user: hxx{}.
+        //
+        // 4. Extension assigned by the rule but may be overridden by the
+        //    user: obje{}.
+        //
+        // By default we only match the extension for (2).
+        //
+        else if (tk.ext && !tk.ext->empty () &&
+                 (tt.fixed_extension == &target_extension_none ||
+                  tt.fixed_extension == &target_extension_must))
+        {
+          oname = *tk.name + '.' + *tk.ext;
+        }
+        else
+          oname = string (); // Use tk.name as is.
+      }
+
+      return oname->empty () ? *tk.name : *oname;
+    };
+
     // Search across target type hierarchy.
     //
-    for (auto tt (&type); tt != nullptr; tt = tt->base)
+    for (auto tt (tk.type); tt != nullptr; tt = tt->base)
     {
       auto i (map_.find (*tt));
 
@@ -1820,7 +1865,7 @@ namespace build2
         //
         if (pat != "*")
         {
-          if (!butl::path_match (name, pat))
+          if (!butl::path_match (name (), pat))
             continue;
         }
 
@@ -1840,6 +1885,12 @@ namespace build2
             if (v->extra == 0 && var.type != nullptr)
               vm.typify (*v, var);
 
+            // Make sure the effective name is computed if this is
+            // append/prepend (it is used as a cache key).
+            //
+            if (v->extra != 0)
+              name ();
+
             return lookup (*v, p.second, vm);
           }
         }
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index d40934c..a40da92 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -1784,7 +1784,7 @@ namespace build2
     bool           empty () const {return map_.empty ();}
 
     lookup
-    find (const target_type&, const string& tname, const variable&) const;
+    find (const target_key&, const variable&, optional<string>& name) const;
 
     // Prepend/append value cache.
     //