From 36fba3138858ee42ba3f03e3de0efa1d1f49d147 Mon Sep 17 00:00:00 2001 From: David Kastrup Date: Wed, 27 Apr 2016 19:41:06 +0200 Subject: [PATCH] Issue 4835: Move Callback_wrapper class to separate file and simplify 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 0000000000..531029543b --- /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 + + 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 . +*/ + +#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 0000000000..b797baf062 --- /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 + + 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 . +*/ + +#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 +{ + // 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 + 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 27e7d85b71..b9042ba301 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 () -// 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 -{ - // 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 - static void trampoline (SCM target, SCM ev) + static SCM trampoline (SCM target, SCM ev) { T *t = unsmob (target); LY_ASSERT_SMOB (T, target, 1); (t->*callback) (ev); - } - template - 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 (target); - LY_ASSERT_SMOB (T, target, 1); - LY_ASSERT_SMOB (Stream_event, event, 2); - - t->protect_event (event); - (t->*callback) (unsmob (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 - static SCM make_smob () - { - static SCM res = scm_permanent_object - (Callback_wrapper (trampoline).smobbed_copy ()); - return res; + return SCM_UNDEFINED; } }; -#define GET_LISTENER(cl, proc) get_listener (Callback_wrapper::make_smob ()) +#define GET_LISTENER(cl, proc) get_listener (Callback_wrapper::make_smob > ()) #endif /* LISTENER_HH */ diff --git a/lily/include/translator.hh b/lily/include/translator.hh index a4cf192fff..118220772a 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 + static SCM trampoline (SCM target, SCM event) + { + T *t = unsmob (target); + LY_ASSERT_SMOB (T, target, 1); + LY_ASSERT_SMOB (Stream_event, event, 2); + + t->protect_event (event); + (t->*callback) (unsmob (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 d505702136..250a923137 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 (), \ - listener_list_); \ + Callback_wrapper::make_smob \ + > (), 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 c1e0b442e2..2a8d28d8cb 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.39.2