[binutils-gdb] gdbsupport/selftest: Replace for_each_selftest with an iterator_range

[binutils-gdb] gdbsupport/selftest: Replace for_each_selftest with an iterator_range

[Lancelot SIX]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c57207c15c4fa980263e6849d0e6472c33e647fc

commit c57207c15c4fa980263e6849d0e6472c33e647fc
Author: Lancelot SIX <lancelot.six@amd.com>
Date:   Wed Mar 23 15:29:53 2022 +0000

    gdbsupport/selftest: Replace for_each_selftest with an iterator_range
    
    Remove the callback-based selftests::for_each_selftest function and use
    an iterator_range instead.
    
    Also use this iterator range in run_tests so all iterations over the
    selftests are done in a consistent way.  This will become useful in a
    later commit.
    
    Change-Id: I0b3a5349a7987fbcb0071f11c394e353df986583

Diff:
---
 gdb/maint.c            | 13 ++++++-------
 gdbsupport/selftest.cc | 27 ++++++++++++---------------
 gdbsupport/selftest.h  | 31 ++++++++++++++++++++++++-------
 3 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/gdb/maint.c b/gdb/maint.c
index 60e183efdd1..7b726c2bc9f 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -1181,11 +1181,11 @@ maintenance_selftest_completer (cmd_list_element *cmd,
     return;
 
 #if GDB_SELF_TEST
-  selftests::for_each_selftest ([&tracker, text] (const std::string &name)
+  for (const auto &test : selftests::all_selftests ())
     {
-      if (startswith (name.c_str (), text))
-	tracker.add_completion (make_unique_xstrdup (name.c_str ()));
-    });
+      if (startswith (test.name.c_str (), text))
+	tracker.add_completion (make_unique_xstrdup (test.name.c_str ()));
+    }
 #endif
 }
 
@@ -1194,9 +1194,8 @@ maintenance_info_selftests (const char *arg, int from_tty)
 {
 #if GDB_SELF_TEST
   gdb_printf ("Registered selftests:\n");
-  selftests::for_each_selftest ([] (const std::string &name) {
-      gdb_printf (" - %s\n", name.c_str ());
-  });
+  for (const auto &test : selftests::all_selftests ())
+    gdb_printf (" - %s\n", test.name.c_str ());
 #else
   gdb_printf (_("\
 Selftests have been disabled for this build.\n"));
diff --git a/gdbsupport/selftest.cc b/gdbsupport/selftest.cc
index 466d7cfeab5..7077f113f97 100644
--- a/gdbsupport/selftest.cc
+++ b/gdbsupport/selftest.cc
@@ -20,16 +20,15 @@
 #include "common-exceptions.h"
 #include "common-debug.h"
 #include "selftest.h"
-#include <map>
 #include <functional>
 
 namespace selftests
 {
-/* All the tests that have been registered.  Using an std::map allows keeping
+/* All the tests that have been registered.  Using an std::set allows keeping
    the order of tests stable and easily looking up whether a test name
    exists.  */
 
-static std::map<std::string, std::function<void(void)>> tests;
+static selftests_registry tests;
 
 /* See selftest.h.  */
 
@@ -38,9 +37,9 @@ register_test (const std::string &name,
 	       std::function<void(void)> function)
 {
   /* Check that no test with this name already exist.  */
-  gdb_assert (tests.find (name) == tests.end ());
-
-  tests[name] = function;
+  auto status = tests.emplace (name, std::move (function));
+  if (!status.second)
+    gdb_assert_not_reached ("Test already registered");
 }
 
 /* See selftest.h.  */
@@ -63,10 +62,8 @@ run_tests (gdb::array_view<const char *const> filters, bool verbose)
   int ran = 0, failed = 0;
   run_verbose_ = verbose;
 
-  for (const auto &pair : tests)
+  for (const auto &test : all_selftests ())
     {
-      const std::string &name = pair.first;
-      const auto &test = pair.second;
       bool run = false;
 
       if (filters.empty ())
@@ -75,7 +72,7 @@ run_tests (gdb::array_view<const char *const> filters, bool verbose)
 	{
 	  for (const char *filter : filters)
 	    {
-	      if (name.find (filter) != std::string::npos)
+	      if (test.name.find (filter) != std::string::npos)
 		run = true;
 	    }
 	}
@@ -85,9 +82,9 @@ run_tests (gdb::array_view<const char *const> filters, bool verbose)
 
       try
 	{
-	  debug_printf (_("Running selftest %s.\n"), name.c_str ());
+	  debug_printf (_("Running selftest %s.\n"), test.name.c_str ());
 	  ++ran;
-	  test ();
+	  test.test ();
 	}
       catch (const gdb_exception_error &ex)
 	{
@@ -104,10 +101,10 @@ run_tests (gdb::array_view<const char *const> filters, bool verbose)
 
 /* See selftest.h.  */
 
-void for_each_selftest (for_each_selftest_ftype func)
+selftests_range
+all_selftests ()
 {
-  for (const auto &pair : tests)
-    func (pair.first);
+  return selftests_range (tests.cbegin (), tests.cend ());
 }
 
 } // namespace selftests
diff --git a/gdbsupport/selftest.h b/gdbsupport/selftest.h
index 3c343e1a761..5ca9bdcc598 100644
--- a/gdbsupport/selftest.h
+++ b/gdbsupport/selftest.h
@@ -21,6 +21,8 @@
 
 #include "gdbsupport/array-view.h"
 #include "gdbsupport/function-view.h"
+#include "gdbsupport/iterator-range.h"
+#include <set>
 
 /* A test is just a function that does some checks and throws an
    exception if something has gone wrong.  */
@@ -28,6 +30,28 @@
 namespace selftests
 {
 
+/* Selftests are registered under a unique name.  */
+
+struct selftest
+{
+  selftest (std::string name, std::function<void (void)> test)
+    : name { std::move (name) }, test { std::move (test) }
+  { }
+  bool operator< (const selftest &rhs) const
+  { return name < rhs.name; }
+
+  std::string name;
+  std::function<void (void)> test;
+};
+
+/* Type of the container of all the registered selftests.  */
+using selftests_registry = std::set<selftest>;
+using selftests_range = iterator_range<selftests_registry::const_iterator>;
+
+/* Create a range to iterate over all registered tests.  */
+
+selftests_range all_selftests ();
+
 /* True if selftest should run verbosely.  */
 
 extern bool run_verbose ();
@@ -48,13 +72,6 @@ extern void run_tests (gdb::array_view<const char *const> filters,
 
 /* Reset GDB or GDBserver's internal state.  */
 extern void reset ();
-
-using for_each_selftest_ftype
-  = gdb::function_view<void(const std::string &name)>;
-
-/* Call FUNC for each registered selftest.  */
-
-extern void for_each_selftest (for_each_selftest_ftype func);
 }
 
 /* Check that VALUE is true, and, if not, throw an exception.  */