Remove order dependence in ad hoc group handling

Also, don't match group_recipe since we neither execute nor access the
state.

9 files changed, 127 insertions, 108 deletions

diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index cb38905..bcdccf6 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -1330,10 +1330,11 @@ namespace build2
         // we will also copy .pdb).
         //
         // Note that we want to avoid group or tt/patter-spec lookup. And
-        // since this is an ad hoc member (which means it was added by the
-        // rule), we assume that the value, if any, will be set as a rule-
-        // specific variable (since setting it as a target-specific wouldn't
-        // be MT-safe).
+        // since this is an ad hoc member (which means it was either declared
+        // in the buildfile or added by the rule), we assume that the value,
+        // if any, will be set as a rule-specific variable (since setting it
+        // as a target-specific wouldn't be MT-safe). @@ Don't think this
+        // applies to declared ad hoc members.
         //
         lookup l (mt->state[a].vars[var_backlink]);
 
diff --git a/build2/algorithm.hxx b/build2/algorithm.hxx
index 5f0854f..f1aad0c 100644
--- a/build2/algorithm.hxx
+++ b/build2/algorithm.hxx
@@ -170,7 +170,7 @@ namespace build2
   const target_lock*
   dependency_cycle (action, const target&);
 
-  // If the target is already applied (for this action ) or executed, then no
+  // If the target is already applied (for this action) or executed, then no
   // lock is acquired. Otherwise, the target must not yet be matched for this
   // action.
   //
@@ -184,8 +184,6 @@ namespace build2
   // existing member of this target type is the same. Return the locked member
   // target.
   //
-  // @@ Maybe the same type and name?
-  //
   target_lock
   add_adhoc_member (action,
                     target&,
@@ -194,33 +192,57 @@ namespace build2
                     const dir_path& out,
                     const string& name);
 
-  // If the suffix is specified, it is added (as an extension) to the member's
-  // target 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* suffix = nullptr);
+                    const char* ext = nullptr);
 
   template <typename T>
   inline target_lock
-  add_adhoc_member (action a, target& t, const char* s = nullptr)
+  add_adhoc_member (action a, target& g, const char* e = nullptr)
   {
-    return add_adhoc_member (a, t, T::static_type, s);
+    return add_adhoc_member (a, g, T::static_type, e);
   }
 
   // Find an ad hoc member of the specified target type returning NULL if not
   // found.
   //
+  target*
+  find_adhoc_member (target&, const target_type&);
+
   const target*
   find_adhoc_member (const target&, const target_type&);
 
   template <typename T>
+  inline T*
+  find_adhoc_member (target& g, const target_type& tt)
+  {
+    return static_cast<T*> (find_adhoc_member (g, tt));
+  }
+
+  template <typename T>
   inline const T*
-  find_adhoc_member (const target& t)
+  find_adhoc_member (const target& g, const target_type& tt)
+  {
+    return static_cast<const T*> (find_adhoc_member (g, tt));
+  }
+
+  template <typename T>
+  inline const T*
+  find_adhoc_member (const target& g)
+  {
+    return find_adhoc_member<T> (g, T::static_type);
+  }
+
+  template <typename T>
+  inline T*
+  find_adhoc_member (target& g)
   {
-    return static_cast<const T*> (find_adhoc_member (t, T::static_type));
+    return find_adhoc_member<T> (g, T::static_type);
   }
 
   // Match and apply a rule to the action/target with ambiguity detection.
diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx
index a01ee58..0150587 100644
--- a/build2/algorithm.ixx
+++ b/build2/algorithm.ixx
@@ -7,14 +7,6 @@
 
 namespace build2
 {
-  inline const target*
-  find_adhoc_member (const target& t, const target_type& tt)
-  {
-    const target* m (t.member);
-    for (; m != nullptr && !m->is_a (tt); m = m->member) ;
-    return m;
-  }
-
   inline const target&
   search (const target& t, const prerequisite& p)
   {
@@ -241,19 +233,35 @@ namespace build2
   }
 
   inline target_lock
-  add_adhoc_member (action a, target& t, const target_type& tt, const char* s)
+  add_adhoc_member (action a, target& t, const target_type& tt, const char* e)
   {
     string n (t.name);
 
-    if (s != nullptr)
+    if (e != nullptr)
     {
       n += '.';
-      n += s;
+      n += e;
     }
 
     return add_adhoc_member (a, t, tt, t.dir, t.out, n);
   }
 
+  inline target*
+  find_adhoc_member (target& g, const target_type& tt)
+  {
+    target* m (g.member);
+    for (; m != nullptr && !m->is_a (tt); m = m->member) ;
+    return m;
+  }
+
+  inline const target*
+  find_adhoc_member (const target& g, const target_type& tt)
+  {
+    const target* m (g.member);
+    for (; m != nullptr && !m->is_a (tt); m = m->member) ;
+    return m;
+  }
+
   const rule_match*
   match_impl (action, target&, const rule* skip, bool try_match = false);
 
diff --git a/build2/cc/common.cxx b/build2/cc/common.cxx
index aca1240..d4c60ec 100644
--- a/build2/cc/common.cxx
+++ b/build2/cc/common.cxx
@@ -657,11 +657,11 @@ namespace build2
               {
                 if (l.owns_lock ())
                 {
-                  s->member = i;
+                  s->member = i; // We are first.
                   l.unlock ();
                 }
                 else
-                  assert (s->member == i);
+                  assert (find_adhoc_member<libi> (*s) == i);
 
                 i->mtime (mt);
                 i->path (move (f));
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index 96e2dc5..cd49689 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -679,7 +679,6 @@ namespace build2
           //
           target_lock obj (add_adhoc_member (a, t, tts.obj, e.c_str ()));
           obj.target->as<file> ().derive_path (o);
-          match_recipe (obj, group_recipe); // Set recipe and unlock.
         }
       }
 
@@ -5408,6 +5407,7 @@ namespace build2
 
       otype ot (compile_type (t, ut));
       linfo li (link_info (bs, ot));
+      compile_target_types tts (compile_types (ot));
 
       environment env;
       cstrings args {cpath.recall_string ()};
@@ -5419,7 +5419,7 @@ namespace build2
       path relo (ut == unit_type::module_header
                  ? path ()
                  : relative (ut == unit_type::module_iface
-                             ? t.member->is_a<file> ()->path ()
+                             ? find_adhoc_member<file> (t, tts.obj)->path ()
                              : tp));
 
       // Build the command line.
diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index b873e40..e333dd0 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -291,7 +291,7 @@ namespace build2
         lk = b;
         append_ext (lk);
 
-        libi& li (ls.member->as<libi> ()); // Note: libi is locked.
+        libi& li (*find_adhoc_member<libi> (ls)); // Note: libi is locked.
         lk = li.derive_path (move (lk), tsys == "mingw32" ? "a" : "lib");
       }
       else if (!v.empty ())
@@ -404,7 +404,7 @@ namespace build2
       // invocation. So for libraries we ignore them later, on pass 3.
       //
       optional<dir_paths> usr_lib_dirs; // Extract lazily.
-      compile_target_types tt (compile_types (ot));
+      compile_target_types tts (compile_types (ot));
 
       auto skip = [&a, &rs] (const target* pt) -> bool
       {
@@ -457,8 +457,8 @@ namespace build2
           bool group (!p.prerequisite.belongs (t)); // Group's prerequisite.
 
           const target_type& rtt (mod
-                                  ? (group ? bmi::static_type : tt.bmi)
-                                  : (group ? obj::static_type : tt.obj));
+                                  ? (group ? bmi::static_type : tts.bmi)
+                                  : (group ? obj::static_type : tts.obj));
 
           const prerequisite_key& cp (p.key ()); // Source key.
 
@@ -537,8 +537,8 @@ namespace build2
           // If this is the obj{} or bmi{} target group, then pick the
           // appropriate member.
           //
-          if      (p.is_a<obj> ()) pt = &search (t, tt.obj, p.key ());
-          else if (p.is_a<bmi> ()) pt = &search (t, tt.bmi, p.key ());
+          if      (p.is_a<obj> ()) pt = &search (t, tts.obj, p.key ());
+          else if (p.is_a<bmi> ()) pt = &search (t, tts.bmi, p.key ());
           //
           // Windows module definition (.def). For other platforms (and for
           // static libraries) treat it as an ordinary prerequisite.
@@ -717,9 +717,6 @@ namespace build2
                   libi = add_adhoc_member<bin::libi> (a, t);
 
                 md.libs_data = derive_libs_paths (t, p, s);
-
-                if (libi)
-                  match_recipe (libi, group_recipe); // Set recipe and unlock.
               }
 
               break;
@@ -734,8 +731,6 @@ namespace build2
             if (find_option ("/DEBUG", t, c_loptions, true) ||
                 find_option ("/DEBUG", t, x_loptions, true))
             {
-              // Note: add after the import library if any.
-              //
               target_lock pdb (
                 add_adhoc_member (a, t, *bs.find_target_type ("pdb")));
 
@@ -743,8 +738,6 @@ namespace build2
               // we can have both foo.exe and foo.dll in the same directory.
               //
               pdb.target->as<file> ().derive_path (t.path (), "pdb");
-
-              match_recipe (pdb, group_recipe); // Set recipe and unlock.
             }
           }
 
@@ -767,14 +760,12 @@ namespace build2
             // 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 suffixes rather
-            // that a./.lib/.so/.dylib/.dll.
+            // is also the reason why we use the .static/.shared second-level
+            // extensions rather that a./.lib/.so/.dylib/.dll.
             //
             pc.target->as<file> ().derive_path (nullptr,
                                                 (p == nullptr ? "lib" : p),
                                                 s);
-
-            match_recipe (pc, group_recipe); // Set recipe and unlock.
           }
 
           // Add the Windows rpath emulating assembly directory as fsdir{}.
@@ -816,7 +807,6 @@ namespace build2
 #ifdef _WIN32
             dir.target->state[a].assign (var_backlink) = "copy";
 #endif
-            match_recipe (dir, group_recipe); // Set recipe and unlock.
           }
         }
       }
@@ -864,12 +854,12 @@ namespace build2
           //
           pt =
             group
-            ? &search (t, (mod ? tt.bmi : tt.obj), rt.dir, rt.out, rt.name)
+            ? &search (t, (mod ? tts.bmi : tts.obj), rt.dir, rt.out, rt.name)
             : &rt;
 
           const target_type& rtt (mod
-                                  ? (group ? bmi::static_type : tt.bmi)
-                                  : (group ? obj::static_type : tt.obj));
+                                  ? (group ? bmi::static_type : tts.bmi)
+                                  : (group ? obj::static_type : tts.obj));
 
           // If this obj*{} already has prerequisites, then verify they are
           // "compatible" with what we are doing here. Otherwise, synthesize
@@ -919,7 +909,7 @@ namespace build2
               //
               if (p.is_a<libx> () ||
                   p.is_a<liba> () || p.is_a<libs> () || p.is_a<libux> () ||
-                  p.is_a<bmi> ()  || p.is_a (tt.bmi))
+                  p.is_a<bmi> ()  || p.is_a (tts.bmi))
               {
                 ps.push_back (p.as_prerequisite ());
               }
@@ -1148,8 +1138,8 @@ namespace build2
               bool group (!p.prerequisite.belongs (t));
 
               const target_type& rtt (mod
-                                      ? (group ? bmi::static_type : tt.bmi)
-                                      : (group ? obj::static_type : tt.obj));
+                                      ? (group ? bmi::static_type : tts.bmi)
+                                      : (group ? obj::static_type : tts.obj));
 
               fail << "synthesized dependency for prerequisite " << p << " "
                    << "would be incompatible with existing target " << *pt <<
@@ -1189,11 +1179,12 @@ namespace build2
     {
       struct data
       {
-        strings&    args;
-        const file& l;
-        action      a;
-        linfo       li;
-      } d {args, l, a, li};
+        strings&             args;
+        const file&          l;
+        action               a;
+        linfo                li;
+        compile_target_types tts;
+      } d {args, l, a, li, compile_types (li.type)};
 
       auto imp = [] (const file&, bool la)
       {
@@ -1263,7 +1254,7 @@ namespace build2
               if (modules)
               {
                 if (pt->is_a<bmix> ()) // @@ MODHDR: hbmix{} has no objx{}
-                  pt = pt->member;
+                  pt = find_adhoc_member (*pt, d.tts.obj);
               }
 
               // We could have dependency diamonds with utility libraries.
@@ -1288,13 +1279,14 @@ namespace build2
               return;
 
             // On Windows a shared library is a DLL with the import library as
-            // a first ad hoc group member. MinGW though can link directly to
-            // DLLs (see search_library() for details).
+            // an ad hoc group member. MinGW though can link directly to DLLs
+            // (see search_library() for details).
             //
-            if (l->member != nullptr &&
-                l->is_a<libs> ()     &&
-                tclass == "windows")
-              l = &l->member->as<file> ();
+            if (tclass == "windows" && l->is_a<libs> ())
+            {
+              if (const libi* li = find_adhoc_member<libi> (*l))
+                l = li;
+            }
 
             string p (relative (l->path ()).string ());
 
@@ -1413,14 +1405,15 @@ namespace build2
           //
           d.update = d.update || l->newer (d.mt);
 
-          // On Windows a shared library is a DLL with the import library as a
-          // first ad hoc group member. MinGW though can link directly to DLLs
+          // On Windows a shared library is a DLL with the import library as
+          // an ad hoc group member. MinGW though can link directly to DLLs
           // (see search_library() for details).
           //
-          if (l->member != nullptr &&
-              l->is_a<libs> ()     &&
-              tclass == "windows")
-            l = &l->member->as<file> ();
+          if (tclass == "windows" && l->is_a<libs> ())
+          {
+            if (const libi* li = find_adhoc_member<libi> (*l))
+              l = li;
+          }
 
           d.cs.append (f);
           hash_path (d.cs, l->path (), d.out_root);
@@ -1630,6 +1623,7 @@ namespace build2
       ltype lt (link_type (t));
       otype ot (lt.type);
       linfo li (link_info (bs, ot));
+      compile_target_types tts (compile_types (ot));
 
       bool binless (md.binless);
       assert (ot != otype::e || !binless); // Sanity check.
@@ -2056,7 +2050,7 @@ namespace build2
           if (modules)
           {
             if (pt->is_a<bmix> ()) // @@ MODHDR: hbmix{} has no objx{}
-              pt = pt->member;
+              pt = find_adhoc_member (*pt, tts.obj);
           }
 
           const file* f;
@@ -2262,26 +2256,30 @@ namespace build2
 
           if (ot == otype::s)
           {
-            // On Windows libs{} is the DLL and its first ad hoc group member
-            // is the import library.
+            // On Windows libs{} is the DLL and an ad hoc group member is the
+            // import library.
             //
             // This will also create the .exp export file. Its name will be
             // derived from the import library by changing the extension.
             // Lucky for us -- there is no option to name it.
             //
-            auto& imp (t.member->as<file> ());
-            out2 = "/IMPLIB:" + relative (imp.path ()).string ();
+            const file& imp (*find_adhoc_member<libi> (t));
+
+            out2 = "/IMPLIB:";
+            out2 += relative (imp.path ()).string ();
             args.push_back (out2.c_str ());
           }
 
-          // If we have /DEBUG then name the .pdb file. It is either the first
-          // (exe) or the second (dll) ad hoc group member.
+          // If we have /DEBUG then name the .pdb file. It is an ad hoc group
+          // member.
           //
           if (find_option ("/DEBUG", args, true))
           {
-            auto& pdb (
-              (ot == otype::e ? t.member : t.member->member)->as<file> ());
-            out1 = "/PDB:" + relative (pdb.path ()).string ();
+            const file& pdb (
+              *find_adhoc_member<file> (t, *bs.find_target_type ("pdb")));
+
+            out1 = "/PDB:";
+            out1 += relative (pdb.path ()).string ();
             args.push_back (out1.c_str ());
           }
 
@@ -2315,10 +2313,10 @@ namespace build2
 
                 if (tsys == "mingw32")
                 {
-                  // On Windows libs{} is the DLL and its first ad hoc group
-                  // member is the import library.
+                  // On Windows libs{} is the DLL and an ad hoc group member
+                  // is the import library.
                   //
-                  auto& imp (t.member->as<file> ());
+                  const file& imp (*find_adhoc_member<libi> (t));
                   out = "-Wl,--out-implib=" + relative (imp.path ()).string ();
                   args.push_back (out.c_str ());
                 }
@@ -2363,7 +2361,7 @@ namespace build2
         if (modules)
         {
           if (pt->is_a<bmix> ()) // @@ MODHDR: hbmix{} has no objx{}
-            pt = pt->member;
+            pt = find_adhoc_member (*pt, tts.obj);
         }
 
         const file* f;
@@ -2806,6 +2804,8 @@ namespace build2
             // Note that .exp is based on the .lib, not .dll name. And with
             // versioning their bases may not be the same.
             //
+            // @@ ADHOC: member order.
+            //
             if (tsys != "mingw32")
               return clean_extra (a, t, {{".d", "-.ilk"}, {"-.exp"}});
           }
diff --git a/build2/cc/msvc.cxx b/build2/cc/msvc.cxx
index 32805c7..08cd43c 100644
--- a/build2/cc/msvc.cxx
+++ b/build2/cc/msvc.cxx
@@ -8,6 +8,7 @@
 #include <build2/target.hxx>
 #include <build2/context.hxx>
 #include <build2/variable.hxx>
+#include <build2/algorithm.hxx>
 #include <build2/filesystem.hxx>
 #include <build2/diagnostics.hxx>
 
@@ -177,7 +178,7 @@ namespace build2
           //
           path i (
             lt == otype::s
-            ? t.member->as<file> ().path ().leaf ()
+            ? find_adhoc_member<libi> (t)->path ().leaf ()
             : t.path ().leaf ().base () + ".lib");
 
           if (l.find (i.string ())                  != string::npos &&
@@ -467,11 +468,11 @@ namespace build2
           {
             if (l.owns_lock ())
             {
-              s->member = i;
+              s->member = i; // We are first.
               l.unlock ();
             }
             else
-              assert (s->member == i);
+              assert (find_adhoc_member<libi> (*s) == i);
 
             // Presumably there is a DLL somewhere, we just don't know where.
             //
diff --git a/build2/parser.cxx b/build2/parser.cxx
index 27910b7..06be54a 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -985,6 +985,8 @@ 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)
         {
@@ -1004,26 +1006,11 @@ namespace build2
           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 ();
-
-      // Pre-match this target. Feels fuzzy/hacky.
-      //
-      // See match_recipe() and set_recipe() that it calls for the
-      // approximate semantics we want to achieve.
-      //
-      // @@ Can such a target be used as a prerequisite? Feels like
-      //    will require a "permanenly applied" task_count value? Maybe
-      //    special "adhoc" value?
-      //
-      {
-        auto& i (at.state.data[0]); // inner opstate
-        auto& o (at.state.data[1]); // outer opstate
-
-        i.rule   = o.rule   = nullptr;
-        i.recipe = o.recipe = group_recipe;
-        i.state  = o.state  = target_state::group;
-      }
     }
   }
 
diff --git a/build2/target.ixx b/build2/target.ixx
index a2be81c..30be02f 100644
--- a/build2/target.ixx
+++ b/build2/target.ixx
@@ -292,8 +292,8 @@ namespace build2
   group () const
   {
     return
-      k_ != nullptr ? k_->member != nullptr                  : /* ad hoc */
-      g_.count != 0 ? g_.members != nullptr && j_ < g_.count : /* normal */
+      k_ != nullptr ? k_->member != nullptr                  : /* ad hoc   */
+      g_.count != 0 ? g_.members != nullptr && j_ < g_.count : /* explicit */
       false;
   }