[PATCH] Move Callback_wrapper class to separate file and simplify

[David Kastrup]

Callback_wrapper::make_smob now is only templated on the address of
the (static) trampoline function.  Moving the trampolining to the
actual functions in question makes callbacks quite more versatile and
transparent and obviates the previous need for friend declarations due
to mixing internals of other classes into the Callback_wrapper
definition.
---
 lily/callback.cc            | 22 ++++++++++++++
 lily/include/callback.hh    | 70 +++++++++++++++++++++++++++++++++++++++++++++
 lily/include/listener.hh    | 70 +++------------------------------------------
 lily/include/translator.hh  | 14 ++++++++-
 lily/include/translator.icc |  6 ++--
 lily/listener.cc            |  2 --
 6 files changed, 112 insertions(+), 72 deletions(-)
 create mode 100644 lily/callback.cc
 create mode 100644 lily/include/callback.hh

diff --git a/lily/callback.cc b/lily/callback.cc
new file mode 100644
index 0000000..5310295
--- /dev/null
+++ b/lily/callback.cc
@@ -0,0 +1,22 @@
+/*
+  This file is part of LilyPond, the GNU music typesetter.
+
+  Copyright (C) 2016  David Kastrup <address@hidden>
+
+  LilyPond 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.
+
+  LilyPond 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 LilyPond.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "callback.hh"
+
+const char * const Callback_wrapper::type_p_name_ = 0;
diff --git a/lily/include/callback.hh b/lily/include/callback.hh
new file mode 100644
index 0000000..b797baf
--- /dev/null
+++ b/lily/include/callback.hh
@@ -0,0 +1,70 @@
+/*
+  This file is part of LilyPond, the GNU music typesetter.
+
+  Copyright (C) 2016  David Kastrup <address@hidden>
+
+  LilyPond 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.
+
+  LilyPond 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 LilyPond.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef CALLBACK_HH
+#define CALLBACK_HH
+
+// A callback wrapper creates a Scheme-callable version of a fixed C++
+// function.  It is generally used for calling template-generated
+// trampoline functions leading to calling a particular member
+// function on a given Smob.
+//
+// The class itself is not templated in order not to explode the
+// number of smob types: each class can support a particular call
+// signature.
+//
+// Check the GET_LISTENER call for a typical use case.
+
+#include "smobs.hh"
+
+class Callback_wrapper : public Simple_smob<Callback_wrapper>
+{
+  // We use an ordinary function pointer pointing to a trampoline
+  // function (templated on the callback in question) instead of
+  // storing a member function pointer to a common base class like
+  // Smob_core.  The additional code for the trampolines is negligible
+  // and the performance implications of using member function
+  // pointers in connection with inheritance are somewhat opaque as
+  // this involves an adjustment of the this pointer from Smob_core to
+  // the scope containing the callback.
+  SCM (*trampoline_) (SCM, SCM);
+  Callback_wrapper (SCM (*trampoline) (SCM, SCM)) : trampoline_ (trampoline)
+  { } // Private constructor, use only in make_smob
+public:
+  static const char * const type_p_name_; // = 0
+  LY_DECLARE_SMOB_PROC (&Callback_wrapper::call, 2, 0, 0)
+  SCM call (SCM target, SCM arg)
+  {
+    return trampoline_ (target, arg);
+  }
+  // Callback wrappers are for an unchanging entity, so we do the Lisp
+  // creation just once on the first call of make_smob.  So we only
+  // get a single Callback_wrapper instance for each differently
+  // templated make_smob call.
+  template <SCM (*trampoline) (SCM, SCM)>
+  static SCM make_smob ()
+  {
+    static SCM res =
+      scm_permanent_object (Callback_wrapper (trampoline).smobbed_copy ());
+    return res;
+  }
+};
+
+
+#endif
diff --git a/lily/include/listener.hh b/lily/include/listener.hh
index 27e7d85..b9042ba 100644
--- a/lily/include/listener.hh
+++ b/lily/include/listener.hh
@@ -74,8 +74,8 @@
   classes derived from Smob<...>.
 */
 
+#include "callback.hh"
 #include "smobs.hh"
-#include "stream-event.hh"
 
 // A listener is essentially any procedure accepting a single argument
 // (namely an event).  The class Listener (or rather a smobbed_copy of
@@ -125,80 +125,18 @@ public:
     return *unchecked_unsmob (a) == *unchecked_unsmob (b)
       ? SCM_BOOL_T : SCM_BOOL_F;
   }
-};
 
-// A callback wrapper creates a Scheme-callable version of a
-// non-static class member function callback which you can call with a
-// class instance and an event.
-//
-// If you have a class member function
-// void T::my_listen (SCM ev)
-// then Callback_wrapper::make_smob<T, SCM, &T::my_listen> ()
-// will return an SCM function roughly defined as
-// (lambda (target ev) (target->my_listen ev))
-//
-// The setup is slightly tricky since the make_smob quasi-constructor
-// call is a template function templated on the given callback, and so
-// is the trampoline it uses for redirecting the callback.  The class
-// itself, however, is not templated as that would create a wagonload
-// of SCM types.
-
-class Callback_wrapper : public Simple_smob<Callback_wrapper>
-{
-  // We use an ordinary function pointer pointing to a trampoline
-  // function (templated on the callback in question) instead of
-  // storing a member function pointer to a common base class like
-  // Smob_core.  The additional code for the trampolines is negligible
-  // and the performance implications of using member function
-  // pointers in connection with inheritance are somewhat opaque as
-  // this involves an adjustment of the this pointer from Smob_core to
-  // the scope containing the callback.
-  void (*trampoline_) (SCM, SCM);
   template <class T, void (T::*callback)(SCM)>
-  static void trampoline (SCM target, SCM ev)
+  static SCM trampoline (SCM target, SCM ev)
   {
     T *t = unsmob<T> (target);
     LY_ASSERT_SMOB (T, target, 1);
 
     (t->*callback) (ev);
-  }
-  template <class T, void (T::*callback)(Stream_event *)>
-  static void trampoline (SCM target, SCM event)
-  {
-    // The same, but for callbacks for translator listeners which get
-    // the unpacked event which, in turn, gets protected previously
-
-    T *t = unsmob<T> (target);
-    LY_ASSERT_SMOB (T, target, 1);
-    LY_ASSERT_SMOB (Stream_event, event, 2);
-
-    t->protect_event (event);
-    (t->*callback) (unsmob<Stream_event> (event));
-  }
-
-  Callback_wrapper (void (*trampoline) (SCM, SCM)) : trampoline_ (trampoline)
-  { } // Private constructor, use only in make_smob
-public:
-  static const char * const type_p_name_; // = 0
-  LY_DECLARE_SMOB_PROC (&Callback_wrapper::call, 2, 0, 0)
-  SCM call (SCM target, SCM ev)
-  {
-    trampoline_ (target, ev);
-    return SCM_UNSPECIFIED;
-  }
-  // Callback wrappers are for an unchanging entity, so we do the Lisp
-  // creation just once on the first call of make_smob.  So we only
-  // get a single Callback_wrapper instance for each differently
-  // templated make_smob call.
-  template <class T, class Arg, void (T::*callback)(Arg)>
-  static SCM make_smob ()
-  {
-    static SCM res = scm_permanent_object
-      (Callback_wrapper (trampoline<T, callback>).smobbed_copy ());
-    return res;
+    return SCM_UNDEFINED;
   }
 };
 
-#define GET_LISTENER(cl, proc) get_listener (Callback_wrapper::make_smob<cl, 
SCM, &cl::proc> ())
+#define GET_LISTENER(cl, proc) get_listener 
(Callback_wrapper::make_smob<Listener::trampoline<cl, &cl::proc> > ())
 
 #endif /* LISTENER_HH */
diff --git a/lily/include/translator.hh b/lily/include/translator.hh
index a4cf192..1182207 100644
--- a/lily/include/translator.hh
+++ b/lily/include/translator.hh
@@ -133,7 +133,19 @@ public:
 protected:                      // should be private.
   Context *daddy_context_;
   void protect_event (SCM ev);
-  friend class Callback_wrapper;
+
+  template <class T, void (T::*callback)(Stream_event *)>
+  static SCM trampoline (SCM target, SCM event)
+  {
+    T *t = unsmob<T> (target);
+    LY_ASSERT_SMOB (T, target, 1);
+    LY_ASSERT_SMOB (Stream_event, event, 2);
+
+    t->protect_event (event);
+    (t->*callback) (unsmob<Stream_event> (event));
+    return SCM_UNSPECIFIED;
+  }
+
   virtual void derived_mark () const;
   static SCM event_class_symbol (const char *ev_class);
   SCM static_translator_description (const char *grobs,
diff --git a/lily/include/translator.icc b/lily/include/translator.icc
index d505702..250a923 100644
--- a/lily/include/translator.icc
+++ b/lily/include/translator.icc
@@ -20,7 +20,7 @@
 #ifndef TRANSLATOR_ICC
 #define TRANSLATOR_ICC
 
-#include "listener.hh"
+#include "callback.hh"
 #include "std-vector.hh"
 #include "translator.hh"
 
@@ -140,8 +140,8 @@ cl :: _internal_declare_ ## m ()                        \
 {                                                       \
   listener_list_ = scm_acons                                            \
     (event_class_symbol (#m),                                           \
-     Callback_wrapper::make_smob<cl, Stream_event *, &cl::listen_ ## m> (), \
-     listener_list_);                                                   \
+     Callback_wrapper::make_smob                                        \
+     <trampoline <cl, &cl::listen_ ## m> > (), listener_list_);         \
 }                                                                       \
                                                                         \
 ADD_SCM_INIT_FUNC (cl ## _declare_event_ ## m, cl::_internal_declare_ ## m);
diff --git a/lily/listener.cc b/lily/listener.cc
index c1e0b44..2a8d28d 100644
--- a/lily/listener.cc
+++ b/lily/listener.cc
@@ -19,6 +19,4 @@
 
 #include "listener.hh"
 
-const char * const Callback_wrapper::type_p_name_ = 0;
-
 const char Listener::type_p_name_[] = "ly:listener?";
-- 
2.7.4