Re: [RFA] Convert observers to C++

[Pedro Alves <palves at redhat dot com>]

On 02/10/2018 03:38 AM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> I'm running this through the buildbot again and when it is working I
> Tom> will re-submit it.
> 
> Here it is.

Thanks again.  I have a few comments / suggestions.  See below, and
the attached patches.

Offhand, let me say that after playing with this today, I found
that this being plain C++ instead of shell-based really makes it
much easier to play around with the implementation.  I like that,
a lot.

> +template<typename... T>
> +class observable
> +{
> +public:
> +
> +  typedef std::function<void(T...)> func_type;
> +
> +  /* The return type of attach, which can be passed to detach to
> +     remove a listener.  */
> +  typedef size_t token_type;
> +
> +  /* A "null" value of token_type that can be used as a
> +     sentinel.  */
> +  static constexpr const token_type null_token = size_t (-1);
> +
> +  explicit observable (const char *name)
> +    : m_count (0),
> +      m_name (name)
> +  {
> +  }
> +
> +  DISABLE_COPY_AND_ASSIGN (observable);
> +
> +  /* Attach F as an observable to this observable.  A token is
> +     returned which can be used to later remove F.  */
> +  token_type attach (const func_type &f)
> +  {
> +    m_observers.emplace_back (m_count, f);
> +    return m_count++;
> +  }


The counter is a size_t, so in principle at least for 64-bit
architectures, it shouldn't ever be a problem in practice (as in
risk of overflow), but my pedantic self still wants to try to see if
we can avoid forever-increasing counters by design.

How about we replace the counter by key that is managed by
the caller?  Very much similar to the registry mechanism.
In turn that allows registering multiple observers with
the same key, meaning at places where we need to
attach/detach a number of observers, we don't need to
track a token per each observable.  See patch #1 attached.

> +
> +  /* Remove an observer from this observable.  F is the token that was
> +     previously returned by "attach".  */
> +  void detach (const token_type &f)
> +  {
> +    auto iter = std::remove_if (m_observers.begin (),
> +				m_observers.end (),
> +				[=] (const std::pair<size_t, func_type> &e)
> +				{
> +				  return e.first == f;
> +				});
> +
> +    m_observers.erase (iter, m_observers.end ());
> +  }
> +
> +  /* Notify all observables that are attached to this observer.  */

I think observable and observer are swapped in this comment.  (see patch)

> +  void notify (T... args) const
> +  {
> +    if (observer_debug)
> +      fprintf_unfiltered (gdb_stdlog, "observer %s notify() called\n",
> +			  m_name);
> +    for (auto &&e : m_observers)
> +      e.second (args...);
> +  }
> +
> +private:
> +
> +  size_t m_count;
> +  std::vector<std::pair<size_t, func_type>> m_observers;
> +  const char *m_name;
> +};
> +
> +} /* namespace observers */
> +
> +} /* namespace gdb */



> --- a/gdb/observer.c
> +++ b/gdb/observer.c
> @@ -17,199 +17,199 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> -/* An observer is an entity who is interested in being notified when GDB
> -   reaches certain states, or certain events occur in GDB.  The entity being
> -   observed is called the Subject.  To receive notifications, the observer
> -   attaches a callback to the subject.  One subject can have several
> -   observers.
> -
> -   This file implements an internal generic low-level event notification
> -   mechanism based on the Observer paradigm described in the book "Design
> -   Patterns".  This generic event notification mechansim is then re-used
> -   to implement the exported high-level notification management routines
> -   for all possible notifications.
> -
> -   The current implementation of the generic observer provides support
> -   for contextual data.  This contextual data is given to the subject
> -   when attaching the callback.  In return, the subject will provide
> -   this contextual data back to the observer as a parameter of the
> -   callback.
> -
> -   FIXME: The current support for the contextual data is only partial,
> -   as it lacks a mechanism that would deallocate this data when the
> -   callback is detached.  This is not a problem so far, as this contextual
> -   data is only used internally to hold a function pointer.  Later on,
> -   if a certain observer needs to provide support for user-level
> -   contextual data, then the generic notification mechanism will need
> -   need to be enhanced to allow the observer to provide a routine to
> -   deallocate the data when attaching the callback.
> -
> -   This file is currently maintained by hand, but the long term plan
> -   if the number of different notifications starts growing is to create
> -   a new script (observer.sh) that would generate this file, and the
> -   associated documentation.  */
> -
>  #include "defs.h"
>  #include "observer.h"
>  #include "command.h"
>  #include "gdbcmd.h"
> +#include "selftest.h"
>  
> -static unsigned int observer_debug;
> -static void
> -show_observer_debug (struct ui_file *file, int from_tty,
> -		     struct cmd_list_element *c, const char *value)
> +namespace gdb
>  {
> -  fprintf_filtered (file, _("Observer debugging is %s.\n"), value);
> -}
> -
> -/* The internal generic observer.  */
>  
> -typedef void (generic_observer_notification_ftype) (const void *data,
> -						    const void *args);
> -
> -struct observer
> +namespace observers
>  {
> -  generic_observer_notification_ftype *notify;
> -  /* No memory management needed for the following field for now.  */
> -  void *data;
> -};
> -
> -/* A list of observers, maintained by the subject.  A subject is
> -   actually represented by its list of observers.  */
>  
> -struct observer_list
> +unsigned int observer_debug;
> +
> +#define DEFINE_OBSERVER(name) decltype (name) name (# name)

Hmm, pedantically, I guess this should be
called DEFINE_OBSERVABLE instead of DEFINE_OBSERVER?
(see patch #3).

> +
> +DEFINE_OBSERVER (normal_stop);
> +DEFINE_OBSERVER (signal_received);
> +DEFINE_OBSERVER (end_stepping_range);


...

> +
> +#ifdef GDB_SELF_TEST
> +
> +namespace selftests
>  {
> -  struct observer_list *next;
> -  struct observer *observer;
> -};
>  
> -/* Allocate a struct observer_list, intended to be used as a node
> -   in the list of observers maintained by a subject.  */
> +/* This observer is used for internal testing.  */
> +observable<int> test_notification ("test_notification");

We have the gdb/unittests/ directory nowadays, so I think
we could/should move the tests there.  See patch #2.

And with patches #2 and #3, I wonder whether gdb/observer.{h,c}
is the right name for the files, given that they contain
definitions of observables, not observers?  Maybe
they should be called gdb/observables.{h,c} instead (plural).
Dunno, it's not a big deal.  Fine with me as is, though I
wonder whether that's because I'm used to the nomenclature;
i.e., whether a newcomer would feel the same.

WDYT?

Thanks,
Pedro Alves

>From 1c21617fc888261094851bd94b35fd1bbfaba650 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 12 Feb 2018 12:36:16 +0000
Subject: [PATCH 1/3] Use address-indentify tokens instead of counters.  Fix
 some typos.

---
 gdb/common/observable.h | 58 ++++++++++++++++++++++-----------------
 gdb/observer.c          | 16 +++++------
 gdb/record-btrace.c     | 22 +++++----------
 gdb/tui/tui-hooks.c     | 72 ++++++++++++++++++++-----------------------------
 4 files changed, 76 insertions(+), 92 deletions(-)

diff --git a/gdb/common/observable.h b/gdb/common/observable.h
index 118718871fe..f5666c14e5c 100644
--- a/gdb/common/observable.h
+++ b/gdb/common/observable.h
@@ -42,56 +42,65 @@ extern unsigned int observer_debug;
    particular, it is therefore not possible to call the attach or
    detach routines during a notification.  */
 
+/* The type of a key that can be passed to attach, which can be passed
+   to detach to remove associated observers.  Tokens have address
+   identity, and are thus usually const globals.  */
+struct token
+{
+  token () = default;
+
+  DISABLE_COPY_AND_ASSIGN (token);
+};
+
 template<typename... T>
 class observable
 {
 public:
 
-  typedef std::function<void(T...)> func_type;
-
-  /* The return type of attach, which can be passed to detach to
-     remove a listener.  */
-  typedef size_t token_type;
-
-  /* A "null" value of token_type that can be used as a
-     sentinel.  */
-  static constexpr const token_type null_token = size_t (-1);
+  typedef std::function<void (T...)> func_type;
 
   explicit observable (const char *name)
-    : m_count (0),
-      m_name (name)
+    : m_name (name)
   {
   }
 
   DISABLE_COPY_AND_ASSIGN (observable);
 
-  /* Attach F as an observable to this observable.  A token is
-     returned which can be used to later remove F.  */
-  token_type attach (const func_type &f)
+  /* Attach F as an observer to this observable.  F cannot be
+     detached.  */
+  void attach (const func_type &f)
+  {
+    m_observers.emplace_back (nullptr, f);
+  }
+
+  /* Attach F as an observer to this observable.  T is a reference to
+     a token that can be used to later remove F.  */
+  void attach (const func_type &f, const token &t)
   {
-    m_observers.emplace_back (m_count, f);
-    return m_count++;
+    m_observers.emplace_back (&t, f);
   }
 
-  /* Remove an observer from this observable.  F is the token that was
-     previously returned by "attach".  */
-  void detach (const token_type &f)
+  /* Remove observers associated with T from this observable.  T is
+     the token that was previously passed to any number of "attach"
+     calls.  */
+  void detach (const token &t)
   {
     auto iter = std::remove_if (m_observers.begin (),
 				m_observers.end (),
-				[=] (const std::pair<size_t, func_type> &e)
+				[&] (const std::pair<const token *,
+				     func_type> &e)
 				{
-				  return e.first == f;
+				  return e.first == &t;
 				});
 
     m_observers.erase (iter, m_observers.end ());
   }
 
-  /* Notify all observables that are attached to this observer.  */
+  /* Notify all observers that are attached to this observable.  */
   void notify (T... args) const
   {
     if (observer_debug)
-      fprintf_unfiltered (gdb_stdlog, "observer %s notify() called\n",
+      fprintf_unfiltered (gdb_stdlog, "observable %s notify() called\n",
 			  m_name);
     for (auto &&e : m_observers)
       e.second (args...);
@@ -99,8 +108,7 @@ public:
 
 private:
 
-  size_t m_count;
-  std::vector<std::pair<size_t, func_type>> m_observers;
+  std::vector<std::pair<const token *, func_type>> m_observers;
   const char *m_name;
 };
 
diff --git a/gdb/observer.c b/gdb/observer.c
index d39e3e7462e..7e44c8c0181 100644
--- a/gdb/observer.c
+++ b/gdb/observer.c
@@ -128,10 +128,10 @@ observer_self_tests ()
      attached.  */
   notify_check_counters (0, 0, 0);
 
-  observable<int>::token_type token1, token2, token3;
+  const gdb::observers::token token1, token2, token3;
 
   /* Now, attach one observer, and send a notification.  */
-  token2 = test_notification.attach (&test_second_notification_function);
+  test_notification.attach (&test_second_notification_function, token2);
   notify_check_counters (0, 1, 0);
 
   /* Remove the observer, and send a notification.  */
@@ -139,15 +139,15 @@ observer_self_tests ()
   notify_check_counters (0, 0, 0);
 
   /* With a new observer.  */
-  token1 = test_notification.attach (&test_first_notification_function);
+  test_notification.attach (&test_first_notification_function, token1);
   notify_check_counters (1, 0, 0);
 
   /* With 2 observers.  */
-  token2 = test_notification.attach (&test_second_notification_function);
+  test_notification.attach (&test_second_notification_function, token2);
   notify_check_counters (1, 1, 0);
 
   /* With 3 observers.  */
-  token3 = test_notification.attach (&test_third_notification_function);
+  test_notification.attach (&test_third_notification_function, token3);
   notify_check_counters (1, 1, 1);
 
   /* Remove middle observer.  */
@@ -164,9 +164,9 @@ observer_self_tests ()
 
   /* Go back to 3 observers, and remove them in a different
      order...  */
-  token1 = test_notification.attach (&test_first_notification_function);
-  token2 = test_notification.attach (&test_second_notification_function);
-  token3 = test_notification.attach (&test_third_notification_function);
+  test_notification.attach (&test_first_notification_function, token1);
+  test_notification.attach (&test_second_notification_function, token2);
+  test_notification.attach (&test_third_notification_function, token3);
   notify_check_counters (1, 1, 1);
 
   /* Remove the third observer.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index c0f36268eae..660ce407c96 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -44,9 +44,9 @@
 /* The target_ops of record-btrace.  */
 static struct target_ops record_btrace_ops;
 
-/* A new thread observer enabling branch tracing for the new thread.  */
-static gdb::observers::observable<struct thread_info *>::token_type
-  record_btrace_thread_observer;
+/* Token associated with a new-thread observer enabling branch tracing
+   for the new thread.  */
+static const gdb::observers::token record_btrace_thread_observer_token;
 
 /* Memory access types used in set/show record btrace replay-memory-access.  */
 static const char replay_memory_access_read_only[] = "read-only";
@@ -177,8 +177,8 @@ record_btrace_auto_enable (void)
 {
   DEBUG ("attach thread observer");
 
-  record_btrace_thread_observer
-    = gdb::observers::new_thread.attach (record_btrace_enable_warn);
+  gdb::observers::new_thread.attach (record_btrace_enable_warn,
+				     record_btrace_thread_observer_token);
 }
 
 /* Disable automatic tracing of new threads.  */
@@ -186,14 +186,9 @@ record_btrace_auto_enable (void)
 static void
 record_btrace_auto_disable (void)
 {
-  /* The observer may have been detached, already.  */
-  if (record_btrace_thread_observer == gdb::observers::new_thread.null_token)
-    return;
-
   DEBUG ("detach thread observer");
 
-  gdb::observers::new_thread.detach (record_btrace_thread_observer);
-  record_btrace_thread_observer = gdb::observers::new_thread.null_token;
+  gdb::observers::new_thread.detach (record_btrace_thread_observer_token);
 }
 
 /* The record-btrace async event handler function.  */
@@ -239,9 +234,6 @@ record_btrace_open (const char *args, int from_tty)
   if (!target_has_execution)
     error (_("The program is not being run."));
 
-  gdb_assert (record_btrace_thread_observer
-	      == gdb::observers::new_thread.null_token);
-
   disable_chain = make_cleanup (null_cleanup, NULL);
   ALL_NON_EXITED_THREADS (tp)
     if (args == NULL || *args == 0 || number_is_in_list (args, tp->global_num))
@@ -3139,6 +3131,4 @@ to see the actual buffer size."), NULL, show_record_pt_buffer_size_value,
 
   record_btrace_conf.bts.size = 64 * 1024;
   record_btrace_conf.pt.size = 16 * 1024;
-
-  record_btrace_thread_observer = gdb::observers::new_thread.null_token;
 }
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index ba87aec2e15..8d0bc8711a1 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -203,21 +203,32 @@ tui_normal_stop (struct bpstats *bs, int print_frame)
   tui_refresh_frame_and_register_information (/*registers_too_p=*/1);
 }
 
-/* Observers created when installing TUI hooks.  */
-static gdb::observers::observable<struct breakpoint *>::token_type
-  tui_bp_created_observer;
-static gdb::observers::observable<struct breakpoint *>::token_type
-  tui_bp_deleted_observer;
-static gdb::observers::observable<struct breakpoint *>::token_type
-  tui_bp_modified_observer;
-static gdb::observers::observable<struct inferior *>::token_type
-  tui_inferior_exit_observer;
-static gdb::observers::observable<const char *>::token_type
-  tui_before_prompt_observer;
-static gdb::observers::observable<struct bpstats *, int>::token_type
-  tui_normal_stop_observer;
-static gdb::observers::observable<struct frame_info *, int>::token_type
-  tui_register_changed_observer;
+/* Token associated with observers registered while TUI hooks are
+   installed.  */
+static const gdb::observers::token tui_observers_token;
+
+/* Attach or detach TUI observers, according to ATTACH.  */
+
+static void
+tui_attach_detach_observers (bool attach)
+{
+  auto ad = [&] (auto &observable,
+		 const auto &func)
+    {
+      if (attach)
+	observable.attach (func, tui_observers_token);
+      else
+	observable.detach (tui_observers_token);
+    };
+
+  ad (gdb::observers::breakpoint_created, tui_event_create_breakpoint);
+  ad (gdb::observers::breakpoint_deleted, tui_event_delete_breakpoint);
+  ad (gdb::observers::breakpoint_modified, tui_event_modify_breakpoint);
+  ad (gdb::observers::inferior_exit, tui_inferior_exit);
+  ad (gdb::observers::before_prompt, tui_before_prompt);
+  ad (gdb::observers::normal_stop, tui_normal_stop);
+  ad (gdb::observers::register_changed, tui_register_changed);
+}
 
 /* Install the TUI specific hooks.  */
 void
@@ -231,20 +242,7 @@ tui_install_hooks (void)
     = tui_dummy_print_frame_info_listing_hook;
 
   /* Install the event hooks.  */
-  tui_bp_created_observer
-    = gdb::observers::breakpoint_created.attach (tui_event_create_breakpoint);
-  tui_bp_deleted_observer
-    = gdb::observers::breakpoint_deleted.attach (tui_event_delete_breakpoint);
-  tui_bp_modified_observer
-    = gdb::observers::breakpoint_modified.attach (tui_event_modify_breakpoint);
-  tui_inferior_exit_observer
-    = gdb::observers::inferior_exit.attach (tui_inferior_exit);
-  tui_before_prompt_observer
-    = gdb::observers::before_prompt.attach (tui_before_prompt);
-  tui_normal_stop_observer
-    = gdb::observers::normal_stop.attach (tui_normal_stop);
-  tui_register_changed_observer
-    = gdb::observers::register_changed.attach (tui_register_changed);
+  tui_attach_detach_observers (true);
 }
 
 /* Remove the TUI specific hooks.  */
@@ -253,21 +251,9 @@ tui_remove_hooks (void)
 {
   deprecated_print_frame_info_listing_hook = 0;
   deprecated_query_hook = 0;
+
   /* Remove our observers.  */
-  gdb::observers::breakpoint_created.detach (tui_bp_created_observer);
-  tui_bp_created_observer = gdb::observers::breakpoint_created.null_token;
-  gdb::observers::breakpoint_deleted.detach (tui_bp_deleted_observer);
-  tui_bp_deleted_observer = gdb::observers::breakpoint_deleted.null_token;
-  gdb::observers::breakpoint_modified.detach (tui_bp_modified_observer);
-  tui_bp_modified_observer = gdb::observers::breakpoint_modified.null_token;
-  gdb::observers::inferior_exit.detach (tui_inferior_exit_observer);
-  tui_inferior_exit_observer = gdb::observers::inferior_exit.null_token;
-  gdb::observers::before_prompt.detach (tui_before_prompt_observer);
-  tui_before_prompt_observer = gdb::observers::before_prompt.null_token;
-  gdb::observers::normal_stop.detach (tui_normal_stop_observer);
-  tui_normal_stop_observer = gdb::observers::normal_stop.null_token;
-  gdb::observers::register_changed.detach (tui_register_changed_observer);
-  tui_register_changed_observer = gdb::observers::register_changed.null_token;
+  tui_attach_detach_observers (false);
 }
 
 void
-- 
2.14.3

>From ac316fd054692670e426e8395f700091604d44f1 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 12 Feb 2018 17:42:03 +0000
Subject: [PATCH 2/3] Move unit tests to gdb/unittests/

---
 gdb/Makefile.in                    |   1 +
 gdb/observer.c                     | 116 -------------------------------
 gdb/unittests/observer-selftests.c | 135 +++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+), 116 deletions(-)
 create mode 100644 gdb/unittests/observer-selftests.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 844f92d7cde..2b09850bd6c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -421,6 +421,7 @@ SUBDIR_UNITTESTS_SRCS = \
 	unittests/memory-map-selftests.c \
 	unittests/memrange-selftests.c \
 	unittests/offset-type-selftests.c \
+	unittests/observer-selftests.c \
 	unittests/optional-selftests.c \
 	unittests/ptid-selftests.c \
 	unittests/rsp-low-selftests.c \
diff --git a/gdb/observer.c b/gdb/observer.c
index 7e44c8c0181..836b083125d 100644
--- a/gdb/observer.c
+++ b/gdb/observer.c
@@ -21,7 +21,6 @@
 #include "observer.h"
 #include "command.h"
 #include "gdbcmd.h"
-#include "selftest.h"
 
 namespace gdb
 {
@@ -76,116 +75,6 @@ DEFINE_OBSERVER (inferior_call_post);
 DEFINE_OBSERVER (register_changed);
 DEFINE_OBSERVER (user_selected_context_changed);
 
-#ifdef GDB_SELF_TEST
-
-namespace selftests
-{
-
-/* This observer is used for internal testing.  */
-observable<int> test_notification ("test_notification");
-
-static int test_first_observer = 0;
-static int test_second_observer = 0;
-static int test_third_observer = 0;
-
-static void
-test_first_notification_function (int arg)
-{
-  test_first_observer++;
-}
-
-static void
-test_second_notification_function (int arg)
-{
-  test_second_observer++;
-}
-
-static void
-test_third_notification_function (int arg)
-{
-  test_third_observer++;
-}
-
-static void
-notify_check_counters (int one, int two, int three)
-{
-  /* Reset.  */
-  test_first_observer = 0;
-  test_second_observer = 0;
-  test_third_observer = 0;
-  /* Notify.  */
-  test_notification.notify (0);
-  /* Check.  */
-  SELF_CHECK (one == test_first_observer);
-  SELF_CHECK (two == test_second_observer);
-  SELF_CHECK (three == test_third_observer);
-}
-
-static void
-observer_self_tests ()
-{
-  /* First, try sending a notification without any observer
-     attached.  */
-  notify_check_counters (0, 0, 0);
-
-  const gdb::observers::token token1, token2, token3;
-
-  /* Now, attach one observer, and send a notification.  */
-  test_notification.attach (&test_second_notification_function, token2);
-  notify_check_counters (0, 1, 0);
-
-  /* Remove the observer, and send a notification.  */
-  test_notification.detach (token2);
-  notify_check_counters (0, 0, 0);
-
-  /* With a new observer.  */
-  test_notification.attach (&test_first_notification_function, token1);
-  notify_check_counters (1, 0, 0);
-
-  /* With 2 observers.  */
-  test_notification.attach (&test_second_notification_function, token2);
-  notify_check_counters (1, 1, 0);
-
-  /* With 3 observers.  */
-  test_notification.attach (&test_third_notification_function, token3);
-  notify_check_counters (1, 1, 1);
-
-  /* Remove middle observer.  */
-  test_notification.detach (token2);
-  notify_check_counters (1, 0, 1);
-
-  /* Remove first observer.  */
-  test_notification.detach (token1);
-  notify_check_counters (0, 0, 1);
-
-  /* Remove last observer.  */
-  test_notification.detach (token3);
-  notify_check_counters (0, 0, 0);
-
-  /* Go back to 3 observers, and remove them in a different
-     order...  */
-  test_notification.attach (&test_first_notification_function, token1);
-  test_notification.attach (&test_second_notification_function, token2);
-  test_notification.attach (&test_third_notification_function, token3);
-  notify_check_counters (1, 1, 1);
-
-  /* Remove the third observer.  */
-  test_notification.detach (token3);
-  notify_check_counters (1, 1, 0);
-
-  /* Remove the second observer.  */
-  test_notification.detach (token2);
-  notify_check_counters (1, 0, 0);
-
-  /* Remove first observer, no more observers.  */
-  test_notification.detach (token1);
-  notify_check_counters (0, 0, 0);
-}
-
-} /* namespace selftests */
-
-#endif
-
 } /* namespace observers */
 } /* namespace gdb */
 
@@ -207,9 +96,4 @@ When non-zero, observer debugging is enabled."),
 			     NULL,
 			     show_observer_debug,
 			     &setdebuglist, &showdebuglist);
-
-#if GDB_SELF_TEST
-  selftests::register_test ("gdb::observers",
-			    gdb::observers::selftests::observer_self_tests);
-#endif
 }
diff --git a/gdb/unittests/observer-selftests.c b/gdb/unittests/observer-selftests.c
new file mode 100644
index 00000000000..6f2f7e8bccf
--- /dev/null
+++ b/gdb/unittests/observer-selftests.c
@@ -0,0 +1,135 @@
+/* Self tests for gdb::observers, GDB notifications to observers.
+
+   Copyright (C) 2003-2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "selftest.h"
+#include "common/observable.h"
+
+namespace selftests {
+namespace observers {
+
+gdb::observers::observable<int> test_notification ("test_notification");
+
+static int test_first_observer = 0;
+static int test_second_observer = 0;
+static int test_third_observer = 0;
+
+static void
+test_first_notification_function (int arg)
+{
+  test_first_observer++;
+}
+
+static void
+test_second_notification_function (int arg)
+{
+  test_second_observer++;
+}
+
+static void
+test_third_notification_function (int arg)
+{
+  test_third_observer++;
+}
+
+static void
+notify_check_counters (int one, int two, int three)
+{
+  /* Reset.  */
+  test_first_observer = 0;
+  test_second_observer = 0;
+  test_third_observer = 0;
+  /* Notify.  */
+  test_notification.notify (0);
+  /* Check.  */
+  SELF_CHECK (one == test_first_observer);
+  SELF_CHECK (two == test_second_observer);
+  SELF_CHECK (three == test_third_observer);
+}
+
+static void
+run_tests ()
+{
+  /* First, try sending a notification without any observer
+     attached.  */
+  notify_check_counters (0, 0, 0);
+
+  const gdb::observers::token token1, token2, token3;
+
+  /* Now, attach one observer, and send a notification.  */
+  test_notification.attach (&test_second_notification_function, token2);
+  notify_check_counters (0, 1, 0);
+
+  /* Remove the observer, and send a notification.  */
+  test_notification.detach (token2);
+  notify_check_counters (0, 0, 0);
+
+  /* With a new observer.  */
+  test_notification.attach (&test_first_notification_function, token1);
+  notify_check_counters (1, 0, 0);
+
+  /* With 2 observers.  */
+  test_notification.attach (&test_second_notification_function, token2);
+  notify_check_counters (1, 1, 0);
+
+  /* With 3 observers.  */
+  test_notification.attach (&test_third_notification_function, token3);
+  notify_check_counters (1, 1, 1);
+
+  /* Remove middle observer.  */
+  test_notification.detach (token2);
+  notify_check_counters (1, 0, 1);
+
+  /* Remove first observer.  */
+  test_notification.detach (token1);
+  notify_check_counters (0, 0, 1);
+
+  /* Remove last observer.  */
+  test_notification.detach (token3);
+  notify_check_counters (0, 0, 0);
+
+  /* Go back to 3 observers, and remove them in a different
+     order...  */
+  test_notification.attach (&test_first_notification_function, token1);
+  test_notification.attach (&test_second_notification_function, token2);
+  test_notification.attach (&test_third_notification_function, token3);
+  notify_check_counters (1, 1, 1);
+
+  /* Remove the third observer.  */
+  test_notification.detach (token3);
+  notify_check_counters (1, 1, 0);
+
+  /* Remove the second observer.  */
+  test_notification.detach (token2);
+  notify_check_counters (1, 0, 0);
+
+  /* Remove first observer, no more observers.  */
+  test_notification.detach (token1);
+  notify_check_counters (0, 0, 0);
+}
+
+} /* namespace observers */
+} /* namespace selftests */
+
+void
+_initialize_observer_selftest ()
+{
+  selftests::register_test ("gdb::observers",
+			    selftests::observers::run_tests);
+}
-- 
2.14.3

>From 5618a1c3d564387dff4799538e16849d2b1df816 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 12 Feb 2018 17:55:14 +0000
Subject: [PATCH 3/3] DEFINE_OBSERVER -> DEFINE_OBSERVABLE

---
 gdb/observer.c | 86 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/gdb/observer.c b/gdb/observer.c
index 836b083125d..d85ecd2cce7 100644
--- a/gdb/observer.c
+++ b/gdb/observer.c
@@ -30,50 +30,50 @@ namespace observers
 
 unsigned int observer_debug;
 
-#define DEFINE_OBSERVER(name) decltype (name) name (# name)
+#define DEFINE_OBSERVABLE(name) decltype (name) name (# name)
 
-DEFINE_OBSERVER (normal_stop);
-DEFINE_OBSERVER (signal_received);
-DEFINE_OBSERVER (end_stepping_range);
-DEFINE_OBSERVER (signal_exited);
-DEFINE_OBSERVER (exited);
-DEFINE_OBSERVER (no_history);
-DEFINE_OBSERVER (sync_execution_done);
-DEFINE_OBSERVER (command_error);
-DEFINE_OBSERVER (target_changed);
-DEFINE_OBSERVER (executable_changed);
-DEFINE_OBSERVER (inferior_created);
-DEFINE_OBSERVER (record_changed);
-DEFINE_OBSERVER (solib_loaded);
-DEFINE_OBSERVER (solib_unloaded);
-DEFINE_OBSERVER (new_objfile);
-DEFINE_OBSERVER (free_objfile);
-DEFINE_OBSERVER (new_thread);
-DEFINE_OBSERVER (thread_exit);
-DEFINE_OBSERVER (thread_stop_requested);
-DEFINE_OBSERVER (target_resumed);
-DEFINE_OBSERVER (about_to_proceed);
-DEFINE_OBSERVER (breakpoint_created);
-DEFINE_OBSERVER (breakpoint_deleted);
-DEFINE_OBSERVER (breakpoint_modified);
-DEFINE_OBSERVER (traceframe_changed);
-DEFINE_OBSERVER (architecture_changed);
-DEFINE_OBSERVER (thread_ptid_changed);
-DEFINE_OBSERVER (inferior_added);
-DEFINE_OBSERVER (inferior_appeared);
-DEFINE_OBSERVER (inferior_exit);
-DEFINE_OBSERVER (inferior_removed);
-DEFINE_OBSERVER (memory_changed);
-DEFINE_OBSERVER (before_prompt);
-DEFINE_OBSERVER (gdb_datadir_changed);
-DEFINE_OBSERVER (command_param_changed);
-DEFINE_OBSERVER (tsv_created);
-DEFINE_OBSERVER (tsv_deleted);
-DEFINE_OBSERVER (tsv_modified);
-DEFINE_OBSERVER (inferior_call_pre);
-DEFINE_OBSERVER (inferior_call_post);
-DEFINE_OBSERVER (register_changed);
-DEFINE_OBSERVER (user_selected_context_changed);
+DEFINE_OBSERVABLE (normal_stop);
+DEFINE_OBSERVABLE (signal_received);
+DEFINE_OBSERVABLE (end_stepping_range);
+DEFINE_OBSERVABLE (signal_exited);
+DEFINE_OBSERVABLE (exited);
+DEFINE_OBSERVABLE (no_history);
+DEFINE_OBSERVABLE (sync_execution_done);
+DEFINE_OBSERVABLE (command_error);
+DEFINE_OBSERVABLE (target_changed);
+DEFINE_OBSERVABLE (executable_changed);
+DEFINE_OBSERVABLE (inferior_created);
+DEFINE_OBSERVABLE (record_changed);
+DEFINE_OBSERVABLE (solib_loaded);
+DEFINE_OBSERVABLE (solib_unloaded);
+DEFINE_OBSERVABLE (new_objfile);
+DEFINE_OBSERVABLE (free_objfile);
+DEFINE_OBSERVABLE (new_thread);
+DEFINE_OBSERVABLE (thread_exit);
+DEFINE_OBSERVABLE (thread_stop_requested);
+DEFINE_OBSERVABLE (target_resumed);
+DEFINE_OBSERVABLE (about_to_proceed);
+DEFINE_OBSERVABLE (breakpoint_created);
+DEFINE_OBSERVABLE (breakpoint_deleted);
+DEFINE_OBSERVABLE (breakpoint_modified);
+DEFINE_OBSERVABLE (traceframe_changed);
+DEFINE_OBSERVABLE (architecture_changed);
+DEFINE_OBSERVABLE (thread_ptid_changed);
+DEFINE_OBSERVABLE (inferior_added);
+DEFINE_OBSERVABLE (inferior_appeared);
+DEFINE_OBSERVABLE (inferior_exit);
+DEFINE_OBSERVABLE (inferior_removed);
+DEFINE_OBSERVABLE (memory_changed);
+DEFINE_OBSERVABLE (before_prompt);
+DEFINE_OBSERVABLE (gdb_datadir_changed);
+DEFINE_OBSERVABLE (command_param_changed);
+DEFINE_OBSERVABLE (tsv_created);
+DEFINE_OBSERVABLE (tsv_deleted);
+DEFINE_OBSERVABLE (tsv_modified);
+DEFINE_OBSERVABLE (inferior_call_pre);
+DEFINE_OBSERVABLE (inferior_call_post);
+DEFINE_OBSERVABLE (register_changed);
+DEFINE_OBSERVABLE (user_selected_context_changed);
 
 } /* namespace observers */
 } /* namespace gdb */
-- 
2.14.3