Iterate over group members in test rule only if resolvable

5 files changed, 64 insertions, 33 deletions

diff --git a/build2/dist/rule.cxx b/build2/dist/rule.cxx
index bf5ab47..205c321 100644
--- a/build2/dist/rule.cxx
+++ b/build2/dist/rule.cxx
@@ -26,23 +26,16 @@ namespace build2
     {
       const dir_path& out_root (t.root_scope ().out_path ());
 
-      auto r (group_prerequisite_members (a, t, false));
-      for (auto i (r.begin ()); i != r.end (); ++i)
+      // If we can, go inside see-through groups.
+      //
+      for (prerequisite_member p:
+             group_prerequisite_members (a, t, members_mode::maybe))
       {
-        prerequisite_member p (*i);
-
         // Skip prerequisites imported from other projects.
         //
         if (p.proj ())
           continue;
 
-        // If we can, go inside see-through groups. Note that here we are
-        // not going into ad hoc groups but maybe we should (which would
-        // have to be done after match()).
-        //
-        if (p.type ().see_through && i.enter_group ())
-          continue;
-
         const target& pt (p.search ());
 
         // Don't match targets that are outside of our project.
diff --git a/build2/target b/build2/target
index 71571bc..b5a8f78 100644
--- a/build2/target
+++ b/build2/target
@@ -925,19 +925,29 @@ namespace build2
   template <typename R>
   class prerequisite_members_range;
 
+  // See-through/ad hoc group members iteration mode. Unless the mode is never,
+  // ad hoc members are always iterated over.
+  //
+  enum class members_mode
+  {
+    always, // Iterate over members, assert if not resolvable.
+    maybe,  // Iterate over members if resolvable, group otherwise.
+    never   // Iterate over group (can still use enter_group()).
+  };
+
   template <typename R>
   inline prerequisite_members_range<R>
-  prerequisite_members (action a, R&& r, bool members = true)
+  prerequisite_members (action a, R&& r, members_mode m = members_mode::always)
   {
-    return prerequisite_members_range<R> (a, forward<R> (r), members);
+    return prerequisite_members_range<R> (a, forward<R> (r), m);
   }
 
   template <typename R>
   class prerequisite_members_range
   {
   public:
-    prerequisite_members_range (action a, R&& r, bool m)
-        : a_ (a), members_ (m), r_ (forward<R> (r)), e_ (r_.end ()) {}
+    prerequisite_members_range (action a, R&& r, members_mode m)
+        : a_ (a), mode_ (m), r_ (forward<R> (r)), e_ (r_.end ()) {}
 
     using base_iterator = decltype (declval<R> ().begin ());
 
@@ -953,7 +963,9 @@ namespace build2
       iterator (const prerequisite_members_range* r, const base_iterator& i)
           : r_ (r), i_ (i), g_ {nullptr, 0}, k_ (nullptr)
       {
-        if (r_->members_ && i_ != r_->e_ && i_->type.see_through)
+        if (r_->mode_ != members_mode::never &&
+            i_ != r_->e_                     &&
+            i_->type.see_through)
           switch_mode ();
       }
 
@@ -971,7 +983,7 @@ namespace build2
       // information is not available. Similar to leave_group(), you should
       // increment the iterator after calling this function (provided it
       // returned true). Note that it cannot be used on ad hoc groups (which
-      // will be always be entered).
+      // will be always entered).
       //
       bool
       enter_group ();
@@ -1043,7 +1055,7 @@ namespace build2
 
   private:
     action a_;
-    bool members_; // Go into group members by default?
+    members_mode mode_;
     R r_;
     base_iterator e_;
   };
@@ -1051,27 +1063,31 @@ namespace build2
   // prerequisite_members(t.prerequisites)
   //
   inline auto
-  prerequisite_members (action a, target& t, bool members = true)
+  prerequisite_members (action a, target& t,
+                        members_mode m = members_mode::always)
   {
-    return prerequisite_members (a, t.prerequisites (), members);
+    return prerequisite_members (a, t.prerequisites (), m);
   }
 
   inline auto
-  prerequisite_members (action a, const target& t, bool members = true)
+  prerequisite_members (action a, const target& t,
+                        members_mode m = members_mode::always)
   {
-    return prerequisite_members (a, t.prerequisites (), members);
+    return prerequisite_members (a, t.prerequisites (), m);
   }
 
   // prerequisite_members(reverse_iterate(t.prerequisites))
   //
   inline auto
-  reverse_prerequisite_members (action a, target& t, bool m = true)
+  reverse_prerequisite_members (action a, target& t,
+                                members_mode m = members_mode::always)
   {
     return prerequisite_members (a, reverse_iterate (t.prerequisites ()), m);
   }
 
   inline auto
-  reverse_prerequisite_members (action a, const target& t, bool m = true)
+  reverse_prerequisite_members (action a, const target& t,
+                                members_mode m = members_mode::always)
   {
     return prerequisite_members (a, reverse_iterate (t.prerequisites ()), m);
   }
@@ -1079,13 +1095,15 @@ namespace build2
   // prerequisite_members(group_prerequisites (t))
   //
   inline auto
-  group_prerequisite_members (action a, target& t, bool m = true)
+  group_prerequisite_members (action a, target& t,
+                              members_mode m = members_mode::always)
   {
     return prerequisite_members (a, group_prerequisites (t), m);
   }
 
   inline auto
-  group_prerequisite_members (action a, const target& t, bool m = true)
+  group_prerequisite_members (action a, const target& t,
+                              members_mode m = members_mode::always)
   {
     return prerequisite_members (a, group_prerequisites (t), m);
   }
@@ -1093,14 +1111,16 @@ namespace build2
   // prerequisite_members(reverse_iterate (group_prerequisites (t)))
   //
   inline auto
-  reverse_group_prerequisite_members (action a, target& t, bool m = true)
+  reverse_group_prerequisite_members (action a, target& t,
+                                      members_mode m = members_mode::always)
   {
     return prerequisite_members (
       a, reverse_iterate (group_prerequisites (t)), m);
   }
 
   inline auto
-  reverse_group_prerequisite_members (action a, const target& t, bool m = true)
+  reverse_group_prerequisite_members (action a, const target& t,
+                                      members_mode m = members_mode::always)
   {
     return prerequisite_members (
       a, reverse_iterate (group_prerequisites (t)), m);
diff --git a/build2/target.ixx b/build2/target.ixx
index f581e5e..0be5aed 100644
--- a/build2/target.ixx
+++ b/build2/target.ixx
@@ -198,7 +198,7 @@ namespace build2
   {
     if (k_ != nullptr) // Iterating over an ad hoc group.
       k_ = k_->member;
-    else if (r_->members_)
+    else if (r_->mode_ != members_mode::never)
     {
       // Get the target if one has been resolved and see if it's an ad hoc
       // group. If so, switch to the ad hoc mode.
@@ -223,7 +223,9 @@ namespace build2
     {
       ++i_;
 
-      if (r_->members_ && i_ != r_->e_ && i_->type.see_through)
+      if (r_->mode_ != members_mode::never &&
+          i_ != r_->e_                     &&
+          i_->type.see_through)
         switch_mode ();
     }
 
diff --git a/build2/target.txx b/build2/target.txx
index 7a222d7..f885e80 100644
--- a/build2/target.txx
+++ b/build2/target.txx
@@ -19,7 +19,14 @@ namespace build2
     do
     {
       g_ = resolve_group_members (r_->a_, search (*i_));
-      assert (g_.members != nullptr); // Group could not be resolved.
+
+      // Group could not be resolved.
+      //
+      if (g_.members == nullptr)
+      {
+        assert (r_->mode_ != members_mode::always);
+        return;
+      }
 
       if (g_.count != 0) // Skip empty see through groups.
       {
diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx
index 9861baf..0a346d8 100644
--- a/build2/test/rule.cxx
+++ b/build2/test/rule.cxx
@@ -63,7 +63,15 @@ namespace build2
         // If we have any prerequisites of the test{} type, then this is the
         // testscript case.
         //
-        for (prerequisite_member p: group_prerequisite_members (a, t))
+        // If we can, go inside see-through groups. Normally groups won't be
+        // resolvable for this action but then normally they won't contain any
+        // testscripts either. In other words, if there is a group that
+        // contains testscripts as members then it will need to arrange for
+        // the members to be resolvable (e.g., by registering an appropriate
+        // rule for the test operation).
+        //
+        for (prerequisite_member p:
+               group_prerequisite_members (a, t, members_mode::maybe))
         {
           if (p.is_a<testscript> ())
           {
@@ -215,7 +223,8 @@ namespace build2
 
         // Collect all the testscript targets in prerequisite_targets.
         //
-        for (prerequisite_member p: group_prerequisite_members (a, t))
+        for (prerequisite_member p:
+               group_prerequisite_members (a, t, members_mode::maybe))
         {
           if (p.is_a<testscript> ())
             t.prerequisite_targets.push_back (&p.search ());