Cleanup clean_extra() mess (pun intended)

4 files changed, 110 insertions, 79 deletions

diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index 10f1cdf..f9f0ea2 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -1925,9 +1925,9 @@ namespace build2
   }
 
   target_state
-  clean_extra (action a,
-               const file& ft,
-               initializer_list<initializer_list<const char*>> extra)
+  perform_clean_extra (action a, const file& ft,
+                       const clean_extras& extras,
+                       const clean_adhoc_extras& adhoc_extras)
   {
     // Clean the extras first and don't print the commands at verbosity level
     // below 3. Note the first extra file/directory that actually got removed
@@ -1941,7 +1941,7 @@ namespace build2
 
     auto clean_extra = [&er, &ed, &ep] (const file& f,
                                         const path* fp,
-                                        initializer_list<const char*> es)
+                                        const clean_extras& es)
     {
       for (const char* e: es)
       {
@@ -2016,14 +2016,8 @@ namespace build2
 
     const path& fp (ft.path ());
 
-    auto ei (extra.begin ()), ee (extra.end ());
-    if (ei != ee)
-    {
-      if (!fp.empty ())
-        clean_extra (ft, nullptr, *ei);
-
-      ++ei;
-    }
+    if (!fp.empty () && !extras.empty ())
+      clean_extra (ft, nullptr, extras);
 
     target_state tr (target_state::unchanged);
 
@@ -2046,8 +2040,18 @@ namespace build2
       if (mf == nullptr || mp->empty ())
         continue;
 
-      if (ei != ee)
-        clean_extra (*mf, mp, *ei++);
+      if (!adhoc_extras.empty ())
+      {
+        auto i (find_if (adhoc_extras.begin (),
+                         adhoc_extras.end (),
+                         [mf] (const clean_adhoc_extra& e)
+                         {
+                           return mf->is_a (e.type);
+                         }));
+
+        if (i != adhoc_extras.end ())
+          clean_extra (*mf, mp, i->extras);
+      }
 
       if (!clean)
         continue;
@@ -2117,7 +2121,7 @@ namespace build2
   {
     const file& f (t.as<file> ());
     assert (!f.path ().empty ());
-    return clean_extra (a, f, {nullptr});
+    return perform_clean_extra (a, f, {});
   }
 
   target_state
@@ -2125,7 +2129,7 @@ namespace build2
   {
     const file& f (t.as<file> ());
     assert (!f.path ().empty ());
-    return clean_extra (a, f, {".d"});
+    return perform_clean_extra (a, f, {".d"});
   }
 
   target_state
@@ -2133,7 +2137,7 @@ namespace build2
   {
     const mtime_target& g (xg.as<mtime_target> ());
 
-    // Similar logic to clean_extra() above.
+    // Similar logic to perform_clean_extra() above.
     //
     target_state r (target_state::unchanged);
 
@@ -2158,7 +2162,7 @@ namespace build2
   target_state
   perform_clean_group_depdb (action a, const target& g)
   {
-    // The same twisted target state merging logic as in clean_extra().
+    // The same twisted target state merging logic as in perform_clean_extra().
     //
     target_state er (target_state::unchanged);
     path ep;
diff --git a/build2/algorithm.hxx b/build2/algorithm.hxx
index e3473ad..b246c37 100644
--- a/build2/algorithm.hxx
+++ b/build2/algorithm.hxx
@@ -190,11 +190,13 @@ namespace build2
   // 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.
+  // Note that here and in find_adhoc_member() below (as well as in
+  // perform_clean_extra()) 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 we have no need
+  // for multiple members of the same type. This also allows us to support
+  // things like changing the ad hoc member name by declaring it in a
+  // buildfile.
   //
   target&
   add_adhoc_member (target&,
@@ -691,30 +693,44 @@ namespace build2
   // paths or "path derivation directives". The directive string can be NULL,
   // or empty in which case it is ignored. If the last character in a
   // directive is '/', then the resulting path is treated as a directory
-  // rather than a file.  The directive can start with zero or more '-'
+  // rather than a file. The directive can start with zero or more '-'
   // characters which indicate the number of extensions that should be
   // stripped before the new extension (if any) is added (so if you want to
   // strip the extension, specify just "-"). For example:
   //
-  // clean_extra (a, t, {".d", ".dlls/", "-.dll"});
+  // perform_clean_extra (a, t, {".d", ".dlls/", "-.dll"});
   //
   // The extra files/directories are removed first in the specified order
   // followed by the ad hoc group member, then target itself, and, finally,
   // the prerequisites in the reverse order.
   //
-  // You can also clean extra files derived from ad hoc group members.
+  // You can also clean extra files derived from ad hoc group members that are
+  // "indexed" using using their target types (see add/find_adhoc_member() for
+  // details).
   //
-  // Note that if the target path is empty then it is assumed "unreal" and
-  // is not cleaned (but its prerequisites/members still are).
+  // Note that if the target path is empty then it is assumed "unreal" and is
+  // not cleaned (but its prerequisites/members still are).
   //
+  using clean_extras = small_vector<const char*, 8>;
+
+  struct clean_adhoc_extra
+  {
+    const target_type& type;
+    clean_extras       extras;
+  };
+
+  using clean_adhoc_extras = small_vector<clean_adhoc_extra, 2>;
+
   target_state
-  clean_extra (action, const file&,
-               initializer_list<initializer_list<const char*>> extra);
+  perform_clean_extra (action, const file&,
+                       const clean_extras&,
+                       const clean_adhoc_extras& = {});
 
   inline target_state
-  clean_extra (action a, const file& f, initializer_list<const char*> extra)
+  perform_clean_extra (action a, const file& f,
+                       initializer_list<const char*> e)
   {
-    return clean_extra (a, f, {extra});
+    return perform_clean_extra (a, f, clean_extras (e));
   }
 
   // Update/clean a backlink issuing appropriate diagnostics at appropriate
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index 3458a1b..bdfa0f0 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -5880,18 +5880,17 @@ namespace build2
     {
       const file& t (xt.as<file> ());
 
-      using ct = compiler_type;
+      clean_extras extras;
 
       switch (ctype)
       {
-      case ct::gcc:   return clean_extra (a, t, {".d", x_pext, ".t"});
-      case ct::clang: return clean_extra (a, t, {".d", x_pext});
-      case ct::msvc:  return clean_extra (a, t, {".d", x_pext, ".idb", ".pdb"});
-      case ct::icc:   return clean_extra (a, t, {".d"});
+      case compiler_type::gcc:   extras = {".d", x_pext, ".t"};          break;
+      case compiler_type::clang: extras = {".d", x_pext};                break;
+      case compiler_type::msvc:  extras = {".d", x_pext, ".idb", ".pdb"};break;
+      case compiler_type::icc:   extras = {".d"};                        break;
       }
 
-      assert (false);
-      return target_state::unchanged;
+      return perform_clean_extra (a, t, extras);
     }
   }
 }
diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index 0365a3b..80ba503 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -2780,65 +2780,77 @@ namespace build2
     perform_clean (action a, const target& xt) const
     {
       const file& t (xt.as<file> ());
+
       ltype lt (link_type (t));
+      const match_data& md (t.data<match_data> ());
 
-      //@@ TODO add .t to clean if _WIN32 (currently that would be just too
-      //   messy).
+      clean_extras extras;
+      clean_adhoc_extras adhoc_extras;
 
-      if (lt.executable ())
+      if (md.binless)
+        ; // Clean prerequsites/members.
+      else
       {
-        if (tclass == "windows")
+        if (tclass != "windows")
         {
-          if (tsys == "mingw32")
-            return clean_extra (
-              a, t, {".d", ".dlls/", ".manifest.o", ".manifest"});
-          else
-            // Assuming it's VC or alike. Clean up .ilk in case the user
-            // enabled incremental linking (note that .ilk replaces .exe).
+          if (lt.shared_library ())
+          {
+            // Here we can have a bunch of symlinks that we need to remove. If
+            // the paths are empty, then they will be ignored.
             //
-            return clean_extra (
-              a, t, {".d", ".dlls/", ".manifest", "-.ilk"});
-        }
-        // For other platforms it's the defaults.
-      }
-      else
-      {
-        const match_data& md (t.data<match_data> ());
+            const libs_paths& paths (md.libs_data);
 
-        if (md.binless)
-          return clean_extra (a, t, {nullptr}); // Clean prerequsites/members.
+            extras = {".d",
+                      paths.link.string ().c_str (),
+                      paths.soname.string ().c_str (),
+                      paths.interm.string ().c_str ()};
+          }
 
-        if (lt.shared_library ())
+          // For executable and static library it's the default.
+        }
+        else if (tsys == "mingw32")
         {
-          if (tclass == "windows")
+          if (lt.executable ())
           {
-            // Assuming it's VC or alike. Clean up .exp and .ilk.
+            extras = {".d", ".dlls/", ".manifest.o", ".manifest"};
+          }
+
+          // For shared and static library it's the default.
+        }
+        else
+        {
+          // Assuming MSVC or alike.
+          //
+          if (lt.executable ())
+          {
+            // Clean up .ilk in case the user enabled incremental linking
+            // (notice that the .ilk extension replaces .exe).
+            //
+            extras = {".d", ".dlls/", ".manifest", "-.ilk"};
+          }
+          else if (lt.shared_library ())
+          {
+            // Clean up .ilk and .exp.
             //
             // 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"}});
+            extras = {".d", "-.ilk"};
+            adhoc_extras.push_back ({libi::static_type, {"-.exp"}});
           }
-          else
-          {
-            // Here we can have a bunch of symlinks that we need to remove. If
-            // the paths are empty, then they will be ignored.
-            //
-            const libs_paths& paths (md.libs_data);
 
-            return clean_extra (a, t, {".d",
-                  paths.link.string ().c_str (),
-                  paths.soname.string ().c_str (),
-                  paths.interm.string ().c_str ()});
-          }
+          // For static library it's the default.
         }
-        // For static library it's the defaults.
+
+        if (extras.empty ())
+          extras = {".d"}; // Default.
+
+#ifdef _WIN32
+        extras.push_back (".t"); // Options file.
+#endif
       }
 
-      return clean_extra (a, t, {".d"});
+      return perform_clean_extra (a, t, extras, adhoc_extras);
     }
   }
 }