Add ability to depend on (declared) ad hoc group member

8 files changed, 242 insertions, 115 deletions

diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index bcdccf6..2e3fe83 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -206,33 +206,34 @@ namespace build2
     sched.resume (task_count);
   }
 
-  target_lock
-  add_adhoc_member (action a,
-                    target& t,
+  target&
+  add_adhoc_member (target& t,
                     const target_type& tt,
                     const dir_path& dir,
                     const dir_path& out,
-                    const string& n)
+                    string n)
   {
+    tracer trace ("add_adhoc_member");
+
     const_ptr<target>* mp (&t.member);
     for (; *mp != nullptr && !(*mp)->is_a (tt); mp = &(*mp)->member) ;
 
-    const target& m (*mp != nullptr // Might already be there.
-                     ? **mp
-                     : search (t, tt, dir, out, n));
-
-    target_lock l (lock (a, m));
-    assert (l.target != nullptr); // Someone messing with ad hoc members?
-
+    target& m (*mp != nullptr // Might already be there.
+               ? **mp
+               : targets.insert (tt,
+                                 dir,
+                                 out,
+                                 move (n),
+                                 nullopt /* ext     */,
+                                 true    /* implied */,
+                                 trace).first);
     if (*mp == nullptr)
     {
-      *mp = l.target;
-      l.target->group = &t;
+      *mp = &m;
+      m.group = &t;
     }
-    else
-      assert ((*mp)->dir == dir && (*mp)->name == n); // Basic sanity check.
 
-    return l;
+    return m;
   };
 
   // Return the matching rule or NULL if no match and try_match is true.
@@ -522,22 +523,62 @@ namespace build2
                  ? optional<scheduler::work_queue> (scheduler::work_none)
                  : nullopt));
 
-    if (l.target == nullptr)
-    {
-      // Already applied, executed, or busy.
-      //
-      if (l.offset >= target::offset_busy)
-        return make_pair (true, target_state::busy);
-
-      // Fall through.
-    }
-    else
+    if (l.target != nullptr)
     {
       assert (l.offset < target::offset_applied); // Shouldn't lock otherwise.
 
       if (try_match && l.offset == target::offset_tried)
         return make_pair (false, target_state::unknown);
 
+      // Handle matching ad hoc group member.
+      //
+      if (ct.adhoc_member ())
+      {
+        const target& g (*ct.group);
+
+        // It feels natural to "convert" this call to the one for the group,
+        // including the try_match and async parts. However, that async part
+        // is tricky: we will be called again to finish the match (the else-
+        // block below) where we need to perform the equivalent conversion.
+        // Semantically, we want to achieve the following:
+        //
+        // match (a, g);                    |  match_async (a, g);
+        //                                  |  match (a, g);
+        // match_recipe (l, group_recipe);  |  match_recipe (l, group_recipe);
+        //
+        // We also have to "unstack" this lock to avoid racing with stack
+        // modifications by the asynchronous match (see below). Since an ad
+        // hoc member doesn't have any prerequisites of its own (and thus
+        // cannot depend on the group), skipping this link during the cycle
+        // detection feels harmless (note that the other way around is
+        // possible and still works).
+        //
+        l.unstack ();
+
+
+#if 0   // The same story as with the lock stack. I think the only way to
+        // make this work is to somehow pass the member down to match_impl().
+        auto df = make_diag_frame (
+          [a, &ct](const diag_record& dr)
+          {
+            if (verb != 0)
+              dr << info << "while matching rule group to " << diag_do (a, ct);
+          });
+#endif
+
+        auto r (match (a, g, start_count, task_count, try_match));
+
+        if (r.first)
+        {
+          match_inc_dependens (a, g);
+          match_recipe (l, group_recipe);
+        }
+        else
+          l.offset = target::offset_tried;
+
+        return r; // Group state.
+      }
+
       if (task_count == nullptr)
         return match_impl (l, false /* step */, try_match);
 
@@ -579,6 +620,20 @@ namespace build2
 
       // Matched synchronously, fall through.
     }
+    else
+    {
+      // Already applied, executed, or busy.
+      //
+      if (l.offset >= target::offset_busy)
+        return make_pair (true, target_state::busy);
+
+      // Handle matching ad hoc group member (the finish part; see above).
+      //
+      if (ct.adhoc_member ())
+        return match (a, *ct.group, 0, nullptr);
+
+      // Fall through.
+    }
 
     return ct.try_matched_state (a, false);
   }
diff --git a/build2/algorithm.hxx b/build2/algorithm.hxx
index f1aad0c..e3473ad 100644
--- a/build2/algorithm.hxx
+++ b/build2/algorithm.hxx
@@ -155,6 +155,9 @@ namespace build2
     const target_lock* stack; // Tip of the stack.
     const target_lock* prev;
 
+    void
+    unstack ();
+
     struct stack_guard
     {
       explicit stack_guard (const target_lock* s): s_ (stack) {stack = s;}
@@ -175,37 +178,49 @@ namespace build2
   // action.
   //
   // @@ MT fuzzy: what if it is already in the desired state, why assert?
-  //    Currently we only use it with match_recipe().
+  //    Currently we only use it with match_recipe() and if it is matched
+  //    but not applied, then it's not clear why we are overriding that
+  //    match.
   //
   target_lock
   lock (action, const target&);
 
   // Add an ad hoc member to the end of the chain assuming that an already
-  // existing member of this target type is the same. Return the locked member
-  // target.
-  //
-  target_lock
-  add_adhoc_member (action,
-                    target&,
+  // existing member of this target type is the same. Return the newly added
+  // or already existing target. The member directories (dir and out) are
+  // expected to be absolute and normalized.
+  //
+  // Note that here and in find_adhoc_member() below we use target type (as
+  // opposed to, say, type and name) as the member's identity. This fits our
+  // current needs where every (rule-managed) ad hoc member has a unique
+  // target type and allows us to support things like overriding the ad hoc
+  // member name by the user.
+  //
+  target&
+  add_adhoc_member (target&,
                     const target_type&,
                     const dir_path& dir,
                     const dir_path& out,
-                    const string& name);
+                    string name);
 
   // If the extension is specified then it is added to the member's target
   // name.
   //
-  target_lock
-  add_adhoc_member (action,
-                    target&,
-                    const target_type&,
-                    const char* ext = nullptr);
+  target&
+  add_adhoc_member (target&, const target_type&, const char* ext = nullptr);
+
+  template <typename T>
+  inline T&
+  add_adhoc_member (target& g, const target_type& tt, const char* e = nullptr)
+  {
+    return static_cast<T&> (add_adhoc_member (g, tt, e));
+  }
 
   template <typename T>
-  inline target_lock
-  add_adhoc_member (action a, target& g, const char* e = nullptr)
+  inline T&
+  add_adhoc_member (target& g, const char* e = nullptr)
   {
-    return add_adhoc_member (a, g, T::static_type, e);
+    return add_adhoc_member<T> (g, T::static_type, e);
   }
 
   // Find an ad hoc member of the specified target type returning NULL if not
diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx
index 0150587..c79ee49 100644
--- a/build2/algorithm.ixx
+++ b/build2/algorithm.ixx
@@ -135,14 +135,29 @@ namespace build2
   }
 
   inline void target_lock::
+  unstack ()
+  {
+    if (target != nullptr && prev != this)
+    {
+      assert (stack == this);
+      stack = prev;
+      prev = this;
+    }
+  }
+
+  inline void target_lock::
   unlock ()
   {
     if (target != nullptr)
     {
       unlock_impl (action, *target, offset);
 
-      assert (stack == this);
-      stack = prev;
+      if (prev != this)
+      {
+        assert (stack == this);
+        stack = prev;
+      }
+
       target = nullptr;
     }
   }
@@ -154,8 +169,12 @@ namespace build2
 
     if (target != nullptr)
     {
-      assert (stack == this);
-      stack = prev;
+      if (prev != this)
+      {
+        assert (stack == this);
+        stack = prev;
+      }
+
       target = nullptr;
     }
 
@@ -174,9 +193,14 @@ namespace build2
   {
     if (target != nullptr)
     {
-      assert (stack == &x);
-      prev = x.prev;
-      stack = this;
+      if (x.prev != &x)
+      {
+        assert (stack == &x);
+        prev = x.prev;
+        stack = this;
+      }
+      else
+        prev = this;
 
       x.target = nullptr;
     }
@@ -195,9 +219,14 @@ namespace build2
 
       if (target != nullptr)
       {
-        assert (stack == &x);
-        prev = x.prev;
-        stack = this;
+        if (x.prev != &x)
+        {
+          assert (stack == &x);
+          prev = x.prev;
+          stack = this;
+        }
+        else
+          prev = this;
 
         x.target = nullptr;
       }
@@ -232,8 +261,8 @@ namespace build2
     return r;
   }
 
-  inline target_lock
-  add_adhoc_member (action a, target& t, const target_type& tt, const char* e)
+  inline target&
+  add_adhoc_member (target& t, const target_type& tt, const char* e)
   {
     string n (t.name);
 
@@ -243,7 +272,7 @@ namespace build2
       n += e;
     }
 
-    return add_adhoc_member (a, t, tt, t.dir, t.out, n);
+    return add_adhoc_member (t, tt, t.dir, t.out, move (n));
   }
 
   inline target*
@@ -271,6 +300,13 @@ namespace build2
   pair<bool, target_state>
   match (action, const target&, size_t, atomic_count*, bool try_match = false);
 
+  inline void
+  match_inc_dependens (action a, const target& t)
+  {
+    dependency_count.fetch_add (1, memory_order_relaxed);
+    t[a].dependents.fetch_add (1, memory_order_release);
+  }
+
   inline target_state
   match (action a, const target& t, bool fail)
   {
@@ -279,10 +315,7 @@ namespace build2
     target_state r (match (a, t, 0, nullptr).second);
 
     if (r != target_state::failed)
-    {
-      dependency_count.fetch_add (1, memory_order_relaxed);
-      t[a].dependents.fetch_add (1, memory_order_release);
-    }
+      match_inc_dependens (a, t);
     else if (fail)
       throw failed ();
 
@@ -300,10 +333,7 @@ namespace build2
     if (r.first)
     {
       if (r.second != target_state::failed)
-      {
-        dependency_count.fetch_add (1, memory_order_relaxed);
-        t[a].dependents.fetch_add (1, memory_order_release);
-      }
+        match_inc_dependens (a, t);
       else if (fail)
         throw failed ();
     }
@@ -333,7 +363,9 @@ namespace build2
       }
     case unmatch::safe:
       {
-        // Safe if unchanged or someone else is also a dependent.
+        // Safe if unchanged or someone else is also a dependent (note that
+        // we never decrement this count during match so that someone else
+        // cannot change their mind).
         //
         if (s == target_state::unchanged                   ||
             t[a].dependents.load (memory_order_consume) != 0)
@@ -343,9 +375,7 @@ namespace build2
       }
     }
 
-    dependency_count.fetch_add (1, memory_order_relaxed);
-    t[a].dependents.fetch_add (1, memory_order_release);
-
+    match_inc_dependens (a, t);
     return false;
   }
 
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index cd49689..3458a1b 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -677,8 +677,10 @@ namespace build2
           // (e.g., foo.mxx and foo.cxx) which means obj*{} targets could
           // collide. So we add the module extension to the target name.
           //
-          target_lock obj (add_adhoc_member (a, t, tts.obj, e.c_str ()));
-          obj.target->as<file> ().derive_path (o);
+          file& obj (add_adhoc_member<file> (t, tts.obj, e.c_str ()));
+
+          if (obj.path ().empty ())
+            obj.derive_path (o);
         }
       }
 
diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index dd34d95..94e19e0 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -291,8 +291,12 @@ namespace build2
         lk = b;
         append_ext (lk);
 
-        libi& li (*find_adhoc_member<libi> (ls)); // Note: libi is locked.
-        lk = li.derive_path (move (lk), tsys == "mingw32" ? "a" : "lib");
+        libi& li (*find_adhoc_member<libi> (ls));
+        const path& pi (li.path ());
+
+        lk = pi.empty ()
+          ? li.derive_path (move (lk), tsys == "mingw32" ? "a" : "lib")
+          : pi;
       }
       else if (!v.empty ())
       {
@@ -600,8 +604,6 @@ namespace build2
       // to not derive the path for the library target itself inside.
       //
       {
-        target_lock libi; // Have to hold until after PDB member addition.
-
         const char* e (nullptr); // Extension.
         const char* p (nullptr); // Prefix.
         const char* s (nullptr); // Suffix.
@@ -715,8 +717,8 @@ namespace build2
                 //
                 if (tclass == "windows")
                 {
-                  libi = add_adhoc_member<bin::libi> (a, t);
                   e = "dll";
+                  add_adhoc_member<libi> (t);
                 }
 
                 md.libs_data = derive_libs_paths (t, p, s);
@@ -734,17 +736,19 @@ namespace build2
             if (find_option ("/DEBUG", t, c_loptions, true) ||
                 find_option ("/DEBUG", t, x_loptions, true))
             {
+              const target_type& tt (*bs.find_target_type ("pdb"));
+
               // We call the target foo.{exe,dll}.pdb rather than just foo.pdb
               // because we can have both foo.exe and foo.dll in the same
               // directory.
               //
-              target_lock pdb (
-                add_adhoc_member (a, t, *bs.find_target_type ("pdb"), e));
+              file& pdb (add_adhoc_member<file> (t, tt, e));
 
               // Note that the path is derived from the exe/dll path (so it
               // will include the version in case of a dll).
               //
-              pdb.target->as<file> ().derive_path (t.path (), "pdb");
+              if (pdb.path ().empty ())
+                pdb.derive_path (t.path (), "pdb");
             }
           }
 
@@ -759,20 +763,19 @@ namespace build2
           //
           if (ot != otype::e)
           {
-            target_lock pc (
-              add_adhoc_member (
-                a, t,
-                ot == otype::a ? pca::static_type : pcs::static_type));
+            file& pc (add_adhoc_member<file> (t,
+                                              (ot == otype::a
+                                               ? pca::static_type
+                                               : pcs::static_type)));
 
             // Note that here we always use the lib name prefix, even on
             // Windows with VC. The reason is the user needs a consistent name
             // across platforms by which they can refer to the library. This
-            // is also the reason why we use the .static/.shared second-level
-            // extensions rather that a./.lib/.so/.dylib/.dll.
+            // is also the reason why we use the .static and .shared second-
+            // level extensions rather that a./.lib and .so/.dylib/.dll.
             //
-            pc.target->as<file> ().derive_path (nullptr,
-                                                (p == nullptr ? "lib" : p),
-                                                s);
+            if (pc.path ().empty ())
+              pc.derive_path (nullptr, (p == nullptr ? "lib" : p), s);
           }
 
           // Add the Windows rpath emulating assembly directory as fsdir{}.
@@ -791,14 +794,14 @@ namespace build2
             // exists (windows_rpath_assembly() does take care to clean it up
             // if not used).
             //
-            target_lock dir (
-              add_adhoc_member (
-                a,
-                t,
-                fsdir::static_type,
-                path_cast<dir_path> (t.path () + ".dlls"),
-                t.out,
-                string ()));
+#ifdef _WIN32
+            target& dir =
+#endif
+              add_adhoc_member (t,
+                                fsdir::static_type,
+                                path_cast<dir_path> (t.path () + ".dlls"),
+                                t.out,
+                                string () /* name */);
 
             // By default our backlinking logic will try to symlink the
             // directory and it can even be done on Windows using junctions.
@@ -812,7 +815,7 @@ namespace build2
             // Windows.
             //
 #ifdef _WIN32
-            dir.target->state[a].assign (var_backlink) = "copy";
+            dir.state[a].assign (var_backlink) = "copy";
 #endif
           }
         }
diff --git a/build2/parser.cxx b/build2/parser.cxx
index 06be54a..12dcd54 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -973,6 +973,24 @@ namespace build2
       if (n.qualified ())
         fail (loc) << "project name in target " << n;
 
+      // We derive the path unless the target name ends with the '...' escape
+      // which here we treat as the "let the rule derive the path" indicator
+      // (see target::split_name() for details). This will only be useful for
+      // referring to ad hoc members that are managed by the group's matching
+      // rule. Note also that omitting '...' for such a member could be used
+      // to override the file name, provided the rule checks if the path has
+      // already been derived before doing it itself.
+      //
+      bool escaped;
+      {
+        const string& v (n.value);
+        size_t p (v.size ());
+
+        escaped = (p > 3 &&
+                   v[--p] == '.' && v[--p] == '.' && v[--p] == '.' &&
+                   v[--p] != '.');
+      }
+
       target& at (
         enter_target::insert_target (*this,
                                      move (n), move (o),
@@ -985,8 +1003,6 @@ namespace build2
       // Add as an ad hoc member at the end of the chain skipping duplicates.
       //
       {
-        // @@ ADHOC: call add_adhoc_member()?
-        //
         const_ptr<target>* mp (&target_->member);
         for (; *mp != nullptr; mp = &(*mp)->member)
         {
@@ -1002,15 +1018,13 @@ namespace build2
           *mp = &at;
           at.group = target_;
         }
-        else
-          continue; // Duplicate.
       }
 
-      // @@ ADHOC: What if it's something like .pdb where the group derives a
-      //           custom extension... Hm...
-      //
-      if (file* ft = at.is_a<file> ())
-        ft->derive_path ();
+      if (!escaped)
+      {
+        if (file* ft = at.is_a<file> ())
+          ft->derive_path ();
+      }
     }
   }
 
diff --git a/build2/search.cxx b/build2/search.cxx
index 7683ef6..68fd5a5 100644
--- a/build2/search.cxx
+++ b/build2/search.cxx
@@ -228,9 +228,13 @@ namespace build2
     //
     // @@ OUT: same story as in search_existing_target() re out.
     //
-    auto r (
-      targets.insert (
-        *tk.type, move (d), *tk.out, *tk.name, tk.ext, true, trace));
+    auto r (targets.insert (*tk.type,
+                            move (d),
+                            *tk.out,
+                            *tk.name,
+                            tk.ext,
+                            true /* implied */,
+                            trace));
 
     const target& t (r.first);
     l5 ([&]{trace << (r.second ? "new" : "existing") << " target " << t
diff --git a/build2/target.hxx b/build2/target.hxx
index e167211..4c0efbe 100644
--- a/build2/target.hxx
+++ b/build2/target.hxx
@@ -1539,14 +1539,18 @@ namespace build2
     // any time but any subsequent updates must set the same path. Or, in
     // other words, once the path is set, it never changes.
     //
-    // A set empty path may signify special unknown/undetermined/unreal
-    // location (for example, a binless library or an installed import library
-    // -- we know the DLL is there, just not exactly where). In this case you
-    // would also normally set its mtime. We used to return a pointer to
-    // properly distinguish between not set and empty but that proved too
-    // tedious. Note that this means there could be a race between path and
-    // mtime (unless you lock the target in some other way; see file_rule) so
-    // for this case it makes sense to set the timestamp first.
+    // An empty path may signify special unknown/undetermined/unreal location
+    // (for example, a binless library or an installed import library -- we
+    // know the DLL is there, just not exactly where). In this case you would
+    // also normally set its mtime.
+    //
+    // We used to return a pointer to properly distinguish between not set and
+    // empty but that proved too tedious to work with. So now we return empty
+    // path both when not set (which will be empty_path so you can distinguish
+    // the two case if you really want to) and when set to empty. Note that
+    // this means there could be a race between path and mtime (unless you
+    // lock the target in some other way; see file_rule) so in this case it
+    // makes sense to set the timestamp first.
     //
     const path_type&
     path () const;